1279320 - Move browser_bug495058.js to urlbar directory

Status: RESOLVED INACTIVE

(Firefox :: Address Bar, defect, P5)

Description

[Mike Conley (:mconley) (:⚙️)]

This was initially attempted in bug 1265055 but got backed out due to unexplained test failures. We should figure out what's going wrong here and then move it.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1279320 - Move browser_bug495058.js to urlbar directory, and fix issues with timing when it is run on its own.

Review commit: https://reviewboard.mozilla.org/r/99504/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/99504/

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 8819849 [details]
Bug 1279320 - Move browser_bug495058.js to urlbar directory, and fix issues with timing when it is run on its own.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/99504/diff/1-2/

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8819849 [details]
Bug 1279320 - Move browser_bug495058.js to urlbar directory, and fix issues with timing when it is run on its own.

https://reviewboard.mozilla.org/r/99504/#review100024

\o/

Comment 4

[Mark Banner (:standard8)]

Unfortunately, try says this test now fails on Linux due to:

TEST-PASS | browser/base/content/test/urlbar/browser_bug495058.js | about:blank: uri loaded in detached tab - "about:blank" == "about:blank" - 
INFO - Buffered messages finished
INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/urlbar/browser_bug495058.js | about:blank: browser is focused - [object XULElement] == [object XULElement] - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/urlbar/browser_bug495058.js :: <TOP_LEVEL> :: line 35
INFO - Stack trace:
INFO -     chrome://mochitests/content/browser/browser/base/content/test/urlbar/browser_bug495058.js:null:35
INFO -     Tester_execTest@chrome://mochikit/content/browser-test.js:737:9
INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:657:7
INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59
INFO - TEST-PASS | browser/base/content/test/urlbar/browser_bug495058.js | about:blank: urlbar is empty - "" == "" - 
TEST-PASS | browser/base/content/test/urlbar/browser_bug495058.js | about:blank: placeholder text is present - "Search or enter address" == true - 

Mike, any ideas off hand why this might be?

Comment 5

[Mike Conley (:mconley) (:⚙️)]

(In reply to Mark Banner (:standard8, limited time in Dec) from comment #4)
> 
> Mike, any ideas off hand why this might be?

Ah, I have an idea, one sec...

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8819849 [details]
Bug 1279320 - Move browser_bug495058.js to urlbar directory, and fix issues with timing when it is run on its own.

https://reviewboard.mozilla.org/r/99504/#review100228

::: browser/base/content/test/urlbar/browser_bug495058.js:20
(Diff revision 2)
> +    let eventPromise;
>      if (tab.linkedBrowser.isRemoteBrowser) {
> -      yield BrowserTestUtils.waitForContentEvent(tab.linkedBrowser, "MozAfterPaint");
> +      eventPromise = BrowserTestUtils.waitForContentEvent(tab.linkedBrowser, "MozAfterPaint");
>      } else {
> -      yield BrowserTestUtils.waitForEvent(tab.linkedBrowser, "MozAfterPaint");
> +      eventPromise = BrowserTestUtils.waitForEvent(tab.linkedBrowser, "MozAfterPaint");
>      }

Bah - I missed this. :/

We're adding the MozAfterPaint event listener on the <xul:browser> that exists _before_ the tab tear out. That's no good - that browser is the one that gets closed once we do the frameloader swap. We're listening to the wrong thing.

You might be able to use BrowserTestUtils.domWindowOpened to get an early reference to the newly opened window, and then wait for the EndSwapDocShells event: http://searchfox.org/mozilla-central/rev/cc2a84852bd4e6f6d8d4d5b17b8382bb5d005749/toolkit/content/widgets/browser.xml#1405-1408

Once that's been done, try setting the MozAfterPaint event listeners. See if that helps?

Comment 7

[Mark Banner (:standard8)]

Mike, I tried this:

https://hg.mozilla.org/try/rev/1a4d365848a4c68550732020f9053818ebfd315f (on top of the patch), but Linux just times out with that. Any other ideas?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b441721c039e2f2aeeb18756d061e531ee8dd97&selectedJob=33125026

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 8819849 [details]
Bug 1279320 - Move browser_bug495058.js to urlbar directory, and fix issues with timing when it is run on its own.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/99504/diff/2-3/

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8819849 [details]
Bug 1279320 - Move browser_bug495058.js to urlbar directory, and fix issues with timing when it is run on its own.

Updated patch per discussions between Mike & I. Clearing review until I've verified the changes on try.

Comment 10

[Mark Banner (:standard8)]

Last time I tried this, on try, it was still randomly failing.

I don't think I have the knowledge to continue this. So un-assigning, and seeing if Mike wants to pick it up sometime.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Maybe someday, but maybe after 57 is out the door (unless I run out of other things to do before then! :) )

Comment 12

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.