Closed
Bug 570192
Opened 15 years ago
Closed 13 years ago
Intermittent timeout in browser_394759.js
Categories
(Firefox :: Session Restore, defect)
Tracking
()
VERIFIED
FIXED
Firefox 7
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
(Keywords: intermittent-failure, Whiteboard: [fixed by bug 595292])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
ehsan.akhgari
:
review-
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•15 years ago
|
||
Attachment #449311 -
Flags: review?(roc)
Comment 2•15 years ago
|
||
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...
Assignee | ||
Comment 3•15 years ago
|
||
(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)
Updated•15 years ago
|
Attachment #449311 -
Flags: review?(gavin.sharp)
Comment 5•15 years ago
|
||
Isn't this just bug 498339? Is with that bug, I don't think the error message has anything to do with the timeout...
Assignee | ||
Comment 6•15 years ago
|
||
(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.
Assignee | ||
Comment 7•15 years ago
|
||
gavin: ping?
Comment 8•15 years ago
|
||
(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 (Legacy TBPL/Treeherder Robot) |
Comment 10•14 years ago
|
||
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+
Comment 11•14 years ago
|
||
Please leave the bug open when landing this -- as noted earlier, the patch shouldn't help browser_394759.js at all.
Assignee | ||
Comment 12•14 years ago
|
||
(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! :-)
Assignee | ||
Comment 13•14 years ago
|
||
(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?
Assignee | ||
Updated•14 years ago
|
Attachment #449311 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #449311 -
Flags: approval2.0? → approval2.0+
Comment 14•14 years ago
|
||
(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
Assignee | ||
Comment 15•14 years ago
|
||
Assignee | ||
Updated•14 years ago
|
Assignee: ehsan → nobody
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•14 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 (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 24•14 years ago
|
||
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 (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 37•14 years ago
|
||
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•14 years ago
|
||
Fix landed a while ago, closing as fixed.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•14 years ago
|
Attachment #449311 -
Attachment is obsolete: true
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 40•14 years ago
|
||
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
Assignee | ||
Comment 41•14 years ago
|
||
(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.
Assignee | ||
Comment 42•14 years ago
|
||
Except that I won't reopen, since it happened on 3.5!
Assignee | ||
Comment 43•14 years ago
|
||
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...
Comment 44•14 years ago
|
||
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.
Assignee | ||
Comment 45•14 years ago
|
||
(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!
Assignee | ||
Updated•14 years ago
|
Whiteboard: [orange] → [orange][not-ready-for-cedar]
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 81•13 years ago
|
||
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 (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 87•13 years ago
|
||
(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";
Comment 88•13 years ago
|
||
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.
Comment 89•13 years ago
|
||
And there are several other "example.com" and "mozilla.org" URLs used in this test.
Comment 90•13 years ago
|
||
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 95•13 years ago
|
||
Attachment #541205 -
Flags: review-
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 97•13 years ago
|
||
(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 (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 122•13 years ago
|
||
I hereby declare victory!
Assignee: nobody → ehsan
Status: REOPENED → RESOLVED
Closed: 14 years ago → 13 years ago
Resolution: --- → FIXED
Whiteboard: [orange][not-ready-for-cedar] → [orange][fixed by bug 595292]
Target Milestone: --- → Firefox 7
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 131•13 years ago
|
||
Hello.
Is there a way in which this issue can be verified?
Comment 132•13 years ago
|
||
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
Updated•12 years ago
|
Keywords: intermittent-failure
Updated•12 years ago
|
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.
Description
•