1328526 - DocGroup labeling in dom/quota

Status: RESOLVED WONTFIX

(Core :: Storage: Quota Manager, defect, P2)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

This bug is to apply labeling APIs (https://wiki.mozilla.org/Quantum/DOM) to StorageManager & QuotaManagerService implementation.

The building block of Storage Manager is designed as followed:
  StorageManager (
       |
QuotaManagerService (XPCOM service)
       |      
     PQuota (on Pbackground protocol)

Where StorageManger is available per window/service-worker but QuotaManagerService is a singleton to be utilized by multiple windows in child process.

The issue to be clarified before applying the API is to figure how we can label the runnable of the request callback of nsIQuotaManagerService which can be used by multiple contents in child process and by internal browser script as well because it's a XPCOM service.

Same problem is available to PQuota ActorChilds even though IProtocol::SetEventTargetForActor() is available if QuotaManagerService is on top of PQuota.

The worst case is to just apply the DocGroup labeling on StorageManager.

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

After further study, we should keep QMS and PQuota mentioned in comment 0 unchanged for any DocGroup labeling because they both are singletons in content process and serve multiple instances of StorageManager to be created in each window and each worker.

The remained task is to have the DocGroup labeling for the runnable dispathed by StorageManager which are the use cases of the subclass of |WorkerMainThreadRunnable| in StorageManager to dispatch runnable from worker(off-main thread) to the main thread.

Fortunately, this is actually done in bug 1320753 by specifying a docgroup-specific MainThreadEventTarget in WorkerPrivate for further dispatching from worker to main thread used by |WorkerMainThreadRunnable::Dispatch|:
http://searchfox.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#4052-4071

Hence, I turn the status to resolved to have this bug as a marker that DocGroup labeling in dom/quota is done.

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #1)
> After further study, we should keep QMS and PQuota mentioned in comment 0
> unchanged for any DocGroup labeling because they both are singletons in
> content process and serve multiple instances of StorageManager to be created
> in each window and each worker.
After further understanding of SystemGroup API and the concern in 
https://wiki.mozilla.org/Quantum/DOM#What_is_the_impact_of_leaving_a_runnable_unlabeled.3F
The PQuota owned by main thread should at least be labeled with SystemGroup to reduce this impact caused from QuotaManager.

Reopen this bug to address this.

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

To label the runnable that runs nsIQuotaXxxCallback properly, we need to
- Refactor QMS to accept an optional nsIEventTarget per request.
  (SystemGroup will be used if not specified.)
- Set event target per PQuotaRequest/PQuotaUsageRequest.
- Refactor StorageManager to specify its DocGroup EventTarget when calling QMS requests.

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: WIP: Bug 1328526 - DocGroup labeling in dom/quota.

By current design and corresponding use cases, DocGroup labeling is only needed if StorageManager APIs are exposed to web content. However, currently,
1. There is a |pref dom.storageManager.enabled| to enable |StorageManager| and is only enabled in nightly.
2. There are 2 more StorageManager APIs: |persist()| and |persisted()| pending in bug 1286717.
Hence, I'd like to suspend the work on this bug until these 2 pending APIs are landed, but I'd like to upload this WIP patch first to show how DocGroup labeling will be done on StorageManager.estimate() by modifying the implementation in StorageManager/QuotaManagerService/PQuotaUsageRequest. The same rule can be applied for labeling the API of persist() and persisted().

Here is the summary of this patch from the bottom up,
1. We keep QuotaChild unlabeled (no actor event target specified even the SystemGroup one), because
   - It's a singleton in child process and will be created only when 1st PQuotaUsageRequest or QuotaRequest is fired. (QuotaChild itself is not belonging to any specific Docgroup. Only the QuotaUsageRequestChilds or QuotaRequestChilds have to be set to a specific DocGroup if the request is fired from StorageManager.)
   - To ensure that the creation and destruction of QuotaChild won't be unnecessarily reordered. (If SystemGroup-EventTarget is specified, flow of the creation to QuotaChild could be reordered when the 1st QuotaRequest comes from a foreground page. The reorder is also unneccesary for the QuotaChild deletion at shutdown phase.)
   - The rest QuotaChild operations like {Start, Stop}IdleMaintenance are only happened in the parent process.
2. For each PQuotaUsageRequest triggered from Web content(StorageManager), a DocGroup-EventTarget has to be set.
3. An optional parameter |nsIEventTarget*| is added in nsIQuotaManagerService::getUsageForPrincipal() for StorageManager to specify the DocGroup-EventTarget when implementing StorageManager::Estimate(). There is no need for the change in chrome JS calls to qms.getUsageForPrincipal().

Treeherder result of this WIP patch looks fine, btw:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=92375597216f82e9ca3e6e9b7bf72906be0fe130

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

set to WONTFIX since q-dom scheduler was tabled.