Closed Bug 600883 Opened 14 years ago Closed 14 years ago

Intermittent browser_bug579872.js | Test timed out

Categories

(Firefox :: Tabbed Browser, defect)

Product:
Firefox
Component:
Tabbed Browser
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 4.0b7

People

(Reporter: orangereporter, Assigned: sindrebugzilla)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 2 obsolete files)

maybe a fix 2
1.10 KB, patch
dao
: review+
Details | Diff | Splinter Review 
Started close enough to bug 600785 to make me think they probably have the same cause, but it lacks even the tiny hints about what's happening that that one has.

http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1285853802.1285854758.20757.gz
Rev3 MacOSX Snow Leopard 10.6.2 tracemonkey opt test mochitest-other on 2010/09/30 06:36:42
s: talos-r3-snow-047

TEST-START | chrome://mochikit/content/browser/browser/base/content/test/browser_bug579872.js
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_bug579872.js | Test timed out
TEST-INFO | chrome://mochikit/content/browser/browser/base/content/test/browser_bug579872.js | Test took 30.011s to complete

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_bug579872.js | Found a tab after previous test timed out: about:blank
http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1285796026.1285796884.14664.gz
Rev3 MacOSX Leopard 10.5.8 tracemonkey opt test mochitest-other on 2010/09/29 14:33:46
s: talos-r3-leopard-020

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_bug579872.js | Test timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/base/content/test/browser_bug579872.js | Found a tab after previous test timed out: about:blank
Where by "probably have the same cause" I apparently mean "are utterly unrelated, and just happened to show up on TM a few times before the first m-c instance."
Assignee: general → nobody
Component: JavaScript Engine → Tabbed Browser
No longer depends on: 600785
Product: Core → Firefox
QA Contact: general → tabbed.browser
Summary: TM: intermittent browser_bug579872.js | Test timed out → Intermittent browser_bug579872.js | Test timed out
(In reply to comment #4)
> Where by "probably have the same cause" I apparently mean "are utterly
> unrelated, and just happened to show up on TM a few times before the first m-c
> instance."

So a TM merge could have introduced the failure on m-c, right?
Assignee: nobody → general
Component: Tabbed Browser → JavaScript Engine
Product: Firefox → Core
QA Contact: tabbed.browser → general
A TM merge *could* have, but then you have to bet on:

1. TM introduces the cause of the failure, but it never happens on TM
2. TM merges the cause of the failure to m-c, but it never happens on m-c
3. Finally, the failure shows up on TM, three times in one day
4. Now the first m-c failure shows up

Or, since m-c -> TM merges are much more frequent, you could instead bet on

1. m-c introduces the cause of the failure, but it doesn't happen before
2. m-c is merged to TM
3. the first three happen on TM
4. one happens on MC
Assignee: general → nobody
Component: JavaScript Engine → Tabbed Browser
Product: Core → Firefox
QA Contact: general → tabbed.browser
Though I'd forgotten when the merges in each direction went this week: yeah, it's not impossible that it was introduced but not seen on TM, merged to m-c, seen on TM, seen on m-c.
Blocks: 579872
Could the problem be in http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_bug579872.js?mark=38,40#38 where we initially add an example.com tab (proxied to localhost, and thus fast), call waitForExplicitFinish(), and only then add the load event listener, when load could possibly have already fired? We do seem to have a few other tests that follow that pattern (though in a quick look I didn't see any calling anything else between addTab and addEventListener), but for the most part we seem to do "addTab(); addEventListener(); loadURI("...");" instead.
Attached patch maybe a fix (obsolete) — — Splinter Review 
This incorporates Phil Ringnalda's suggested fix. Though I can't actually reproduce this time out, so I can't tell if this fixes it. Perhaps someone could push it to try server to check?
I did push a similar patch to try yesterday, and it didn't time out, but the frequency is low enough that you can't really draw any conclusions from that.
Comment on attachment 482221 [details] [diff] [review]
maybe a fix

Pushed to try in http://tbpl.swatinem.de/index.html?tree=MozillaTry&rev=f17660278eb4 which was happy with it. No way of telling if it really fixes it, but it seems plausible, at least.
Attachment #482221 - Flags: review?(dao)
Attachment #482221 - Flags: review?(dao) → review+
Landed in http://hg.mozilla.org/mozilla-central/rev/3f75beee46f2

If your memory is better than mine, you might want to leave the bug open for a day or two to see if we still get any reports (other than from pushes before that, and things like TraceMonkey that won't merge it for a while) and then close the bug.
I doubt that fix will help; I'm pretty sure the load event is guaranteed not to fire before that function terminates, regardless of how fast it is, given that it's still coming "from the network". I think the only synchronous loads we have are about:blank under some circumstances, and maybe data URIs.
(And that being TraceMonkey without a merge, doesn't tell us anything.)
Attached patch maybe a fix 2 — — Splinter Review 
Well, it appears like that didn't work.

After investigating it seems to me like "http://www.example.com" (unlike "http://example.com") is not proxied to localhost. The test will therefore connect to the web in order of loading the page, if it can't connect the test times out.
Attachment #482221 - Attachment is obsolete: true
Attachment #482633 - Flags: review?(dao)
(In reply to comment #116)
> After investigating it seems to me like "http://www.example.com" (unlike
> "http://example.com") is not proxied to localhost. The test will therefore
> connect to the web in order of loading the page, if it can't connect the test
> times out.

That probably calls for a patch to add www.example.com:80 and www.example.org:80 to http://mxr.mozilla.org/mozilla-central/source/build/pgo/server-locations.txt, which will then let you close a dozen or so other timeout [orange] bugs. Nice work!
Attached patch moar www (obsolete) — — Splinter Review 
So much [orange].
Attachment #482689 - Flags: review?(jwalden+bmo)
Comment on attachment 482633 [details] [diff] [review]
maybe a fix 2

>diff --git a/browser/base/content/test/browser_bug579872.js b/browser/base/content/test/browser_bug579872.js
>--- a/browser/base/content/test/browser_bug579872.js
>+++ b/browser/base/content/test/browser_bug579872.js
>@@ -41,21 +41,21 @@ function test() {
>   
>   function mainPart() {
>     gBrowser.pinTab(newTab);
>     gBrowser.selectedTab = newTab;
>     
>     openUILinkIn("javascript:var x=0;", "current");
>     is(gBrowser.tabs.length, 2, "Should open in current tab");
>     
>-    openUILinkIn("http://www.example.com/1", "current");
>+    openUILinkIn("http://example.com/1", "current");
>     is(gBrowser.tabs.length, 2, "Should open in current tab");
>     
>-    openUILinkIn("http://www.example.org/", "current");
>+    openUILinkIn("http://example.org/", "current");
>     is(gBrowser.tabs.length, 3, "Should open in new tab");
>     
>     newTab.removeEventListener("load", mainPart, true);
>     gBrowser.removeTab(newTab);
>     gBrowser.removeTab(gBrowser.tabs[1]); // example.org tab
>     finish();
>   }
>-  newTab.linkedBrowser.loadURI("http://www.example.com");
>+  newTab.linkedBrowser.loadURI("http://example.com");
> }
Attachment #482633 - Attachment is obsolete: true
Attachment #482633 - Flags: review?(dao)
Comment on attachment 482633 [details] [diff] [review]
maybe a fix 2

Sadly, my first thought about pretty much everything always turns out to be wrong, and my thought that I had seen a bunch of other tests loading www.example.com/.org isn't an exception - apparently everyone but us already knew about that, and avoided it (or, had to actually care what they loaded, and found out about it that way when it didn't load localhost for them).
Attachment #482633 - Attachment is obsolete: false
Attachment #482633 - Flags: review?(dao)
Attachment #482689 - Attachment is obsolete: true
Attachment #482689 - Flags: review?(jwalden+bmo)
Attachment #482633 - Flags: review?(dao) → review+
http://hg.mozilla.org/mozilla-central/rev/1193f7bfec2e
Assignee: nobody → sindrebugzilla
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
Something tells me this isn't fixed... correct?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
No, it appears to be fixed - those instances on the Places tree were from before it got a mozilla-central merge, so it was stale, and the one from mozilla-central was from a push before the fix landed.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 4.0b8 → Firefox 4.0b7
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: