Closed Bug 1416153 Opened 7 years ago Closed 7 years ago

The tab progress listener or the target browser can be removed inside onLocationChange, after bug 1193394

Categories

(Firefox :: General, defect)

Product:
Firefox
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(3 files, 2 obsolete files)

Part 1: Wait for the next event tick before resolving promise in BrowserTestUtils.waitForNewTab.
(deleted), patch
Gijs
: review+
Details | Diff | Splinter Review
Part 0: Fix tests that uses BrowserTestUtils.waitForNewTab to perform the registration of the next event handler instantly inside the new tab event handler. r=Gijs
(deleted), patch
Gijs
: review+
Details | Diff | Splinter Review
Link to GitHub pull-request: https://github.com/mozilla/normandy/pull/1148
(deleted), text/x-github-pull-request
Details
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc35b181a4a55c0bd3cca8a19346ff224545a649&selectedJob=143221687 > FAIL | browser/base/content/test/general/browser_bug575561.js | uncaught exception > TypeError: this.mBrowser is undefined at onLocationChange@chrome://browser/content/tabbrowser.xml:974:52 https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/browser/base/content/tabbrowser.xml#965-974 > if (!this.mBlank) { > this._callProgressListeners("onLocationChange", > [aWebProgress, aRequest, aLocation, > aFlags]); > } > > if (topLevel) { > this.mBrowser.lastURI = aLocation; > this.mBrowser.lastLocationChange = Date.now(); > } It expects `this.mBrowser` to be valid there, but it can be undefined after destroy() https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/browser/base/content/tabbrowser.xml#573-577 > destroy() { > delete this.mTab; > delete this.mBrowser; > delete this.mTabBrowser; > }, So, it should check the validness before accessing `this.mBrowser`
The error for this.mBrowser seems to be happening in several testcases.
looks like the issue is from a bit different place than _callProgressListeners. it's destroyed while doing `this.mBrowser.lastURI = aLocation;` there... :/
Summary: The tab progress listener or the target browser can be removed before returning from onLocationChange handlers, after bug 1193394 → The tab progress listener or the target browser can be removed inside onLocationChange, after bug 1193394
here's what I observed: receiveMessage @ RemoteWebProgress.jsm:270:7 +- _callProgressListeners @ RemoteWebProgress.jsm:179:11 +- onLocationChange @ tabbrowser.xml:1017:17 +- <performs `this.mBrowser.lastURI = aLocation`> | <invokes XBL setter> | +- testLink/promise< @ browser_bug575561.js:68:14 | +- removeTab @ BrowserTestUtils.jsm:976:7 | +- removeTab @ tabbrowser.xml:3162:18 | +- _beginRemoveTab @ tabbrowser.xml:3348:15 | +- destroy: destroy @ tabbrowser.xml:575:26 | +- <performs `delete this.mBrowser`> | +- <performs `this.mBrowser.lastLocationChange = Date.now()`> <fails because this.mBrowser is undefined> so, looks like XBL setter can execute microtask checkpoint.
The flow is like following. the testcase uses `BrowserTestUtils.waitForNewTab`, that returns a promise that gets resolved in `onLocationChange` event, and the resolution handler removes the tab. https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/browser/base/content/test/general/browser_bug575561.js#66-69 > promise = BrowserTestUtils.waitForNewTab(gBrowser).then(tab => { > let loaded = tab.linkedBrowser.documentURI.spec; > return BrowserTestUtils.removeTab(tab).then(() => loaded); > }); https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm#422 > waitForNewTab(tabbrowser, url, waitForLoad = false, waitForAnyTab = false) { >... > return new Promise((resolve, reject) => { > tabbrowser.tabContainer.addEventListener("TabOpen", function tabOpenListener(openEvent) { >... > let progressListener = { > onLocationChange(aBrowser) { >... > resolve(result); > }, > }; > tabbrowser.addTabsProgressListener(progressListener); > }); > }); > }, then, the tab's progress listener invokes `onLocationChange` event, and after that it invokes XBL setter, that executes microtask checkpoint. https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/browser/base/content/tabbrowser.xml#965-974 > <method name="mTabProgressListener"> > ... > <body> > <![CDATA[ > ... > return ({ > ... > mBrowser: aBrowser, > ... > destroy() { > delete this.mTab; > delete this.mBrowser; > delete this.mTabBrowser; > }, > ... > onLocationChange(aWebProgress, aRequest, aLocation, > aFlags) { > if (!this.mBlank) { > this._callProgressListeners("onLocationChange", > [aWebProgress, aRequest, aLocation, > aFlags]); > } > > if (topLevel) { > this.mBrowser.lastURI = aLocation; > this.mBrowser.lastLocationChange = Date.now(); > } > }, > }); > ]]> > </body> > </method> So, the promise gets resolved in `this._callProgressListeners("onLocationChange",` line above, and the resolution handler gets executed inside `this.mBrowser.lastURI = aLocation;` line above. receiveMessage @ RemoteWebProgress.jsm:270:7 +- _callProgressListeners @ RemoteWebProgress.jsm:179:11 +- onLocationChange @ tabbrowser.xml:1017:17 +- <performs `this._callProgressListeners("onLocationChange", ...)`> | +- _callProgressListeners @ tabbrowser.xml:579:12 | +- _callProgressListeners @ tabbrowser.xml:465 | +- callListeners @ tabbrowser.xml:479 | +- onLocationChange @ BrowserTestUtils.jsm:410 | +- <resolves the promise and enqueues the microtask> | +- <performs `this.mBrowser.lastURI = aLocation`> | <invokes XBL setter> | +- <executes microtask checkpoint, see below> | +- <microtask for promise resolution gets executed> | +- testLink/promise< @ browser_bug575561.js:68:14 | +- removeTab @ BrowserTestUtils.jsm:976:7 | +- removeTab @ tabbrowser.xml:3162:18 | +- _beginRemoveTab @ tabbrowser.xml:3348:15 | +- destroy: destroy @ tabbrowser.xml:575:26 | +- <performs `delete this.mBrowser`> | +- <performs `this.mBrowser.lastLocationChange = Date.now()`> <fails because this.mBrowser is undefined> this is the native call stack for setting XBL property to PerformMicroTaskCheckPoint. SetExistingProperty +- js::CallSetter +- js::InternalCallOrConstruct +- FieldSetter +- FieldSetterImpl +- InstallXBLField +- nsXBLProtoImplField::InstallField +- nsAutoMicroTask::~nsAutoMicroTask +- CycleCollectedJSContext::LeaveMicroTask +- CycleCollectedJSContext::PerformMicroTaskCheckPoint I was thinking that microtasks queued from a certain subtree is executed when leaving the subtree, but apparently everthing queued before the checkpoint is executed, so, outside the subtree.
now I'm wondering what the best solution for this is... the following will surely fix this specific case, but I think it's not a good idea to introduce such kind of check into everywhere. browser/base/content/tabbrowser.xml > if (topLevel) { > + // The target tab can be destroyed at any point. > + if (!this.mBrowser) { > + return; > + } > this.mBrowser.lastURI = aLocation; > + if (!this.mBrowser) { > + return; > + } > this.mBrowser.lastLocationChange = Date.now(); > }
before fixing the issue here, I'd like to make sure the above understanding is correct. bevis, can you confirm the comment #4 is expected behavior?
Flags: needinfo?(btseng)
(In reply to Tooru Fujisawa [:arai] from comment #5) > now I'm wondering what the best solution for this is... > > the following will surely fix this specific case, but I think it's not a > good idea to introduce such kind of check into everywhere. > > browser/base/content/tabbrowser.xml > > if (topLevel) { > > + // The target tab can be destroyed at any point. > > + if (!this.mBrowser) { > > + return; > > + } > > this.mBrowser.lastURI = aLocation; > > + if (!this.mBrowser) { > > + return; > > + } > > this.mBrowser.lastLocationChange = Date.now(); > > } for this case, just stopping removing tab in the event handler for new tab (in testcase) might be smarter...
It looks strange to perform a microtask checkpoint at XBL setter while we are not at the return of an outermost JS callback. I'll debug more to figure out what's going on here. Keep this NI on me.
After further investigation, I found that we didn't add nsAutoMicroTask in nsXPCWrappedJSClass::CallMethod() while nsAutoMicroTask has already been used in nsXBLProtoImplField::InstallField() and nsXBLProtoImplAnonymousMethod::Execute() in current m-c which causes the resolved callback at https://searchfox.org/mozilla-central/source/browser/base/content/tabbrowser.xml#966 be performed unexpectedly at https://searchfox.org/mozilla-central/source/browser/base/content/tabbrowser.xml#972 Olli, is adding: > nsAutoMicroTask mt; at the following line right approach to go? https://searchfox.org/mozilla-central/rev/fe75164c55321e011d9d13b6d05c1e00d0a640be/js/xpconnect/src/XPCWrappedJSClass.cpp#1097 Is there any reason why we didn't do this in current m-c? Test browser_bug575561.js is PASSED if we did this, btw. I'll run more tests on try to see what's the difference if this change is included.
Flags: needinfo?(btseng) → needinfo?(bugs)
(In reply to Bevis Tseng[:bevis][:btseng] from comment #9) > Olli, is adding: > > nsAutoMicroTask mt; > at the following line right approach to go? > https://searchfox.org/mozilla-central/rev/ > fe75164c55321e011d9d13b6d05c1e00d0a640be/js/xpconnect/src/XPCWrappedJSClass. > cpp#1097 > Is there any reason why we didn't do this in current m-c? Unfortunately, things became worst on try after adding it, especially in xpcshell test and wpt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbdf13f489366a287f1c0a7f3b35dd6b02b99aea compared to the one without this change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=afeeb22cd302ddb089ffa008e82a754eca570444 Microtask is defined in HTML spec only for web contents. The inconsistency for the use of Promise(Microtask) in the scripts in web content and in browser internal could be confusing. :(
My aim has been to use the microtasks in DOM-y cases, which does include XBL. If all the xpconnect would use it too, then microtask callbacks might start to run at rather bad times when some very internal JS-implemented xpcom components run.
Flags: needinfo?(bugs)
(In reply to Olli Pettay [:smaug] from comment #11) > My aim has been to use the microtasks in DOM-y cases, which does include XBL. > If all the xpconnect would use it too, then microtask callbacks might start > to run at rather bad times when some very internal JS-implemented xpcom > components run. In this bug, we have xbl inside xpconnect which cause the problem we saw in comment 4, do you think this behavior shall be expected if bug 1193394 is landed and some change is needed in tabbrowser.xml to meet this change?
Flags: needinfo?(bugs)
Sounds like the promise comment 4 talks about should be resolved later, probably asynchronously. https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm#422 looks buggy to me.
Flags: needinfo?(bugs)
so, the current behavior around microtask handling is expected/correct, and that should be fixed on testcase's side, right?
(In reply to Tooru Fujisawa [:arai] from comment #14) > so, the current behavior around microtask handling is expected/correct, and > that should be fixed on testcase's side, right? sorry, by "current", I meant this one https://treeherder.mozilla.org/#/jobs?repo=try&revision=afeeb22cd302ddb089ffa008e82a754eca570444
Not quite sure I understand the question. BrowserTestUtils.jsm seems to expect the current wrong Promise scheduling. (microtask handling is ok, but Promises don't use it)
(In reply to Tooru Fujisawa [:arai] from comment #15) > (In reply to Tooru Fujisawa [:arai] from comment #14) > > so, the current behavior around microtask handling is expected/correct, and > > that should be fixed on testcase's side, right? > > sorry, by "current", I meant this one > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=afeeb22cd302ddb089ffa008e82a754eca570444 After further reading of tabbrowser.xml, I just realized that the object returned by mTabProgressListener is still an xpc callback of nsIWebProgressListener which belongs to the part of gecko internal instead of the part of the xbl script and a resolved promise in xpc was consumed unexpectedly by the microtask checkpoint at the return of accessing xbl field. So, with the fix of bug 1193394 and keep the xpc away from nsAutoMicroTask per comment 11, to prevent the promises resolved/rejected in xpc be consumed by xbl script, as suggested in comment 13, either the resolve() call or the xbl scripts touched by xpc callback shall be done asynchronously to fix this problem.
this solves some problem, but introduces other problem. now investigating new failure.
3 new failure with the change, that uses BrowserTestUtils.waitForNewTab. browser/base/content/test/siteIdentity/browser_insecureLoginForms.js toolkit/components/viewsource/test/browser/browser_srcdoc.js toolkit/mozapps/extensions/test/xpinstall/browser_bug638292.js
In browser/base/content/test/siteIdentity/browser_insecureLoginForms.js, the test expects the promise `loaded` gets resolved earlier than the next tick, to observe InsecureLoginFormsStateChange events. https://dxr.mozilla.org/mozilla-central/rev/d4753dc14b2ab9c42123b6d60a68106df40f45cd/browser/base/content/test/siteIdentity/browser_insecureLoginForms.js#141-147 > let loaded = BrowserTestUtils.waitForNewTab(gBrowser, newTabURL); > await ContentTask.spawn(browser, {}, function() { > content.document.getElementById("link").click(); > }); > let tab = await loaded; > browser = tab.linkedBrowser; > await waitForInsecureLoginFormsStateChange(browser, 2); So, in this case BrowserTestUtils.waitForNewTab shouldn't wait for the next event tick. To workaround this, we can add extra parameter to BrowserTestUtils.waitForNewTab that controls the promise resolution timing. Then, there are already 2 parameters, and adding one more parameter will make it complicated. So I'd like to make the parameter an object with properties for each option.
This patch fixes ~20 testcases. I'll ask review after checking if this works on non-patched tree. https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9611170dbbd2fc098e3015b761382a844dd8222
(In reply to Tooru Fujisawa [:arai] from comment #20) > So, in this case BrowserTestUtils.waitForNewTab shouldn't wait for the next > event tick. To workaround this, we can add extra parameter to > BrowserTestUtils.waitForNewTab that controls the promise resolution timing. > Then, there are already 2 parameters, and adding one more parameter will > make it complicated. So I'd like to make the parameter an object with > properties for each option. Can't we just fix the test so it works irrespective of the ordering? Adding parameters to BTU methods to control Promise resolution timing doesn't seem like the right fix to me. Ideally, testcases shouldn't depend on the timing being one or the other (I understand that they currently do, but I'd much rather fix the individual testcases than cluttering up BTU with parameters to deal with those individual testcases).
Flags: needinfo?(arai.unmht)
(In reply to :Gijs (away until Nov 20) from comment #22) > (In reply to Tooru Fujisawa [:arai] from comment #20) > > So, in this case BrowserTestUtils.waitForNewTab shouldn't wait for the next > > event tick. To workaround this, we can add extra parameter to > > BrowserTestUtils.waitForNewTab that controls the promise resolution timing. > > Then, there are already 2 parameters, and adding one more parameter will > > make it complicated. So I'd like to make the parameter an object with > > properties for each option. > > Can't we just fix the test so it works irrespective of the ordering? Adding > parameters to BTU methods to control Promise resolution timing doesn't seem > like the right fix to me. Ideally, testcases shouldn't depend on the timing > being one or the other (I understand that they currently do, but I'd much > rather fix the individual testcases than cluttering up BTU with parameters > to deal with those individual testcases). what the testcase doing is, wait for new tab, and wait for events happening inside it in very early stage. with the updated way (wait for the next event tick in BrowserTestUtils.waitForNewTab), looks like the testcase misses those events. I'm not sure if there's other way than stop using BrowserTestUtils.waitForNewTab there... I'll look into it some more. (maybe, "wait for the next event tick there" is wrong solution at the beginning...?)
(In reply to Tooru Fujisawa [:arai] from comment #23) > (In reply to :Gijs (away until Nov 20) from comment #22) > > (In reply to Tooru Fujisawa [:arai] from comment #20) > > > So, in this case BrowserTestUtils.waitForNewTab shouldn't wait for the next > > > event tick. To workaround this, we can add extra parameter to > > > BrowserTestUtils.waitForNewTab that controls the promise resolution timing. > > > Then, there are already 2 parameters, and adding one more parameter will > > > make it complicated. So I'd like to make the parameter an object with > > > properties for each option. > > > > Can't we just fix the test so it works irrespective of the ordering? Adding > > parameters to BTU methods to control Promise resolution timing doesn't seem > > like the right fix to me. Ideally, testcases shouldn't depend on the timing > > being one or the other (I understand that they currently do, but I'd much > > rather fix the individual testcases than cluttering up BTU with parameters > > to deal with those individual testcases). > > what the testcase doing is, wait for new tab, and wait for events happening > inside it in very early stage. > with the updated way (wait for the next event tick in > BrowserTestUtils.waitForNewTab), > looks like the testcase misses those events. > I'm not sure if there's other way than stop using > BrowserTestUtils.waitForNewTab there... > I'll look into it some more. > (maybe, "wait for the next event tick there" is wrong solution at the > beginning...?) Yeah, so, I would solve like this: let eventsFired; let loaded = new Promise(r => { gBrowser.tabContainer.addEventListener("TabOpen", e => { browser = e.target.linkedBrowser; eventsFired = waitForInsecureLoginFormsStateChange(browser, 2); resolve(); }, {once: true}); }); await loaded; await eventsFired; TBH, I don't understand why that promise is called 'loaded', because BTU.waitForNewTab() doesn't wait for load events by default, but this is what the current test does. Could maybe name the promise "tabOpened" instead. If this is insufficient, :johannh can probably provide more context / background for this test.
thanks. I'll try that
Flags: needinfo?(arai.unmht)
Depends on: 1418210
[browser/base/content/test/referrer/head.js] [browser/base/content/test/tabs/browser_opened_file_tab_navigated_to_web.js] [toolkit/mozapps/extensions/test/xpinstall/browser_bug638292.js] Changed to use `waitForLoad` parameter, instead of manually wait for load in inappropriate timing. [browser/base/content/test/siteIdentity/browser_insecureLoginForms.js] waitForInsecureLoginFormsStateChange needs to be called inside the "TabOpen" event handler, so stopped using BrowserTestUtils.waitForNewTab. [toolkit/components/viewsource/test/browser/browser_open_docgroup.js] [toolkit/components/viewsource/test/browser/browser_srcdoc.js] [toolkit/components/viewsource/test/browser/head.js] waitForSourceLoaded should be done in "TabOpen" or window's "load" event handler, so moved it to waitForViewSourceTabOrWindow in head.js, and refactored the following functions to call waitForViewSourceTabOrWindow directly or indirectly: * waitForViewSourceWindow * openViewSource * openViewPartialSource * openViewFrameSourceTab * openDocumentSelect
Attachment #8929721 - Attachment is obsolete: true
Attachment #8931217 - Flags: review?(gijskruitbosch+bugs)
And changed BrowserTestUtils.waitForNewTab to wait for the next event tick before resolving the promise.
Attachment #8931218 - Flags: review?(gijskruitbosch+bugs)
Attachment #8931218 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8931217 [details] [diff] [review] Part 0: Fix tests that uses BrowserTestUtils.waitForNewTab to perform the registration of the next event handler instantly inside the new tab event handler. Review of attachment 8931217 [details] [diff] [review]: ----------------------------------------------------------------- r=me assuming this passes on try. ::: browser/base/content/test/referrer/head.js @@ -153,5 @@ > * @resolves With the tab once it's loaded. > */ > function someTabLoaded(aWindow) { > - return BrowserTestUtils.waitForNewTab(gTestWindow.gBrowser).then((tab) => { > - return BrowserTestUtils.browserStopped(tab.linkedBrowser).then(() => tab); If this passes on try, I'm fine with it, but note you're swapping out 'stopped' with 'loaded', and they're not the same (see e.g. discussion on bug 980904).
Attachment #8931217 - Flags: review?(gijskruitbosch+bugs) → review+
Thank you for reviewing :) (In reply to :Gijs from comment #28) > Comment on attachment 8931217 [details] [diff] [review] > Part 0: Fix tests that uses BrowserTestUtils.waitForNewTab to perform the > registration of the next event handler instantly inside the new tab event > handler. > > Review of attachment 8931217 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me assuming this passes on try. > > ::: browser/base/content/test/referrer/head.js > @@ -153,5 @@ > > * @resolves With the tab once it's loaded. > > */ > > function someTabLoaded(aWindow) { > > - return BrowserTestUtils.waitForNewTab(gTestWindow.gBrowser).then((tab) => { > > - return BrowserTestUtils.browserStopped(tab.linkedBrowser).then(() => tab); > > If this passes on try, I'm fine with it, but note you're swapping out > 'stopped' with 'loaded', and they're not the same (see e.g. discussion on > bug 980904). yes, it passes try https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a21cdea9f7adc2093725a97fb29a46eaba8897e&group_state=expanded
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e261d018c0371b60877450b6d3d925eb6b54ac3 Bug 1416153 - Part 0: Fix tests that uses BrowserTestUtils.waitForNewTab to perform the registration of the next event handler instantly inside the new tab event handler. r=Gijs https://hg.mozilla.org/integration/mozilla-inbound/rev/1eff507bf0f47cf7cc2bdee4925252f92a477955 Bug 1416153 - Part 1: Wait for the next event tick before resolving promise in BrowserTestUtils.waitForNewTab. r=Gijs
overlooked that browser/extensions/shield-recipe-client/test/browser/browser_Heartbeat.js was not same as intermittent. I'll fix it.
Fixed browser/extensions/shield-recipe-client/test/browser/browser_Heartbeat.js to directly wait for TabOpen event, and call BrowserTestUtils.browserLoaded inside it, so it won't miss the event before adding event listener. it skips some steps (progress listener etc) in BrowserTestUtils.waitForNewTab, but it should be fine for this testcase. https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3b3b0866ab09e4b179d094534d938230b2eeea9&group_state=expanded&selectedJob=147285826
Attachment #8931217 - Attachment is obsolete: true
Attachment #8931546 - Flags: review?(gijskruitbosch+bugs)
Attachment #8931546 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/976f45726ed5 Part 0: Fix tests that uses BrowserTestUtils.waitForNewTab to perform the registration of the next event handler instantly inside the new tab event handler. r=Gijs https://hg.mozilla.org/integration/mozilla-inbound/rev/76288affab86 Part 1: Wait for the next event tick before resolving promise in BrowserTestUtils.waitForNewTab. r=Gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/976f45726ed5e5ca5e6961f5a8d6abeaa41a3637 Bug 1416153 - Part 0: Fix tests that uses BrowserTestUtils.waitForNewTab to perform the registration of the next event handler instantly inside the new tab event handler. r=Gijs https://hg.mozilla.org/integration/mozilla-inbound/rev/76288affab86b424af5be018b4e25c7e8254e522 Bug 1416153 - Part 1: Wait for the next event tick before resolving promise in BrowserTestUtils.waitForNewTab. r=Gijs
Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c58e9e70f971 followup: Fix browser_report_site_issue.js to wait for events in right order. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/c58e9e70f971a1a17384984dce52aec6de7ac3bb Bug 1416153 - followup: Fix browser_report_site_issue.js to wait for events in right order. r=me
(In reply to Tooru Fujisawa [:arai] from comment #38) > https://hg.mozilla.org/integration/mozilla-inbound/rev/ > c58e9e70f971a1a17384984dce52aec6de7ac3bb > Bug 1416153 - followup: Fix browser_report_site_issue.js to wait for events > in right order. r=me applied the same fix to browser_report_site_issue.js
Commits pushed to master at https://github.com/mozilla/normandy https://github.com/mozilla/normandy/commit/f63e4717d250a09a5eb1a26118f1b7e12d6a8021 Bug 1416153 - Part 0: Fix tests that uses BrowserTestUtils.waitForNewTab to perform the registration of the next event handler instantly inside the new tab event handler. r=Gijs https://github.com/mozilla/normandy/commit/a910bac51bb067b647f99bd369f31c2601a9675f Merge #1148 1148: Bug 1416153 - Part 0: Fix tests that uses BrowserTestUtils.waitForNew Tab to perform the registration of the next event handler instantly inside the new tab event handler. r=Gijs r=gijsk a=mythmon This is a backport from [Bug 1416153](https://bugzilla.mozilla.org/show_bug.cgi?id=1416153). It helps with race conditions in tests that we hit when trying to sync v80.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: