Intermittent browser_alltabslistener.js | Test timed out - expected PASS

NEW
Unassigned

Status

()

Firefox
General
P3
normal
4 years ago
2 years ago

People

(Reporter: Tomcat, Unassigned)

Tracking

({intermittent-failure, leave-open})

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Ubuntu VM 12.04 b2g-inbound pgo test mochitest-browser-chrome-1

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=865215&repo=b2g-inbound

07:39:55 INFO - 59 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_alltabslistener.js | Test timed out - expected PASS
Comment hidden (Treeherder Robot)

Comment 2

4 years ago
Dave, looks like something broke. :-(
Blocks: 1093594
Flags: needinfo?(dtownsend+bugmail)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
My best guess here is that if we initiated a real network load (dummy_page.html would do) after starting the waitForDocLoadComplete promises it might avoid a race condition. I may not have time to test or try that for at least a week if not two though.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 66

3 years ago
Created attachment 8529057 [details] [diff] [review]
use dummy page instead of about:blank to avoid a race,

comment 25 seems worth trying. Mike, look OK to you?
Attachment #8529057 - Flags: review?(mconley)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment on attachment 8529057 [details] [diff] [review]
use dummy page instead of about:blank to avoid a race,

Review of attachment 8529057 [details] [diff] [review]:
-----------------------------------------------------------------

Let's try it.

::: browser/base/content/test/general/browser_alltabslistener.js
@@ +85,5 @@
>  
>  function test() {
>    waitForExplicitFinish();
>  
> +  gBackgroundTab = gBrowser.addTab("http://example.org/browser/browser/base/content/test/general/dummy_page.html");

If this sticks, it'd probably make more sense to have the URL be a const that these refer to.

So maybe do that before landing.
Attachment #8529057 - Flags: review?(mconley) → review+
Comment hidden (Treeherder Robot)

Comment 73

3 years ago
w/ the URL pulled into a global constant:

remote:   https://hg.mozilla.org/integration/fx-team/rev/6e82407a7c65

Comment 74

3 years ago
... I should just go to sleep.

I meant:

https://hg.mozilla.org/integration/fx-team/rev/09034b8710cb
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 78

3 years ago
(In reply to TBPL Robot from comment #77)
> submit_timestamp: 2014-11-26T21:10:22
> log:
> https://treeherder.mozilla.org/ui/logviewer.html#?repo=fx-team&job_id=1307952
> repository: fx-team
> who: philringnalda[at]gmail[dot]com
> machine: tst-linux64-spot-797
> buildname: Ubuntu VM 12.04 x64 fx-team debug test mochitest-browser-chrome-1
> revision: 679466398b30
> 
> 54 INFO TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/browser/browser/base/content/test/general/
> browser_alltabslistener.js | Test timed out - expected PASS
> 56 INFO TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/browser/browser/base/content/test/general/
> browser_alltabslistener.js | Found a tab after previous test timed out:
> http://example.org/browser/browser/base/content/test/general/dummy_page.html
> - expected PASS
> 57 INFO TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/browser/browser/base/content/test/general/
> browser_alltabslistener.js | Found a tab after previous test timed out:
> http://example.org/browser/browser/base/content/test/general/dummy_page.html
> - expected PASS

Computer says no. I almost wonder if there's an actual bug with the state listener not being fired? :-\
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Updated

3 years ago
Keywords: leave-open
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Updated

3 years ago
Attachment #8529057 - Flags: checkin+
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Created attachment 8536798 [details] [diff] [review]
patch

So the last change wasn't quite what I described, this is what I meant. I ran this test a few hundred times by itself on try and saw no failures. But then I also ran it by itself without the change a few hundred times and also saw no failure so seems likely something happening in an earlier test is making this one unstable somehow.

Either way probably doesn't hurt to try this change and see what happens.
Flags: needinfo?(dtownsend)
Attachment #8536798 - Flags: review?(gijskruitbosch+bugs)

Comment 154

3 years ago
Comment on attachment 8536798 [details] [diff] [review]
patch

Review of attachment 8536798 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/general/browser_alltabslistener.js
@@ +99,5 @@
>      waitForDocLoadComplete(gBackgroundBrowser),
>      waitForDocLoadComplete(gForegroundBrowser)
>    ];
> +  gBackgroundBrowser.loadURI(kBasePage);
> +  gForegroundBrowser.loadURI(kBasePage);

While we're here, any reason not to switch to promiseTabLoadEvent(tab, kBasePage), which seems to be more frequently used in browser/base/content/test/general, and automagically ignores about:blank, and will save us a few lines of code here? :-)
Attachment #8536798 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #154)
> Comment on attachment 8536798 [details] [diff] [review]
> patch
> 
> Review of attachment 8536798 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/test/general/browser_alltabslistener.js
> @@ +99,5 @@
> >      waitForDocLoadComplete(gBackgroundBrowser),
> >      waitForDocLoadComplete(gForegroundBrowser)
> >    ];
> > +  gBackgroundBrowser.loadURI(kBasePage);
> > +  gForegroundBrowser.loadURI(kBasePage);
> 
> While we're here, any reason not to switch to promiseTabLoadEvent(tab,
> kBasePage), which seems to be more frequently used in
> browser/base/content/test/general, and automagically ignores about:blank,
> and will save us a few lines of code here? :-)

