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)
DevTools
Storage Inspector
Tracking
(firefox47 fixed, firefox48 fixed)
RESOLVED
FIXED
Firefox 48
People
(Reporter: hiro, Assigned: ochameau)
References
Details
(Keywords: intermittent-failure)
Attachments
(2 files, 1 obsolete file)
29.13 KB,
patch
|
miker
:
review+
|
Details | Diff | Splinter Review |
1.72 KB,
patch
|
miker
:
review+
|
Details | Diff | Splinter Review |
Comment 2•8 years ago
|
||
Alexandre, Ryan, any idea what changed to cause this? It's very frequent.
Flags: needinfo?(poirot.alex)
Flags: needinfo?(jryans)
Assignee | ||
Comment 3•8 years ago
|
||
I imagine that's bug 1248929.
Flags: needinfo?(poirot.alex)
Flags: needinfo?(jryans)
Comment hidden (Intermittent Failures Robot) |
Comment 5•8 years ago
|
||
This is currently the #8 overall top orange. Can we please prioritize investigating and fixing it?
Flags: needinfo?(poirot.alex)
Assignee | ||
Comment 6•8 years ago
|
||
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.
Assignee | ||
Comment 7•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0addfacfdd2f
Assignee | ||
Comment 8•8 years ago
|
||
Ok. That was not enough, let's see if waiting for explicit load/unload fixes this.
Assignee | ||
Comment 9•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b274f412bf7
Assignee | ||
Comment 10•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=04341bdc6c8f
Flags: needinfo?(poirot.alex)
Assignee | ||
Updated•8 years ago
|
Attachment #8724655 -
Flags: review?(mratcliffe)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → poirot.alex
Assignee | ||
Comment 11•8 years ago
|
||
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+
Assignee | ||
Comment 14•8 years ago
|
||
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/
Assignee | ||
Comment 15•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=590589d37ca6
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 22•8 years ago
|
||
Last try push highlights these two patches are not enough to get rid of this intermittent...
Assignee | ||
Comment 23•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4abac3d55f0
Assignee | ||
Comment 24•8 years ago
|
||
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)
Assignee | ||
Updated•8 years ago
|
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+
Assignee | ||
Comment 26•8 years ago
|
||
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?
Assignee | ||
Comment 27•8 years ago
|
||
Try seems to say it is being fixed. I'll try to push these patches once fx-team reopens.
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 29•8 years ago
|
||
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
Comment 30•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/95656a398e8f https://hg.mozilla.org/mozilla-central/rev/c35895f48511
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment hidden (Intermittent Failures Robot) |
Updated•8 years ago
|
Whiteboard: [checkin-needed-aurora]
Comment 32•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/d6257152dcbf https://hg.mozilla.org/releases/mozilla-aurora/rev/425673829611
status-firefox47:
--- → fixed
Whiteboard: [checkin-needed-aurora]
Comment hidden (Intermittent Failures Robot) |
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•