Intermittent timeout in browser_394759.js

VERIFIED FIXED in Firefox 7

Status

()

Firefox
Session Restore
VERIFIED FIXED
7 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

({intermittent-failure})

Trunk
Firefox 7
x86
All
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed by bug 595292])

Attachments

(1 attachment, 1 obsolete attachment)

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1275672883.1275674743.28939.gz

This happens because the _setNewlineHandling method in textbox.xml (which is called upon construction) does not check if this.editor is null (which it can be, for example, if we don't have a frame).

Patch+test forthcoming.
Blocks: 498339
Created attachment 449311 [details] [diff] [review]
Patch (v1)
Attachment #449311 - Flags: review?(roc)
When does this actually happen? If it's possible for the constructor to be firing while this.editor is null, then the newline handling needs to be set up at some other point...
(In reply to comment #2)
> When does this actually happen?

See the test case in the patch for at least one way in which this could happen (not to say that the test case is realistic!  But this has happened intermittently on the unit test machines.)

> If it's possible for the constructor to be
> firing while this.editor is null, then the newline handling needs to be set up
> at some other point...

It's kind of tricky, because the widget has no way of knowing when an editor object becomes available.  One good place to set newline handling is right before pasting, but I'm not sure if it's possible to hook into cmd_paste that way.
Comment on attachment 449311 [details] [diff] [review]
Patch (v1)

I can't review toolkit
Attachment #449311 - Flags: review?(roc)
Attachment #449311 - Flags: review?(gavin.sharp)
Attachment #449311 - Flags: review?(gavin.sharp)
Isn't this just bug 498339? Is with that bug, I don't think the error message has anything to do with the timeout...
(In reply to comment #5)
> Isn't this just bug 498339?

Yes.  Except that this patch is solving the real problem (which is a code problem, not a test problem).

> Is with that bug, I don't think the error message
> has anything to do with the timeout...

Well, if I understand correctly, the error message might be causing the execution to be halted, and therefore finish never being called, hence the timeout.
gavin: ping?
(In reply to comment #6)
> > Is with that bug, I don't think the error message
> > has anything to do with the timeout...
> 
> Well, if I understand correctly, the error message might be causing the
> execution to be halted, and therefore finish never being called, hence the
> timeout.

It's in a different window and asynchronous, so similar to event listeners and timer callbacks, a halted xbl constructor isn't going to affect other JS, as I understand it.
Comment hidden (Treeherder Robot)

Updated

7 years ago
OS: Mac OS X → All
Comment on attachment 449311 [details] [diff] [review]
Patch (v1)

I filed bug 592528 on fixing the real issue - I'm doubtful that this will do anything but silence the exception, but since it doesn't really do any harm, I suppose it's fine.

Does the test fail without the code change? I wouldn't expect it to... I'm not sure that it's useful.
Attachment #449311 - Flags: review?(gavin.sharp) → review+
Please leave the bug open when landing this -- as noted earlier, the patch shouldn't help browser_394759.js at all.
(In reply to comment #10)
> Does the test fail without the code change? I wouldn't expect it to... I'm not
> sure that it's useful.

Yes, it does.  That's why it can test the patch!  :-)
(In reply to comment #11)
> Please leave the bug open when landing this -- as noted earlier, the patch
> shouldn't help browser_394759.js at all.

So, what does happen when you open a new window and an XBL constructor on that window throws?  Do we get a load event like normal?
Attachment #449311 - Flags: approval2.0?
Attachment #449311 - Flags: approval2.0? → approval2.0+
(In reply to comment #13)
> (In reply to comment #11)
> > Please leave the bug open when landing this -- as noted earlier, the patch
> > shouldn't help browser_394759.js at all.
> 
> So, what does happen when you open a new window and an XBL constructor on that
> window throws?  Do we get a load event like normal?

Why wouldn't we? We get the "this.editor is null" error message even when the test passes: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1283423979.1283425603.15720.gz&fulltext=1
Fix pushed:

http://hg.mozilla.org/mozilla-central/rev/01cbf9182f77
Assignee: ehsan → nobody
Comment hidden (Treeherder Robot)

Updated

7 years ago
Status: ASSIGNED → NEW
Component: XUL Widgets → Session Restore
Product: Toolkit → Firefox
QA Contact: xul.widgets → session.restore
Summary: Intermittent timeout in browser_394759.js with "JavaScript error: chrome://global/content/bindings/textbox.xml, line 140: this.editor is null" → Intermittent timeout in browser_394759.js
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)
I would guess this latest timeout explosion means "and there's also timeout potential if there's no access to the network, because this test is bad and hits it instead of just stuff on localhost."
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)
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1298416270.1298419410.28800.gz
Rev3 Fedora 12x64 mozilla-central debug test mochitest-other on 2011/02/22 15:11:10
s: talos-r3-fed64-017

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_overflowScroll.js | Scrolled one tab to the left with a single click - Got 154, expected 54
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_394759.js | Test timed out
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_394759.js | Found a browser window after previous test timed out
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_bug577990.js | Test timed out
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_bug577990.js | Found unexpected add-ons manager window still open
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_bug577990.js | Should not get category when manager window is not loaded - Didn't expect null, but got it
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/xpinstall/browser_relative.js | Test timed out
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/xpinstall/browser_relative.js | Found a tab after previous test timed out: about:blank

Comment 38

7 years ago
Fix landed a while ago, closing as fixed.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

7 years ago
Attachment #449311 - Attachment is obsolete: true
Comment hidden (Treeherder Robot)
I want to point out that the above failure looks suspiciously like bug 498339; in particular, we have the "textbox is null" error before the timeout.

Running chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759.js...
TEST-INFO | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759.js | Console message: [JavaScript Error: "textbox is null" {file: "chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759.js" line: 70}]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759.js | Timed out
Running chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759_privatebrowsing.js...
TEST-PASS | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759_privatebrowsing.js | sessionstore.js was removed
(In reply to comment #40)
> I want to point out that the above failure looks suspiciously like bug 498339;
> in particular, we have the "textbox is null" error before the timeout.

Well, it's bug 498339 in fact!  I'll comment there and reopen.
Except that I won't reopen, since it happened on 3.5!
Dao: do you think I can close this bug now?  I think by this time we know that the orange was fixed by my patch here...
I'm not sure what exactly you're saying. browser_394759.js keeps timing out intermittently, but without the textbox error message, as predicted before your patch landed.
(In reply to comment #44)
> I'm not sure what exactly you're saying. browser_394759.js keeps timing out
> intermittently, but without the textbox error message, as predicted before your
> patch landed.

You're right.  Please ignore my comment!
Whiteboard: [orange] → [orange][not-ready-for-cedar]
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)
Depends on: 595292
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)
And to add to your fun figuring out why this exploded, it failed four times (Mac Mac Win Win) on TraceMonkey, during this morning's downtime.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(In reply to comment #24)
> I would guess this latest timeout explosion means "and there's also timeout
> potential if there's no access to the network, because this test is bad and
> hits it instead of just stuff on localhost."

Looks like the culprit might be right here:
http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/test/browser/browser_394759.js#164

> let url = "http://window" + windowsToOpen.length + ".example.com";
I also see in the log:

TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_394759.js | Console message: [JavaScript Error: "ASSERTION: browser.js host is inconsistent. Content window has <window4.example.com> but cached host is <>.
" {file: "chrome://browser/content/browser.js" line: 9533}]

Is that relevant?  Looking at browser.js's onSecurityChange, it looks like this assertion will prevent navigation from happening.
And there are several other "example.com" and "mozilla.org" URLs used in this test.
Created attachment 541205 [details] [diff] [review]
disable the test
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment on attachment 541205 [details] [diff] [review]
disable the test

This is bug 595292.
Attachment #541205 - Flags: review-
Comment hidden (Treeherder Robot)
(In reply to comment #88)
> I also see in the log:
> 
> TEST-INFO |
> chrome://mochitests/content/browser/browser/components/sessionstore/test/
> browser/browser_394759.js | Console message: [JavaScript Error: "ASSERTION:
> browser.js host is inconsistent. Content window has <window4.example.com>
> but cached host is <>.
> " {file: "chrome://browser/content/browser.js" line: 9533}]
> 
> Is that relevant?

nope
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)
I hereby declare victory!
Assignee: nobody → ehsan
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [orange][not-ready-for-cedar] → [orange][fixed by bug 595292]
Target Milestone: --- → Firefox 7
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)
Hello.

Is there a way in which this issue can be verified?
Steps to verify [orange] bugs:
1) Look for the latest TinderboxPushlog Robot comments on the bug report
2) Ensure that none of the recent comments are for a branch that got the fix (e.g. trunk)
Status: RESOLVED → VERIFIED
Keywords: intermittent-failure
Whiteboard: [orange][fixed by bug 595292] → [fixed by bug 595292]
You need to log in before you can comment on or make changes to this bug.