Closed Bug 951142 Opened 6 years ago Closed 6 years ago

Intermittent browser_browserDrop.js | uncaught exception - TypeError: this.close is not a function at chrome://global/content/bindings/findbar.xml:1168 | Test timed out

Categories

(Firefox :: General, defect)

x86_64
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 29
Tracking Status
firefox27 --- fixed
firefox28 --- fixed
firefox29 --- fixed
firefox-esr24 --- fixed
b2g-v1.2 --- fixed
b2g-v1.3 --- fixed

People

(Reporter: RyanVM, Assigned: mikedeboer)

References

(Depends on 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

https://tbpl.mozilla.org/php/getParsedLog.php?id=32052526&tree=Fx-Team

Rev5 MacOSX Mountain Lion 10.8 fx-team opt test mochitest-browser-chrome on 2013-12-16 15:40:05 PST for push c12ebde00d15
slave: talos-mtnlion-r5-038

16:08:53     INFO -  TEST-START | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js
16:08:53  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | uncaught exception - TypeError: this.close is not a function at chrome://global/content/bindings/findbar.xml:1168
16:08:53     INFO -  Stack trace:
16:08:53     INFO -      JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 1216
16:08:53     INFO -      native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
16:08:53     INFO -  JavaScript error: chrome://global/content/bindings/findbar.xml, line 1168: this.close is not a function
16:08:53  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | uncaught exception - TypeError: this.close is not a function at chrome://global/content/bindings/findbar.xml:1168
16:08:53     INFO -  Stack trace:
16:08:53     INFO -      JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 1216
16:08:53     INFO -      native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
16:08:53     INFO -  JavaScript error: chrome://global/content/bindings/findbar.xml, line 1168: this.close is not a function
16:08:53  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | uncaught exception - TypeError: this.close is not a function at chrome://global/content/bindings/findbar.xml:1168
16:08:53     INFO -  Stack trace:
16:08:53     INFO -      JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 1216
16:08:53     INFO -      native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
16:08:53     INFO -  JavaScript error: chrome://global/content/bindings/findbar.xml, line 1168: this.close is not a function
16:08:53  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | uncaught exception - TypeError: this.close is not a function at chrome://global/content/bindings/findbar.xml:1168
16:08:53     INFO -  Stack trace:
16:08:53     INFO -      JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 1216
16:08:53     INFO -      native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
16:08:53     INFO -  JavaScript error: chrome://global/content/bindings/findbar.xml, line 1168: this.close is not a function
16:08:53  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | uncaught exception - TypeError: this.close is not a function at chrome://global/content/bindings/findbar.xml:1168
16:08:53     INFO -  Stack trace:
16:08:53     INFO -      JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 1216
16:08:53     INFO -      native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
16:08:53     INFO -  JavaScript error: chrome://global/content/bindings/findbar.xml, line 1168: this.close is not a function
16:08:53  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | uncaught exception - TypeError: this.close is not a function at chrome://global/content/bindings/findbar.xml:1168
16:08:53     INFO -  Stack trace:
16:08:53     INFO -      JS frame :: chrome://mochikit/content/tests/SimpleTest/SimpleTest.js :: simpletestOnerror :: line 1216
16:08:53     INFO -      native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
16:08:53     INFO -  JavaScript error: chrome://global/content/bindings/findbar.xml, line 1168: this.close is not a function
16:08:53     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | Console message: [JavaScript Error: "TypeError: this.close is not a function" {file: "chrome://global/content/bindings/findbar.xml" line: 1168}]
16:08:53     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | Console message: [JavaScript Error: "TypeError: this.close is not a function" {file: "chrome://global/content/bindings/findbar.xml" line: 1168}]
16:08:53     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | Console message: [JavaScript Error: "TypeError: this.close is not a function" {file: "chrome://global/content/bindings/findbar.xml" line: 1168}]
16:08:53     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | Console message: [JavaScript Error: "TypeError: this.close is not a function" {file: "chrome://global/content/bindings/findbar.xml" line: 1168}]
16:08:53     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | Console message: [JavaScript Error: "TypeError: this.close is not a function" {file: "chrome://global/content/bindings/findbar.xml" line: 1168}]
16:08:53     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | Console message: [JavaScript Error: "TypeError: this.close is not a function" {file: "chrome://global/content/bindings/findbar.xml" line: 1168}]
16:09:24     INFO -  libpng warning: zero length keyword
16:09:24     INFO -  libpng warning: Empty language field in iTXt chunk
16:09:29     INFO -  SCREENSHOT: <see log>
16:09:29  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | Test timed out
16:09:29     INFO -  TEST-INFO | MEMORY STAT vsize after test: 4552404992
16:09:29     INFO -  TEST-INFO | MEMORY STAT residentFast after test: 865214464
16:09:29     INFO -  TEST-INFO | MEMORY STAT heapAllocated after test: 190782816
16:09:29     INFO -  INFO TEST-END | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_browserDrop.js | finished in 30059ms
In bug 899286 I fixed a similar issue where I could circumvent putting a conditional before the action. Bug 926897 is VERY similar.

I could do that here and prevent future issues like this (see http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/findbar.xml#1172); `if (this.close) this.close();`

However, the issue seems to be that the findbar receives a keypress event even though it's not initialized and opened at all. The event handler phase is set to 'capturing'. This can be due to bug 537013, which moved the findbar into the tabbrowser, thus making it the first target handler during event capture phase for VK_ESCAPE key events originating from elements lower down the DOM tree.

IMO, the findbar ESC key event handler is set to be too greedy by using the capturing phase.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
See Also: → 926897
Taking this as the first route, because I think that changing the event phase has too much potential impact on other toolkit consumers.

Try push: https://tbpl.mozilla.org/?tree=Try&rev=689539f1587a
Attachment #8348777 - Flags: review?(dao)
The underlying issue here is that someone is trying to call this.close when the binding has been torn down. The XBL handler should be removed during binding teardown, so I'm skeptical that that patch would fix anything. But there is another this.close() caller in handleEvent, I bet it's the one throwing this error. Shouldn't we remove the keypress and mouseup handlers added by the "browser" setter in this binding's destructor?
(and I assume bug 926897 is the same bug, just in a different test/different platform)
Blocks: 926897
Attachment #8348777 - Flags: review?(dao) → review-
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #3)
> The underlying issue here is that someone is trying to call this.close when
> the binding has been torn down. The XBL handler should be removed during
> binding teardown, so I'm skeptical that that patch would fix anything. But
> there is another this.close() caller in handleEvent, I bet it's the one
> throwing this error.

I don't think so, as the error "uncaught exception - TypeError: this.close is not a function at chrome://global/content/bindings/findbar.xml:1168" definitively points to the call in the handler element.

> Shouldn't we remove the keypress and mouseup handlers
> added by the "browser" setter in this binding's destructor?

We should. Will that also prevent event handlers defined in <handler> elements to be disabled?
Flags: needinfo?(gavin.sharp)
Flags: needinfo?(dao)
(In reply to Mike de Boer [:mikedeboer] from comment #5)
> > Shouldn't we remove the keypress and mouseup handlers
> > added by the "browser" setter in this binding's destructor?
> 
> We should. Will that also prevent event handlers defined in <handler>
> elements to be disabled?

...but this is already done by setting the browser property to `null` at http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/findbar.xml#379

So handleEvent will not be called anymore after the findbar destructor is invoked. I still think that somehow the capturing event handler is active and causes this orange.
Indeed, you are right!

This is mysterious, and I'd like to understand the underlying issue, but I won't have a chance to look into it further and I suppose it's not necessary to block on that.
Flags: needinfo?(gavin.sharp)
Comment on attachment 8348777 [details] [diff] [review]
Patch 1: check for a close method to be present on the binding before invoking it

At the very least there should be a comment explaining why the null check is there (wallpaper over some unknown issue) and a followup bug filed, though.
Attachment #8348777 - Flags: review-
Flags: needinfo?(dao)
Depends on: 957999
Comment on attachment 8357688 [details] [diff] [review]
Patch 1: check for a close method to be present on the binding before invoking it

Thanks!
Attachment #8357688 - Flags: review?(gavin.sharp) → review+
https://hg.mozilla.org/mozilla-central/rev/113e065a2803
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Duplicate of this bug: 926897
This applies cleanly or with minor unbitrotting all the way down to esr24. Is there any reason we can't take this patch in the name of fewer intermittents?
Flags: needinfo?(gavin.sharp)
no
Flags: needinfo?(gavin.sharp)
Comment on attachment 8357688 [details] [diff] [review]
Patch 1: check for a close method to be present on the binding before invoking it

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: random oranges on TBPL
Testing completed: on m-c
Risk to taking this patch (and alternatives if risky): minimal, it just adds some extra protection against this.close being null
String or UUID changes made by this patch: none
Attachment #8357688 - Flags: approval-mozilla-esr24?
Attachment #8357688 - Flags: approval-mozilla-beta?
Attachment #8357688 - Flags: approval-mozilla-b2g26?
Attachment #8357688 - Flags: approval-mozilla-aurora?
Comment on attachment 8357688 [details] [diff] [review]
Patch 1: check for a close method to be present on the binding before invoking it

Approving on all branches to avoid TBPL failures.
Attachment #8357688 - Flags: approval-mozilla-esr24?
Attachment #8357688 - Flags: approval-mozilla-esr24+
Attachment #8357688 - Flags: approval-mozilla-beta?
Attachment #8357688 - Flags: approval-mozilla-beta+
Attachment #8357688 - Flags: approval-mozilla-b2g26?
Attachment #8357688 - Flags: approval-mozilla-b2g26+
Attachment #8357688 - Flags: approval-mozilla-aurora?
Attachment #8357688 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.