Closed
Bug 1426599
Opened 7 years ago
Closed 7 years ago
browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js fails after bug 1193394
Categories
(WebExtensions :: Untriaged, defect)
WebExtensions
Untriaged
Tracking
(firefox59 fixed)
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: bevis, Assigned: arai)
References
Details
Attachments
(1 file)
New BC failure found in today's try on all desktop platform:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe67e1680fd8958d759cf41662f2db8c152d76e3
INFO TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js | uncaught exception - TypeError: this.mBrowser is undefined at onLocationChange@chrome://browser/content/tabbrowser.xml:937:52
INFO TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_url_overrides_newtab.js | uncaught exception - TypeError: this.mBrowser is undefined at onLocationChange@chrome://browser/content/tabbrowser.xml:937:52
Priority: -- → P3
Updated•7 years ago
|
Component: General → WebExtensions: Untriaged
Priority: P3 → --
Product: Firefox → Toolkit
Updated•7 years ago
|
Assignee: nobody → bob.silverberg
Comment 1•7 years ago
|
||
The test in question was updated in mid-December to include two calls to BrowserTestUtils.waitForLocationChange() [1, 2], and the API code itself was also changed to add an onLocationChange listener [3]. It seems like one of these is causing the error in RemoteWebProgress.jsm, which is being passed up from tabbrowser.xml.
I'm not sure what's causing the error "this.mBrowser is undefined" {file: "chrome://browser/content/tabbrowser.xml" line: 937}.
I'm also not sure if this is something that needs to be fixed in the test, in the API code, or elsewhere.
[1] https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js#303
[2] https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js#424
[3] https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/browser/components/extensions/ext-url-overrides.js#31
Assignee: bob.silverberg → nobody
Assignee | ||
Comment 3•7 years ago
|
||
it's the similar issue to bug 1416153, removeTab is executed too early, after BrowserTestUtils.waitForLocationChange.
that is executed inside tabbrowser's onLocationChange event handler unexpectedly.
the test should wait for the next event tick before removing tab, after onLocationChange event handler.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•7 years ago
|
||
similar to bug 1416153, the test needs to wait for the next event tick before removing tab, after BrowserTestUtils.waitForLocationChange.
otherwise the remaining part of the test will be exected inside tabbrowser's onLocationChange handler (the same situation as bug 1416153), and the tab is removed unexpectedly.
in this case, I think it's better just wait inside the testcase itself with TestUtils.waitForTick, instead of modifying test utility functions, since this is specific to BrowserTestUtils.waitForLocationChange+BrowserTestUtils.removeTab combination, not BrowserTestUtils.waitForLocationChange itself.
Attachment #8940062 -
Flags: review?(gijskruitbosch+bugs)
Comment 5•7 years ago
|
||
Comment on attachment 8940062 [details] [diff] [review]
Wait for the next event tick after onLocationChange, in browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js.
Review of attachment 8940062 [details] [diff] [review]:
-----------------------------------------------------------------
rs=me, but re-reading the other bug, it's pretty surprising that promises can get resolved in the middle of the execution of an otherwise-synchronous JS method (because of XBL setters).
Can the same thing happen when using setAttribute, setting expando properties on a DOM node, or setting e.g. HTML5 dataset properties? If not, I guess this is yet another case where XBL removal will help us...
Attachment #8940062 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 6•7 years ago
|
||
Ni'ing bgrins because comment #5 (see also bug 1416153 esp. comments 4 through 16) seems like it'd be relevant to his interests (no actual questions, just wanting to make sure you've seen this).
Flags: needinfo?(bgrinstead)
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aaf2752a9ffe
Wait for the next event tick after onLocationChange, in browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js. r=Gijs
Assignee | ||
Comment 8•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/aaf2752a9ffea531c4093d8a3536e7cfd7241fa6
Bug 1426599 - Wait for the next event tick after onLocationChange, in browser/components/extensions/test/browser/browser_ext_url_overrides_newtab.js. r=Gijs
Comment 9•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 10•7 years ago
|
||
(In reply to :Gijs from comment #5)
> Can the same thing happen when using setAttribute, setting expando
> properties on a DOM node, or setting e.g. HTML5 dataset properties?
I think I'd quite like to know the answer to this question, actually. :-)
Flags: needinfo?(arai.unmht)
Assignee | ||
Comment 11•7 years ago
|
||
(In reply to :Gijs from comment #10)
> (In reply to :Gijs from comment #5)
> > Can the same thing happen when using setAttribute, setting expando
> > properties on a DOM node, or setting e.g. HTML5 dataset properties?
>
> I think I'd quite like to know the answer to this question, actually. :-)
afaik the issue happens only for XBL, and it doesn't happen for those standard things.
but I'd forward the question to bevis.
Flags: needinfo?(arai.unmht) → needinfo?(btseng)
Reporter | ||
Comment 12•7 years ago
|
||
(In reply to Tooru Fujisawa [:arai] from comment #11)
> (In reply to :Gijs from comment #10)
> > (In reply to :Gijs from comment #5)
> > > Can the same thing happen when using setAttribute, setting expando
> > > properties on a DOM node, or setting e.g. HTML5 dataset properties?
> >
> > I think I'd quite like to know the answer to this question, actually. :-)
>
> afaik the issue happens only for XBL, and it doesn't happen for those
> standard things.
> but I'd forward the question to bevis.
Yes, this issue only happens in XBL similar to what we found in bug 1416153 comment 17 that both xpc and xbl scripts are interleaved in tabbrowser.xml.
It shall not be possible for a HTML web content to define a xpc script/listener and interact with the xpc components directly.
Flags: needinfo?(btseng)
Comment 13•7 years ago
|
||
(In reply to :Gijs from comment #6)
> Ni'ing bgrins because comment #5 (see also bug 1416153 esp. comments 4
> through 16) seems like it'd be relevant to his interests (no actual
> questions, just wanting to make sure you've seen this).
Thanks for the heads up. Hopefully now that tabbrowser isn't a XBL binding anymore we'll see this sort of thing less.
Flags: needinfo?(bgrinstead)
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•