Defect - Intermittent browser_topsites.js | This test exceeded the timeout threshold. (this.activeTileset.clearSelection is not a function)

RESOLVED FIXED in Firefox 25

Status

Firefox for Metro
Tests
P2
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: RyanVM, Assigned: sfoster)

Tracking

({intermittent-failure})

unspecified
Firefox 25
x86
Windows 8.1
intermittent-failure
Dependency tree / graph

Firefox Tracking Flags

(firefox24 unaffected, firefox25 fixed)

Details

(Whiteboard: feature=defect c=testing u=developer p=1)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=25888063&tree=Mozilla-Inbound

WINNT 6.2 mozilla-inbound opt test mochitest-metro-chrome on 2013-07-29 17:54:20 PDT for push fed05531f9e3
slave: t-w864-ix-049

18:08:56     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Brian site was blocked
18:08:56     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Dougal site was blocked
18:08:56     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Gris selection is empty after deletion
18:08:56     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | First tile was restored to the grid
18:08:56     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | 2nd tile was restored to the grid
18:08:58     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | 2 tiles are still selected
18:08:58     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Brian is still selected
18:08:58     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Dougal is still selected
18:08:58     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Brian tile is still pinned
18:08:58     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Brian site was unblocked
18:08:58     INFO -  TEST-PASS | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Dougal site was unblocked
18:08:58     INFO -  TEST-INFO | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | END delete and restore site tiles
18:08:58     INFO -  TEST-INFO | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Console message: [JavaScript Error: "this.activeTileset.clearSelection is not a function" {file: "chrome://browser/content/appbar.js" line: 239}]
18:08:58     INFO -  TEST-INFO | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Console message: [JavaScript Error: "this.activeTileset.clearSelection is not a function" {file: "chrome://browser/content/appbar.js" line: 239}]
18:08:58     INFO -  TEST-INFO | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | Console message: [JavaScript Error: "this.activeTileset.clearSelection is not a function" {file: "chrome://browser/content/appbar.js" line: 239}]
18:08:58  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort.
18:08:58     INFO -  INFO TEST-END | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_topsites.js | finished in 52517ms

Updated

5 years ago
Blocks: 880298

Updated

5 years ago
Summary: Intermittent browser_topsites.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort. → Intermittent browser_topsites.js | This test exceeded the timeout threshold. (this.activeTileset.clearSelection is not a function)
(Assignee)

Comment 1

5 years ago
I'll take this, I think I see what's going on
Assignee: nobody → sfoster
(Assignee)

Comment 2

5 years ago
Created attachment 783229 [details] [diff] [review]
Ensure activeTileset is bound before calling methods on it

Its possible for the activeTileset richgrid element reference that appbar keeps to become unbound. Particularly in a test scenario where we are building up and tearing down UI. Then the activeTileset would still test truthy but the clearSelection method wouldn't be defined. I've added guards here to head this off.
Attachment #783229 - Flags: review?(jmathies)
(Assignee)

Comment 3

5 years ago
p=1 btw.

Updated

5 years ago
Blocks: 893311, 865451
Priority: -- → P2
QA Contact: jbecerra
Summary: Intermittent browser_topsites.js | This test exceeded the timeout threshold. (this.activeTileset.clearSelection is not a function) → Defect - Intermittent browser_topsites.js | This test exceeded the timeout threshold. (this.activeTileset.clearSelection is not a function)
Whiteboard: feature=defect c=testing u=developer p=1

Updated

5 years ago
Status: NEW → ASSIGNED

Updated

5 years ago
Attachment #783229 - Attachment is patch: true
Attachment #783229 - Attachment mime type: message/rfc822 → text/plain

Comment 4

5 years ago
Comment on attachment 783229 [details] [diff] [review]
Ensure activeTileset is bound before calling methods on it

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

::: browser/metro/base/content/appbar.js
@@ +35,5 @@
>          this.update();
>          break;
>  
>        case 'MozAppbarDismissing':
> +        if (this.activeTileset && ('isBound' in this.activeTileset)) {

Can "('isBound' in this.activeTileset)" be a getter on activeTileset since you use it in a couple places?

@@ +239,5 @@
> +    if (
> +        this.activeTileset &&
> +        ('isBound' in this.activeTileset) &&
> +        this.activeTileset !== activeTileset
> +    ) {

nit - funky formatting here
Attachment #783229 - Flags: review?(jmathies) → review+
(Assignee)

Comment 5

5 years ago
If the binding isn't applied, the isBound field will be undefined, its kind of a chicken and egg thing. So checking for its existence tells us what we need to know without the undefined property warning. 

https://hg.mozilla.org/integration/mozilla-inbound/rev/10730c6907fc
(Reporter)

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/10730c6907fc
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Reporter)

Comment 9

5 years ago
Two instances post-fix, both on t-w864-ix-049. hmm...
Is this required QA testing? If yes, how to test this?
Flags: needinfo?(ryanvm)
(Reporter)

Comment 11

5 years ago
You're better off asking the bug assignee on that. But to wager a guess, probably not since this was an intermittent test failure in our automation.
Flags: needinfo?(ryanvm) → needinfo?(sfoster)
(Reporter)

Updated

5 years ago
status-firefox24: --- → unaffected
status-firefox25: --- → fixed
(Assignee)

Comment 12

5 years ago
Do I gather this test has been failing since that fix landed? 

Samvedana, no (manual) QA needed on this one we just need the automated test run to pass reliably. Its likely a race-condition that only occurs during automated testing.
Flags: needinfo?(sfoster)
(Reporter)

Comment 13

5 years ago
(In reply to Sam Foster [:sfoster] from comment #12)
> Do I gather this test has been failing since that fix landed? 

Comment 8 and comment 9 show instances of this post-fix yes. But clearly the frequency is low if those are the only two.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.