903274 - this.updateDisplay is not a function at chrome://browser/content/search/search.xml:79 from every window during tpaint and tspaint

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Matthew N. [:MattN]]

21:20:44     INFO -  [Exception... "'[JavaScript Error: "this.updateDisplay is not a function" {file: "chrome://browser/content/search/search.xml" line: 79}]' when calling method: [nsIBrowserSearchInitObserver::onInitComplete]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: resource://gre/components/nsSearchService.js :: onSuccess :: line 3216"  data: yes]

This seems to happen on most, if not all, windows during tpaint talos runs. I think the error happens because the callback is being called after the binding is destroyed.

Log: https://tbpl.mozilla.org/php/getParsedLog.php?id=26337997&tree=Try&full=1

I think this is mostly just log spew and probably doesn't have much impact elsewhere but correct me if I'm wrong.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

I agree that this is probably harmless.

Can we set a "_destroyed" expando on the node from the XBL destructor, and then check it in the initialization callback? Failing that, we could just null check this.updateDisplay.

Comment 2

[Chris Peterson [:cpeterson]]

I see this same JavaScript error every time I run a debug Firefox build if I have customized Australis to remove the search bar.

************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "this.updateDisplay is not a function" {file: "chrome://browser/content/search/search.xml" line: 83}]' when calling method: [nsIBrowserSearchInitObserver::onInitComplete]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: file:///Users/cpeterson/Code/mozilla/inbound/objdir-mac/dist/NightlyDebug.app/Contents/MacOS/components/nsSearchService.js :: onSuccess :: line 3720"  data: yes]
************************************************************

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to Chris Peterson (:cpeterson) from comment #2)
> I see this same JavaScript error every time I run a debug Firefox build if I
> have customized Australis to remove the search bar.

That's strange. Search bar binding code shouldn't be running at all in that case...

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Oh, unless you meant that you moved the search bar into the Australis menu panel - I see this bug then as well.

Comment 5

[Chris Peterson [:cpeterson]]

I see this error when I move the search bar into the Australis menu panel, but I confirmed that I also see this error when I remove the search bar.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Oh, I suppose the search bar is in the document to start with, until the customizable toolbar code removes it. It would be nice to find a way to avoid that somehow...

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

This fixes it for me.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/integration/fx-team/rev/f3e7fa7d9748

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f3e7fa7d9748

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8357531 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: tbpl logspam on nearly every talos run
Testing completed: on m-c
Risk to taking this patch (and alternatives if risky): none, just an early return in the event of an already-destroyed binding
String or UUID changes made by this patch: none

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/016a7b78157a
https://hg.mozilla.org/releases/mozilla-beta/rev/ea46482ad289
https://hg.mozilla.org/releases/mozilla-esr24/rev/85745204e10f
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/c877921e74d3

Comment 13

[u279076]

Should this have in-testsuite tests?

Comment 14

[u279076]

Gavin, given in-testsuite-, is there any manual testing QA needs to do here?

Comment 15

[:Gavin Sharp [email: [e-mail]]]

no

Comment 16

[u279076]

Thanks Gavin, tagging this bug [qa-].

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Backed out from the branches for causing bug 960864.

https://hg.mozilla.org/releases/mozilla-aurora/rev/c2fef4e82f98
https://hg.mozilla.org/releases/mozilla-beta/rev/94895bef7698
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/f903e807a133
https://hg.mozilla.org/releases/mozilla-esr24/rev/9a9701aacae4