1693846 - Only last call of multiple concurrent calls to StorageActor#listStores resolves

Status: RESOLVED FIXED

(DevTools :: Storage Inspector, defect, P3)

Description

[Nicolas Chevobbe [:nchevobbe]]

This is something I noticed when doing the following change in the resource watcher:

diff --git a/devtools/shared/resources/resource-watcher.js b/devtools/shared/resources/resource-watcher.js
--- a/devtools/shared/resources/resource-watcher.js
+++ b/devtools/shared/resources/resource-watcher.js
@@ -326,9 +326,13 @@ class ResourceWatcher {
     );
 
     if (isTargetSwitching) {
-      for (const resourceType of resources) {
-        await this._startListening(resourceType, { bypassListenerCount: true });
-      }
+      await Promise.all(
+        resources.map(resourceType =>
+          this._startListening(resourceType, {
+            bypassListenerCount: true,
+          })
+        )
+      );
     }
   }

All the storage inspector that were doing a target switch (e.g. devtools/client/storage/test/browser_storage_cache_navigation.js) where timing out.

After some investigation, I found that the culprit was this call to listStores from the storage resources legacy listeners: devtools/shared/resources/legacy-listeners/storage-utils.js#42
So here, with the change I mentioned earlier, multiple calls to the same method are done around the same time.
Then, on the server, I found that all the calls but the last one were waiting forever the result of the indexedDb actor preListStores method.

In devtools/server/actors/storage.js#2548 , we're resetting the this.hostVsStores property, and since it is used in a few methods, we must be missing something.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1693846 - [devtools] Fix Storage actor listStores so it can handles multiple concurrent calls. r=ladybenko.

If there was multiple concurrent calls to listStores, only the last one made would
resolve, the other one would be stuck indefinitely.
This patch fixes the issue by caching the call to indexedDb actor preListStores.
A test is added to make sure we don't regress this.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/54d89870678f
[devtools] Fix Storage actor listStores so it can handles multiple concurrent calls. r=ladybenko.

Comment 3

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/54d89870678f