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

RESOLVED FIXED in Firefox 5

Status

defect
P3
normal
RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: Ehsan, Assigned: raymondlee)

Tracking

({intermittent-failure})

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

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)
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)
Blocks: 585689
Assignee

Comment 23

8 years ago
Re-open if this happens again.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
OS: Mac OS X → Windows 7
Resolution: --- → WORKSFORME
OS: Windows 7 → All
Hardware: x86 → All
Comment hidden (Legacy TBPL/Treeherder Robot)
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Assignee

Comment 25

8 years ago
Posted patch v1 (obsolete) — Splinter Review
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)
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 on attachment 516182 [details] [diff] [review]
v1

Please address Tim's comments.
Attachment #516182 - Flags: review?(ian)
Assignee

Comment 28

8 years ago
Posted patch v2 (obsolete) — Splinter Review
(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)
Attachment #516848 - Attachment is patch: true
Attachment #516848 - Attachment mime type: application/octet-stream → text/plain
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 on attachment 516848 [details] [diff] [review]
v2

Unflagging myself until Tim's new comments are addressed.
Attachment #516848 - Flags: review?(ian)
Assignee

Comment 32

8 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

8 years ago
Posted patch v3 (obsolete) — Splinter Review
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)
(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 on attachment 517676 [details] [diff] [review]
v3

Looks good, thanks!
Attachment #517676 - Flags: review?(ian) → review+
Assignee

Updated

8 years ago
Keywords: checkin-needed
Comment hidden (Legacy TBPL/Treeherder Robot)
No longer blocks: 585689

Comment 38

8 years ago
bugspam
http://hg.mozilla.org/mozilla-central/rev/86aa49121031
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox4.2
Target Milestone: Firefox5 → Firefox 5
Comment hidden (Legacy TBPL/Treeherder Robot)
Filed bug 657219 to investigate timeout of browser_tabview_undo_group.js (this one's for browser_tabview_launch.js).
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Whiteboard: [orange]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.