Last Comment Bug 625273 - Intermittent browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
: Intermittent browser_tabview_bug587503.js | In the end, the group was showing...
Status: RESOLVED FIXED
: intermittent-failure
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: P3 normal
: Firefox 17
Assigned To: Andres Hernandez [:andreshm]
:
Mentors:
Depends on: 780134
Blocks: 438871 585689
  Show dependency treegraph
 
Reported: 2011-01-12 19:30 PST by Phil Ringnalda (:philor)
Modified: 2016-04-12 14:00 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (12.92 KB, patch)
2012-07-09 16:35 PDT, Andres Hernandez [:andreshm]
ttaubert: feedback+
Details | Diff | Splinter Review
Patch v2 (13.30 KB, patch)
2012-07-11 16:04 PDT, Andres Hernandez [:andreshm]
ttaubert: review+
Details | Diff | Splinter Review
Patch v3 (13.27 KB, patch)
2012-07-26 10:58 PDT, Andres Hernandez [:andreshm]
no flags Details | Diff | Splinter Review
Patch v4 (12.95 KB, patch)
2012-07-27 15:07 PDT, Andres Hernandez [:andreshm]
no flags Details | Diff | Splinter Review
Patch v5 (12.99 KB, patch)
2012-08-01 09:42 PDT, Andres Hernandez [:andreshm]
ttaubert: review+
Details | Diff | Splinter Review

Description Phil Ringnalda (:philor) 2011-01-12 19:30:44 PST
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1294886929.1294888492.26455.gz#err4
Build Log (Brief)
Rev3 WINNT 6.1 mozilla-central opt test mochitest-other on 2011/01/12 18:48:49
s: talos-r3-w7-043

TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_bug587503.js | Tab 5 is back and again the fifth tab.
TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_bug587503.js | The group began by not showing a dropSpace
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_bug587503.js | The group is empty again
Comment 1 Treeherder Robot 2011-01-12 22:36:47 PST
philringnalda%gmail.com
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1294898870.1294900389.22212.gz
Rev3 WINNT 6.1 mozilla-central opt test mochitest-other on 2011/01/12 22:07:50

s: talos-r3-w7-033
PROCESS-CRASH | Main app process exited normally | application crashed (minidump found)
Thread 0 (crashed)
PROCESS-CRASH | Main app process exited normally | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 2 Raymond Lee [:raymondlee] 2011-02-23 19:35:35 PST
Don't see this happens recently.  May be this has been fixed by other patch(es)?
Comment 3 Treeherder Robot 2011-11-28 01:01:24 PST
philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=7602445&tree=Mozilla-Aurora
Rev3 Fedora 12x64 mozilla-aurora pgo test mochitest-other on 2011-11-27 08:09:57

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 4 Treeherder Robot 2012-03-06 04:53:00 PST
ttaubert
https://tbpl.mozilla.org/php/getParsedLog.php?id=9848632&tree=Fx-Team
Rev3 MacOSX Leopard 10.5.8 fx-team debug test mochitest-other on 2012-03-06 03:51:38

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 5 Tim Taubert [:ttaubert] 2012-03-06 04:53:27 PST
Seems to happen very rarely, but it does.
Comment 6 Treeherder Robot 2012-03-07 04:54:58 PST
msucan
https://tbpl.mozilla.org/php/getParsedLog.php?id=9877364&tree=Fx-Team
Rev3 MacOSX Leopard 10.5.8 fx-team debug test mochitest-other on 2012-03-07 02:05:34

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 7 Treeherder Robot 2012-03-07 06:57:21 PST
msucan
https://tbpl.mozilla.org/php/getParsedLog.php?id=9881269&tree=Fx-Team
Rev3 MacOSX Leopard 10.5.8 fx-team debug test mochitest-other on 2012-03-07 05:32:29

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 8 Treeherder Robot 2012-03-09 15:55:03 PST
dolske
https://tbpl.mozilla.org/php/getParsedLog.php?id=9953045&tree=Try
Rev3 WINNT 6.1 try debug test mochitest-other on 2012-03-09 14:01:37

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 9 Treeherder Robot 2012-03-09 15:55:16 PST
dolske
https://tbpl.mozilla.org/php/getParsedLog.php?id=9954061&tree=Try
Rev3 WINNT 6.1 try debug test mochitest-other on 2012-03-09 14:40:41

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 564 bytes during test execution
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of CondVar with size 16 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of Mutex with size 12 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of nsCacheEntryHashTable with size 36 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of nsCacheService with size 148 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 3 instances of nsDiskCacheBinding with size 52 bytes each (156 bytes total)
Comment 10 Treeherder Robot 2012-03-26 13:03:41 PDT
philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=10371015&tree=Mozilla-Aurora
Rev3 Fedora 12 mozilla-aurora pgo test mochitest-other on 2012-03-26 05:15:41

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 11 Treeherder Robot 2012-03-26 13:04:05 PDT
philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=10371010&tree=Mozilla-Aurora
Rev3 Fedora 12x64 mozilla-aurora pgo test mochitest-other on 2012-03-26 05:16:32

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 12 Treeherder Robot 2012-04-08 09:47:49 PDT
philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=10736014&tree=Mozilla-Aurora
Rev3 Fedora 12 mozilla-aurora pgo test mochitest-other on 2012-04-08 05:16:00

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 13 Treeherder Robot 2012-04-13 16:20:33 PDT
philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=10873004&tree=Mozilla-Aurora
Rev3 Fedora 12x64 mozilla-aurora pgo test mochitest-other on 2012-04-13 05:21:07

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 14 Treeherder Robot 2012-04-23 16:02:10 PDT
philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=11122449&tree=Mozilla-Aurora
Rev3 Fedora 12x64 mozilla-aurora pgo test mochitest-other on 2012-04-23 05:16:29
slave: talos-r3-fed64-021

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/aboutmemory/tests/test_memoryReporters.xul | application timed out after 330 seconds with no output
PROCESS-CRASH | chrome://mochitests/content/chrome/toolkit/components/aboutmemory/tests/test_memoryReporters.xul | application crashed (minidump found)
Thread 0 (crashed)
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 15 Andrew McCreight [:mccr8] 2012-06-19 11:48:32 PDT
Does anybody have any idea why this happened?  I have a cycle collector patch that is triggering this about 1/4 the time, which is bizarre.
Comment 16 Treeherder Robot 2012-07-03 03:32:44 PDT
edmorley
https://tbpl.mozilla.org/php/getParsedLog.php?id=13174363&tree=Profiling
Rev3 WINNT 6.1 profiling opt test mochitest-other on 2012-07-02 16:56:53
slave: talos-r3-w7-016

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 17 Andres Hernandez [:andreshm] 2012-07-05 17:20:43 PDT
I'm able to reproduce this bug when moving focus out of the window while testing. 
I think is probably related to the timeouts in the test code. 

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | Tab 5 is back and again the fifth tab. - Got 6, expected 4
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 18 Andres Hernandez [:andreshm] 2012-07-09 16:35:40 PDT
Created attachment 640417 [details] [diff] [review]
Patch v1

