Closed
Bug 1167579
Opened 9 years ago
Closed 9 years ago
Intermittent browser_broadcast.js,browser_sessionStorage.js | Uncaught exception - at chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_broadcast.js:60 - TypeError: storage is undefined
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
Tracking | Status | |
---|---|---|
firefox39 | --- | unaffected |
firefox40 | --- | fixed |
firefox41 | --- | fixed |
firefox-esr31 | --- | unaffected |
firefox-esr38 | --- | unaffected |
People
(Reporter: RyanVM, Assigned: ttaubert)
References
Details
(Keywords: intermittent-failure)
Attachments
(2 files, 1 obsolete file)
1.50 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
1.62 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1147396 +++ 03:48:29 INFO - ************************* 03:48:29 INFO - A coding exception was thrown and uncaught in a Task. 03:48:29 INFO - Full message: TypeError: storage is undefined 03:48:29 INFO - Full stack: flush_on_duplicate@chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_broadcast.js:60:3 03:48:29 INFO - Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:867:23 03:48:29 INFO - this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:746:7 03:48:29 INFO - this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:37 03:48:29 INFO - Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:5 03:48:29 INFO - this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:709:7 03:48:29 INFO - this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:671:7 03:48:29 INFO - listener@chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:197:7 03:48:29 INFO - ************************* 03:48:29 INFO - 1644 INFO checking window state 03:48:29 INFO - 1645 INFO Entering test flush_on_tabclose 03:48:29 INFO - 1646 INFO TEST-PASS | browser/components/sessionstore/test/browser_broadcast.js | sessionStorage data has been flushed on TabClose 03:48:29 INFO - 1647 INFO Leaving test flush_on_tabclose 03:48:29 INFO - 1648 INFO Entering test flush_on_quit_requested 03:48:29 INFO - 1649 INFO TEST-PASS | browser/components/sessionstore/test/browser_broadcast.js | sessionStorage data has been flushed when a quit is requested 03:48:29 INFO - 1650 INFO Leaving test flush_on_quit_requested 03:48:29 INFO - 1651 INFO Entering test flush_on_duplicate 03:48:29 INFO - 1652 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_broadcast.js | Uncaught exception - at chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_broadcast.js:60 - TypeError: storage is undefined 03:48:29 INFO - Stack trace: 03:48:29 INFO - flush_on_duplicate@chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_broadcast.js:60:3 03:48:29 INFO - Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:867:23 03:48:29 INFO - this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:746:7 03:48:29 INFO - this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:37 03:48:29 INFO - Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:5 03:48:29 INFO - this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:709:7 03:48:29 INFO - this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:671:7 03:48:29 INFO - listener@chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:197:7 03:48:29 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:741:9 03:48:29 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:664:7 03:48:29 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:745:59 03:48:29 INFO - 1653 INFO Leaving test flush_on_duplicate 03:48:29 INFO - 1654 INFO Entering test flush_on_windowclose
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Comment 4•9 years ago
|
||
Tim, maybe related to your recent sessionstore test changes?
Flags: needinfo?(ttaubert)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 7•9 years ago
|
||
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Flags: needinfo?(ttaubert)
Attachment #8609700 -
Flags: review?(wmccloskey)
Assignee | ||
Comment 8•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d08155cf619a
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 10•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a73f9e6fef8d
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•9 years ago
|
Iteration: --- → 41.1 - May 25
Flags: qe-verify?
Flags: firefox-backlog+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Updated•9 years ago
|
Points: --- → 2
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment on attachment 8609700 [details] [diff] [review] 0001-Bug-1167579-Fix-intermittent-browser_broadcast.js-fa.patch Review of attachment 8609700 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/sessionstore/test/browser_sessionStorage.js @@ +29,5 @@ > is(storage["http://mochi.test:8888"].test, OUTER_VALUE, > "sessionStorage data for mochi.test has been serialized correctly"); > > // Ensure that modifying sessionStore values works for the inner frame only. > + yield modifySessionStorage(browser, {test: "modified1"}, 0); It's very easy to miss the meaning of the 0 here. Could you maybe pass an options parameter with {frameIndex: 0}? ::: browser/components/sessionstore/test/head.js @@ +656,5 @@ > + resolve(); > + } > + }, true); > + > + for (let key of [...keys]) { Can't you just say |for (let key of keys)|?
Attachment #8609700 -
Flags: review?(wmccloskey) → review+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 30•9 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #26) > > // Ensure that modifying sessionStore values works for the inner frame only. > > + yield modifySessionStorage(browser, {test: "modified1"}, 0); > > It's very easy to miss the meaning of the 0 here. Could you maybe pass an > options parameter with {frameIndex: 0}? Sure, will do. > > + for (let key of [...keys]) { > > Can't you just say |for (let key of keys)|? Did this to account for sync changes that might modify the set while we're iterating but as MozStorageChanged events are always dispatched off a runnable I guess we don't need this.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
https://hg.mozilla.org/mozilla-central/rev/994744c57cbc
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Reporter | ||
Updated•9 years ago
|
status-firefox39:
--- → unaffected
status-firefox40:
--- → unaffected
status-firefox-esr31:
--- → unaffected
status-firefox-esr38:
--- → unaffected
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Updated•9 years ago
|
Reporter | ||
Comment 38•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/6989af9a929f
Flags: in-testsuite+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Updated•9 years ago
|
Summary: Intermittent browser_broadcast.js | Uncaught exception - at chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_broadcast.js:60 - TypeError: storage is undefined | Found an unexpected tab at the end of test run → Intermittent browser_broadcast.js,browser_sessionStorage.js | Uncaught exception - at chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_broadcast.js:60 - TypeError: storage is undefined
Reporter | ||
Updated•9 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 41 → ---
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 50•9 years ago
|
||
Can reproduce locally, will have a fix soon.
Flags: qe-verify? → qe-verify-
OS: Mac OS X → All
Hardware: x86_64 → All
Assignee | ||
Comment 51•9 years ago
|
||
This is a fix for only browser_broadcast.js. We currently duplicate the tab and then call getTabState() without flushing. I'm actually surprised this doesn't fail more frequently. The easy fix here is to remove the superfluous check and rely only on checking the closedTabData. If we properly flushed when duplicating the tab the data will be in there.
Attachment #8609700 -
Attachment is obsolete: true
Attachment #8612615 -
Flags: review?(wmccloskey)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 60•9 years ago
|
||
browser_sessionStorage.js seems to sometimes fail because Date.now() isn't monotonous, that occurs esp. often on Win32 machines. The closedAt property of the closed tab might be "wrong" and then the closed tab is put into the wrong spot and we check the wrong closedTabData.
Attachment #8612979 -
Flags: review?(wmccloskey)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Attachment #8612615 -
Flags: review?(wmccloskey) → review+
Comment on attachment 8612979 [details] [diff] [review] 0002-Bug-1167579-Fix-intermittent-browser_sessionStorage..patch Review of attachment 8612979 [details] [diff] [review]: ----------------------------------------------------------------- I guess this is fine, but it kinda suggests we might be better off using a global counter rather than Date.now(). ::: browser/components/sessionstore/test/browser_sessionStorage.js @@ +164,5 @@ > [{state: {storage}}] = JSON.parse(ss.getClosedTabData(window)); > ok(!storage, "sessionStorage data has *not* been saved"); > > + // Remove all closed tabs before continuing with the next test. > + // As Date.now() isn't monotonous we might sometimes check I think the word you want is "monotonic". Monotonous basically means boring :-).
Attachment #8612979 -
Flags: review?(wmccloskey) → review+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 78•9 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #70) > I guess this is fine, but it kinda suggests we might be better off using a > global counter rather than Date.now(). Yeah we would be. closedAt was introduced to give UI or add-ons a chance to provide more information about closed tabs, it wasn't really meant to provide a sort order. Also, it should mostly not be a problem in everyday use - probably more a test problem. > > + // Remove all closed tabs before continuing with the next test. > > + // As Date.now() isn't monotonous we might sometimes check > > I think the word you want is "monotonic". Monotonous basically means boring > :-). Hah, oops :)
Comment 79•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/e3af4ebf8ffb https://hg.mozilla.org/integration/fx-team/rev/cf4d1a0497a6
Assignee | ||
Comment 80•9 years ago
|
||
(In reply to Tim Taubert [:ttaubert] from comment #78) > (In reply to Bill McCloskey (:billm) from comment #70) > > > + // Remove all closed tabs before continuing with the next test. > > > + // As Date.now() isn't monotonous we might sometimes check > > > > I think the word you want is "monotonic". Monotonous basically means boring > > :-). > > Hah, oops :) Forgot to fix the commit message. Oh well.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
https://hg.mozilla.org/mozilla-central/rev/e3af4ebf8ffb https://hg.mozilla.org/mozilla-central/rev/cf4d1a0497a6
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Comment 90•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/fd63e1ef8cbd https://hg.mozilla.org/releases/mozilla-aurora/rev/f25907a388cc
You need to log in
before you can comment on or make changes to this bug.
Description
•