Remember how we switched away from the tab load event in bug 1093594 because progress events were still happening afterwards?
Comment hidden (Treeherder Robot)

Comment 157

3 years ago
(In reply to Dave Townsend [:mossop] (Out till December 1st) from comment #155)
> (In reply to :Gijs Kruitbosch from comment #154)
> > Comment on attachment 8536798 [details] [diff] [review]
> > patch
> > 
> > Review of attachment 8536798 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: browser/base/content/test/general/browser_alltabslistener.js
> > @@ +99,5 @@
> > >      waitForDocLoadComplete(gBackgroundBrowser),
> > >      waitForDocLoadComplete(gForegroundBrowser)
> > >    ];
> > > +  gBackgroundBrowser.loadURI(kBasePage);
> > > +  gForegroundBrowser.loadURI(kBasePage);
> > 
> > While we're here, any reason not to switch to promiseTabLoadEvent(tab,
> > kBasePage), which seems to be more frequently used in
> > browser/base/content/test/general, and automagically ignores about:blank,
> > and will save us a few lines of code here? :-)
> 
> Remember how we switched away from the tab load event in bug 1093594 because
> progress events were still happening afterwards?

Well, the previous incarnation only checked 1 of the two tabs, and didn't ignore about:blank loads. Is fixing those bits not enough here?
(In reply to :Gijs Kruitbosch from comment #157)
> (In reply to Dave Townsend [:mossop] (Out till December 1st) from comment
> #155)
> > (In reply to :Gijs Kruitbosch from comment #154)
> > > Comment on attachment 8536798 [details] [diff] [review]
> > > patch
> > > 
> > > Review of attachment 8536798 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > ::: browser/base/content/test/general/browser_alltabslistener.js
> > > @@ +99,5 @@
> > > >      waitForDocLoadComplete(gBackgroundBrowser),
> > > >      waitForDocLoadComplete(gForegroundBrowser)
> > > >    ];
> > > > +  gBackgroundBrowser.loadURI(kBasePage);
> > > > +  gForegroundBrowser.loadURI(kBasePage);
> > > 
> > > While we're here, any reason not to switch to promiseTabLoadEvent(tab,
> > > kBasePage), which seems to be more frequently used in
> > > browser/base/content/test/general, and automagically ignores about:blank,
> > > and will save us a few lines of code here? :-)
> > 
> > Remember how we switched away from the tab load event in bug 1093594 because
> > progress events were still happening afterwards?
> 
> Well, the previous incarnation only checked 1 of the two tabs, and didn't
> ignore about:blank loads. Is fixing those bits not enough here?

Maybe, not sure. But I have to divert my attention to something else right now so I'm going to land this as-is and come back to it if it is still failing:

https://hg.mozilla.org/integration/fx-team/rev/802cc1027e76
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Clearly that didn't work
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)

Comment 176

3 years ago
(In reply to Dave Townsend [:mossop] from comment #161)
> Clearly that didn't work

No, but the odd thing is that it seems that from:

https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-inbound&job_id=4657968

the pages are loading fine, and are loading well before the timeout happens. So it seems like the promises aren't being fulfilled despite the pages loading:

18:01:37 INFO - TEST-INFO | screentopng: exit 0
18:01:37 INFO - 49 INFO checking window state
18:01:37 INFO - 50 INFO Waiting for browser load
18:01:37 INFO - 51 INFO Waiting for browser load
18:01:37 INFO - 52 INFO Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.org/browser/browser/base/content/test/general/dummy_page.html" line: 0}]
18:01:37 INFO - 53 INFO Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.org/browser/browser/base/content/test/general/dummy_page.html" line: 0}]
18:01:37 INFO - 54 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_alltabslistener.js | Test timed out - expected PASS 


on the other hand, the timeout screenshot doesn't show either of the tabs, so it's not clear to me when that screenshot was taken relative to when the test timed out and/or the harness cleaned up the extra tabs.
Comment hidden (Treeherder Robot)
(In reply to :Gijs Kruitbosch from comment #176)
> (In reply to Dave Townsend [:mossop] from comment #161)
> > Clearly that didn't work
> 
> No, but the odd thing is that it seems that from:
> 
> https://treeherder.mozilla.org/ui/logviewer.html#?repo=mozilla-
> inbound&job_id=4657968
> 
> the pages are loading fine, and are loading well before the timeout happens.
> So it seems like the promises aren't being fulfilled despite the pages
> loading:

What's more, the progress listener logs every onStateChange it sees. And it apparently sees none so it appears that the progress listener is completely broken, and this is in non-e10s!

Comment 179

3 years ago
At least some of the (orange) logs have them, I also suspect I picked a run from before your last patch, but I did think there should have been logs even then...
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)