Closed Bug 1249447 Opened 8 years ago Closed 8 years ago

Intermittent browser_storage_dynamic_windows.js | A promise chain failed to handle a rejection: - at resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/storage.js:254 - TypeError: this.storageActor is null

Categories

(DevTools :: Storage Inspector, defect)

defect
Not set
normal

Tracking

(firefox47 fixed, firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: hiro, Assigned: ochameau)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 1 obsolete file)

Alexandre, Ryan, any idea what changed to cause this?  It's very frequent.
Flags: needinfo?(poirot.alex)
Flags: needinfo?(jryans)
I imagine that's bug 1248929.
Flags: needinfo?(poirot.alex)
Flags: needinfo?(jryans)
This is currently the #8 overall top orange. Can we please prioritize investigating and fixing it?
Flags: needinfo?(poirot.alex)
I have no idea how to apply Mike's bug 1248929 comment 1.
But I know how to first cleanup these tests by doing same things I did for devtools/client/storage/tests ones.

Hopefully, with this more sane way of waiting for indexed db creation, we might be correctly waiting for window ready...
Or we shouldn't inline an explicit wait in the test script for document to be fully set before proceeding with the test.
I'm not sure we should tweak the actor itself.
Ok. That was not enough, let's see if waiting for explicit load/unload fixes this.
Attachment #8724655 - Flags: review?(mratcliffe)
Assignee: nobody → poirot.alex
Comment on attachment 8724719 [details] [diff] [review]
Wait for document load/unload in storage tests.

It looks like explicit wait for load and unload reduced or killed this intermittent?!
Attachment #8724719 - Flags: review?(mratcliffe)
Comment on attachment 8724655 [details] [diff] [review]
Prevent using CPOW and wait for indexedDB setup/clear with tasks.

Review of attachment 8724655 [details] [diff] [review]:
-----------------------------------------------------------------

Nice... thanks for looking into this.
Attachment #8724655 - Flags: review?(mratcliffe) → review+
Comment on attachment 8724719 [details] [diff] [review]
Wait for document load/unload in storage tests.

Review of attachment 8724719 [details] [diff] [review]:
-----------------------------------------------------------------

I prefer this approach over creating a workaround in the actor.
Attachment #8724719 - Flags: review?(mratcliffe) → review+
Hum... I thought it was failing on linux opt, but it does only on debug.
So I'm not sure it is fixed.
Especially now that I finally got results from windows,
and it is "interesting" to see that it looks non-intermittent with my patches \o/
Last try push highlights these two patches are not enough to get rid of this intermittent...
Ok. I think I got it this time.
We don't need to wait for document load/unload.
Actually we were not waiting for all storage types creation/destruction.
Locally, there seem to be only one stores-update event, each time with all storage types at once.
But I imagine on try, with slow slaves, it is split in multiple events
and we end up not correctly waiting.

onWindowReady ends up emitting a stores-update event, we should be able to wait correctly!
Attachment #8731277 - Flags: review?(mratcliffe)
Attachment #8724719 - Attachment is obsolete: true
Comment on attachment 8731277 [details] [diff] [review]
Wait correctly for all storage creation/deletion in browser_storage_dynamic_windows.js

Review of attachment 8731277 [details] [diff] [review]:
-----------------------------------------------------------------

These changes look fine but they don't match your comments... assuming that you attached the correct patch then r+.
Attachment #8731277 - Flags: review?(mratcliffe) → review+
Actually it matches what I have in mind:
  Wait correctly for all storage creation/deletion in browser_storage_dynamic_windows.js

By adding these Cache and indexedDB entries, we ensure waiting for *all* stores-update.
The first hunk is for when we creating an iframe, the second one is for when we remove the iframe.
These fixes ensure that we wait for storage creation when we add the iframe, by waiting for all stores-update.
Same thing when we remove the iframe, we wait for all storages to be reseted by waiting for all stores-delete events.

Would you phrase the commit message differently?
Try seems to say it is being fixed. I'll try to push these patches once fx-team reopens.
https://hg.mozilla.org/integration/fx-team/rev/95656a398e8f7754d358f1f0250a2e8069f5db47
Bug 1249447 - Prevent using CPOW and wait for indexedDB setup/clear with tasks. r=mratcliffe

https://hg.mozilla.org/integration/fx-team/rev/c35895f48511d4ed74a9935415c5a786546fe982
Bug 1249447 - Wait correctly for all storage creation/deletion in browser_storage_dynamic_windows.js. r=mratcliffe
https://hg.mozilla.org/mozilla-central/rev/95656a398e8f
https://hg.mozilla.org/mozilla-central/rev/c35895f48511
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Whiteboard: [checkin-needed-aurora]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.