Closed Bug 1300822 Opened 3 years ago Closed 3 years ago
Tab and refresh Tab from shared-head .js possibly resolve sooner than expected
58 bytes, text/x-review-board-request
Waiting for "load" event on a <xul:browser> from a privileged code can be misleading as you also receive "load" event for iframes loading in the top document loaded in the frame. So that addTab helper which is used almost everywhere can sometime resolve sooner than expected anytime a document has iframes. I saw that exact scenario happen in gcli helper listenOnce used in browser_cmd_csscoverage_startstop.js: https://dxr.mozilla.org/mozilla-central/source/devtools/client/commandline/test/browser_cmd_csscoverage_startstop.js?q=file%3Abrowser_cmd_csscoverage_startstop.js&redirect_type=single#49 // Page 2 is a frame in page 1. JS in the page navigates to page 3. yield helpers.listenOnce(options.browser, "load", true); This line was always resolving on the "Page 2 frame from page 1" load event. And as addTab use the exact same approach, I'm expecting it to have the same issue.
Yikes, I think I've been bitten by this before as well... let's fix it for sure! :)
Comment on attachment 8788528 [details] Bug 1300822 - Fix tab load event wait in devtools test helpers. https://reviewboard.mozilla.org/r/76992/#review75126 It looks like `BrowserTestUtils.browserLoaded` handles the top-frame-only case already. Can we use this helper so that we stay in sync with browser team? (BrowserTestUtils is auto-loaded by the mochitest browser harness.) : https://dxr.mozilla.org/mozilla-central/rev/8c9c4e816e86f903c1d820f3f29715dc070a5a4a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm#183
Attachment #8788528 - Flags: review?(jryans) → review-
Ok, I have a new patch using BrowserTestUtils.browserLoaded. I also converted much more code to use that.
Attachment #8788528 - Attachment is obsolete: true
Tom, Ryan is on PTO for quite a while, so I'm lurking for alternative reviewers. Hopefully you won't mind this r?. Sorry about this depressive patch. It is quite significant but I'm only converting things to stop listening for "load" event manually on <xul:browser> element. Instead it uses BrowserTestUtils.browserLoaded which ensure ignoring the load event for inner iframes. It should kill intermittents in any test where we didn't knew about that.
Comment on attachment 8790347 [details] Bug 1300822 - Fix tab load event wait in devtools test helpers. https://reviewboard.mozilla.org/r/78228/#review76958 Thank you. I read through this and everything looked good. This is a great cleanup and fix.
Attachment #8790347 - Flags: review?(ttromey) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/272f009cfe07 Fix tab load event wait in devtools test helpers. r=tromey
You need to log in before you can comment on or make changes to this bug.