Closed
Bug 609803
Opened 14 years ago
Closed 13 years ago
Intermittent timeout in browser/base/content/test/tabview/browser_tabview_launch.js followed by "browser_tabview_privatebrowsing.js | Tab View is visible again" and finally a suite timeout in browser_tabview_undo_group.js
Categories
(Firefox Graveyard :: Panorama, defect, P3)
Firefox Graveyard
Panorama
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 5
People
(Reporter: ehsan.akhgari, Assigned: raymondlee)
References
Details
(Keywords: intermittent-failure)
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1288912309.1288913819.3803.gz&fulltext=1
Rev3 MacOSX Snow Leopard 10.6.2 mozilla-central debug test mochitest-other on 2010/11/04 16:11:49
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•14 years ago
|
Priority: -- → P3
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 23•14 years ago
|
||
Re-open if this happens again.
Status: NEW → RESOLVED
Closed: 14 years ago
OS: Mac OS X → Windows 7
Resolution: --- → WORKSFORME
Updated•14 years ago
|
OS: Windows 7 → All
Hardware: x86 → All
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•14 years ago
|
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Assignee | ||
Comment 25•14 years ago
|
||
I've checked the launch test and the group test which runs before the launch test but couldn't find any potential issues.
I've updated the test so it runs in a new window so the test wouldn't be affected by other tests run before it.
Attachment #516182 -
Flags: review?(ian)
Comment 26•14 years ago
|
||
I unfortunately don't think that this will solve it. As you see in the log above browser_tabview_launch.js keeps running in the background while "_multiwindow_search.js" and "_undo_group.js" and "_privatebrowsing.js" are firing events.
1.) I think we should definitely use registerCleanupFunction() to remove our listeners and hide the tabview (etc.) when our test times out.
2.) The log shows that the "tabviewshown" event is fired but the condition "deck.selectedIndex == 1" never becomes true. That seems to become true when the tabview is shown in "_multiwindow_search.js".
I can't really explain why this isn't working but is .doCommand() really reliable? Though that seems to work because of the "tabviewshown" event. But then really selectedIndex should become "1"?!
Comment 27•14 years ago
|
||
Comment on attachment 516182 [details] [diff] [review]
v1
Please address Tim's comments.
Attachment #516182 -
Flags: review?(ian)
Assignee | ||
Comment 28•14 years ago
|
||
(In reply to comment #26)
> I unfortunately don't think that this will solve it. As you see in the log
> above browser_tabview_launch.js keeps running in the background while
> "_multiwindow_search.js" and "_undo_group.js" and "_privatebrowsing.js" are
> firing events.
>
> 1.) I think we should definitely use registerCleanupFunction() to remove our
> listeners and hide the tabview (etc.) when our test times out.
Added registerCleanupFunction()
>
> 2.) The log shows that the "tabviewshown" event is fired but the condition
> "deck.selectedIndex == 1" never becomes true. That seems to become true when
> the tabview is shown in "_multiwindow_search.js".
I read the log but I can't see that the "tabviewshown" event is fired (onTabViewLoadedAndShown is called) and the condition "deck.selectedIndex == 1" never becomes true. May be I am missing something?
>
> I can't really explain why this isn't working but is .doCommand() really
> reliable? Though that seems to work because of the "tabviewshown" event. But
> then really selectedIndex should become "1"?!
As mentioned above, I can't see the "tabviewshown" event is fired after .doCommand(). I've switched the selectedIndex to selectedPanel and hope it would make a difference.
If doCommand is not reliable, I am not sure what we can do to fix it. The "Browser:ToggleTabView" command element is simply called Tabview.toggle() and we are pretty sure that TabView.toggle() works fine because we use it in most of our tests.
Attachment #516182 -
Attachment is obsolete: true
Attachment #516848 -
Flags: review?(ian)
Attachment #516848 -
Flags: feedback?(tim.taubert)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•14 years ago
|
Attachment #516848 -
Attachment is patch: true
Attachment #516848 -
Attachment mime type: application/octet-stream → text/plain
Comment 30•14 years ago
|
||
Comment on attachment 516848 [details] [diff] [review]
v2
(In reply to comment #28)
> Added registerCleanupFunction()
Please use that to cancel the poll timeout, too. Otherwise the tests are run concurrently as shown below.
> > 2.) The log shows that the "tabviewshown" event is fired but the condition
> > "deck.selectedIndex == 1" never becomes true. That seems to become true when
> > the tabview is shown in "_multiwindow_search.js".
>
> I read the log but I can't see that the "tabviewshown" event is fired
> (onTabViewLoadedAndShown is called) and the condition "deck.selectedIndex == 1"
> never becomes true. May be I am missing something?
Have a look at http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1299371110.1299372254.10283.gz#err2
The test keeps polling here: https://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/tabview/browser_tabview_launch.js#40
browser_tabview_launch.js times out and continues to poll. deck.selectedIndex becomes "1" when the tabview gets shown in browser_tabview_multiwindow_search.js:
TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_multiwindow_search.js | Tab View is hidden
TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_launch.js | Tab View is visible. Count: 0
> If doCommand is not reliable, I am not sure what we can do to fix it. The
> "Browser:ToggleTabView" command element is simply called Tabview.toggle() and
> we are pretty sure that TabView.toggle() works fine because we use it in most
> of our tests.
You're right. IMHO we could safely use TabView.toggle() instead of doCommand(). I think that could solve the problem - I have no idea what else could cause it.
Attachment #516848 -
Flags: feedback?(tim.taubert) → feedback-
Comment 31•14 years ago
|
||
Comment on attachment 516848 [details] [diff] [review]
v2
Unflagging myself until Tim's new comments are addressed.
Attachment #516848 -
Flags: review?(ian)
Assignee | ||
Comment 32•14 years ago
|
||
(In reply to comment #30)
> Have a look at
> http://tinderbox.mozilla.org/showlog.cgi?log=TraceMonkey/1299371110.1299372254.10283.gz#err2
>
> The test keeps polling here:
> https://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/tabview/browser_tabview_launch.js#40
>
> browser_tabview_launch.js times out and continues to poll. deck.selectedIndex
> becomes "1" when the tabview gets shown in
> browser_tabview_multiwindow_search.js:
>
> TEST-PASS |
> chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_multiwindow_search.js
> | Tab View is hidden
> TEST-PASS |
> chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_launch.js
> | Tab View is visible. Count: 0
>
Yes but we can't tell whether the poll has started in the _launch.js or not. In the _launch.js, a "tabviewshown" event listener is added to call onTabViewLoadedAndShown(). In the _multiwindow_search.js, TabView.toggle() is called which can either trigger the "tabviewshown" code in the _launch.js or as you said the polling code which is already running. But there is not way we can tell whether the polling code has started or not.
Assignee | ||
Comment 33•14 years ago
|
||
Added code to clear the timer and use TabView.toggle() instead.
Assignee: nobody → raymond
Attachment #516848 -
Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #517676 -
Flags: review?(ian)
Comment 34•14 years ago
|
||
(In reply to comment #32)
> Yes but we can't tell whether the poll has started in the _launch.js or not.
> In the _launch.js, a "tabviewshown" event listener is added to call
> onTabViewLoadedAndShown(). In the _multiwindow_search.js, TabView.toggle() is
> called which can either trigger the "tabviewshown" code in the _launch.js or as
> you said the polling code which is already running. But there is not way we
> can tell whether the polling code has started or not.
Sorry, you're right. But thanks for making sure both ways don't lead to cascading failures :)
Comment 35•14 years ago
|
||
Comment on attachment 517676 [details] [diff] [review]
v3
Looks good, thanks!
Attachment #517676 -
Flags: review?(ian) → review+
Assignee | ||
Comment 36•14 years ago
|
||
Attachment #517676 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 38•14 years ago
|
||
bugspam
Reporter | ||
Comment 39•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox4.2
Updated•14 years ago
|
Target Milestone: Firefox5 → Firefox 5
Seen again:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1305268695.1305270014.3550.gz&fulltext=1
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 42•13 years ago
|
||
Filed bug 657219 to investigate timeout of browser_tabview_undo_group.js (this one's for browser_tabview_launch.js).
Status: REOPENED → RESOLVED
Closed: 14 years ago → 13 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Keywords: intermittent-failure
Updated•12 years ago
|
Whiteboard: [orange]
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•