I updated the test in order to use the common methods in the head file and to try to avoid the focus issues with timeouts. 
The tests should be checking the same things that in the old test.  

Also, if we include the TestRunner in the TabCandy tests, then we can improve it more.
Comment 19 Tim Taubert [:ttaubert] 2012-07-11 08:37:13 PDT
Comment on attachment 640417 [details] [diff] [review]
Patch v1

Review of attachment 640417 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for refactoring this antique test! Looks good but there are still some issues we need to fix.

The test failed for me while running the whole test suites but not when running standalone. This is most definitely caused by tests that run before this one that move and resize groups. To achieve more consistency when having tests like this that need exact group and tab positions and sizes we usually open a new window, do our testing and close it afterwards. It's pretty easy as you'll see below.

::: browser/components/tabview/test/browser_tabview_bug587503.js
@@ +41,5 @@
> +
> +    children.forEach(function(child) {
> +      url = child.tab.linkedBrowser.currentURI.spec;
> +      indexes.push(url.replace("about:blank#", ''));
> +    });

You could do:

> return aGroup.getChildren().map(function (child) {
>   let url = child.tab.linkedBrowser.currentURI.spec;
>   return url.replace("about:blank#", "");
> }).join(",");

@@ +65,5 @@
> +    is(aCW.GroupItems.groupItems.length, 3, "Validate group count in test 3.");
> +
> +    // Test 4: Move the fifth tab back into the group, on the second row.
> +    // Wait for the tab to resize, as it does after we move it out of the group.
> +    window.setTimeout(function() {

This doesn't work locally for me because it's still unreliable. We can't expect the transition to have ended when our timeout gets called. As we use css transitions to animate the group resizing you can do the following:

> let groupContainer = tab.parent.container;
> groupContainer.addEventListener("transitionend", function onTransitionEnd() {
>   groupContainer.removeEventListener("transitionend", onTransitionEnd);
>   /* ... validate tab positions and finish ... */
> });

@@ +73,5 @@
> +    }, 0);
> +  }
> +
> +  function createGroup() {
> +    let cw = TabView.getContentWindow();

function createGroup(win) {
  registerCleanupFunction(function () win.close());
  let cw = win.TabView.getContentWindow();

@@ +87,5 @@
> +
> +    moveTabs(group, cw);
> +  }
> +
> +  showTabView(createGroup);

newWindowWithTabView(createGroup);
Comment 20 Andres Hernandez [:andreshm] 2012-07-11 16:04:14 PDT
Created attachment 641238 [details] [diff] [review]
Patch v2

Updated patch.
Comment 21 Treeherder Robot 2012-07-18 07:05:09 PDT
edmorley
https://tbpl.mozilla.org/php/getParsedLog.php?id=13634936&tree=Mozilla-Aurora
Rev3 MacOSX Leopard 10.5.8 mozilla-aurora opt test mochitest-other on 2012-07-18 05:55:17
slave: talos-r3-leopard-040

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 22 Tim Taubert [:ttaubert] 2012-07-26 02:26:24 PDT
Comment on attachment 641238 [details] [diff] [review]
Patch v2

Review of attachment 641238 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you! r=me with the little fix below.

::: browser/components/tabview/test/browser_tabview_bug587503.js
@@ +108,5 @@
> +function waitForTransition(aTab, aCallback) {
> +  let groupContainer = aTab.parent.container;
> +  groupContainer.addEventListener("transitionend", function onTransitionEnd() {
> +    groupContainer.removeEventListener("transitionend", onTransitionEnd);
> +    if (aCallback) {

We don't need to check for aCallback here as there's only one consumer that passes a callback - so the second argument is required.
Comment 23 Andres Hernandez [:andreshm] 2012-07-26 10:58:13 PDT
Created attachment 646213 [details] [diff] [review]
Patch v3

Fixed issue.
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=3debd6feaa81
Comment 24 Andres Hernandez [:andreshm] 2012-07-26 15:43:29 PDT
Try finished with some oranges: https://tbpl.mozilla.org/?tree=Try&rev=3debd6feaa81
Comment 25 Tim Taubert [:ttaubert] 2012-07-26 16:00:43 PDT
Win opt shows this:

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | Validate tab positions in test 4. - Got 0,6,2,3,5,1,4, expected 0,6,2,3,4,5,1 

Looks like the test is flaky again :/
Comment 26 Andres Hernandez [:andreshm] 2012-07-27 15:07:35 PDT
Created attachment 646737 [details] [diff] [review]
Patch v4

I changed the last test to move the tab to the desired tab position instead of the group center. 

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=cf634618f129
Comment 27 Treeherder Robot 2012-07-30 06:42:10 PDT
edmorley
https://tbpl.mozilla.org/php/getParsedLog.php?id=13967685&tree=Mozilla-Aurora
Rev3 MacOSX Leopard 10.5.8 mozilla-aurora opt test mochitest-other on 2012-07-30 05:42:34
slave: talos-r3-leopard-006

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
Comment 28 Andres Hernandez [:andreshm] 2012-07-31 07:25:25 PDT
Try fail again on the last test. When moving the tab back to the group center. 

https://tbpl.mozilla.org/?tree=Try&rev=a97968e46315

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | Validate tab positions in test 4. - Got 0,6,2,3,5,1,4, expected 0,6,2,3,4,5,1
Comment 29 Tim Taubert [:ttaubert] 2012-07-31 08:34:01 PDT
Comment on attachment 646737 [details] [diff] [review]
Patch v4

Clearing review until you got that orange sorted out.
Comment 30 Treeherder Robot 2012-07-31 09:33:50 PDT
edmorley
https://tbpl.mozilla.org/php/getParsedLog.php?id=14002226&tree=Mozilla-Aurora
Rev3 MacOSX Leopard 10.5.8 mozilla-aurora opt test mochitest-other on 2012-07-31 05:39:34
slave: talos-r3-leopard-045

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/tabview/test/browser_tabview_bug587503.js | In the end, the group was showing a dropSpace - Got false, expected true
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_createRemote.js | Test timed out
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_createRemote.js | Found a  after previous test timed out
Comment 31 Tim Taubert [:ttaubert] 2012-08-01 05:19:45 PDT
I think the solution here is to make "waitForTransition" look like this:

> function waitForTransition(aTab, aCallback) {
>   let groupContainer = aTab.parent.container;
>   groupContainer.addEventListener("transitionend", function onTransitionEnd(aEvent) {
>     if (aEvent.target == groupContainer) {
>       groupContainer.removeEventListener("transitionend", onTransitionEnd);
>       executeSoon(aCallback);
>     }
>   });
> }

I checked locally and the .resizer element does indeed send a transitionend event as well. That might be what causes the intermittent orange. Also we should probably use executeSoon() to make sure the transition has ended here and other listeners have done their work.
Comment 32 Tim Taubert [:ttaubert] 2012-08-01 05:21:38 PDT
Try seems to agree:

https://tbpl.mozilla.org/?tree=Try&rev=a232603371b8
Comment 33 Andres Hernandez [:andreshm] 2012-08-01 09:42:33 PDT
Created attachment 647994 [details] [diff] [review]
Patch v5

Great! I just updated the patch.
Comment 34 Tim Taubert [:ttaubert] 2012-08-02 02:44:00 PDT
Comment on attachment 647994 [details] [diff] [review]
Patch v5

Review of attachment 647994 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Comment 35 Tim Taubert [:ttaubert] 2012-08-13 11:43:29 PDT
https://hg.mozilla.org/integration/fx-team/rev/84324c323762
Comment 36 Tim Taubert [:ttaubert] 2012-08-13 21:20:39 PDT
https://hg.mozilla.org/mozilla-central/rev/84324c323762

Note You need to log in before you can comment on or make changes to this bug.