1411351 - Console error "WeakMap key must be an object, got undefined" from isArticleChangeListener

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P2)

Description

[Andrew Swan [:aswan]]

I'm seeing this error in the browser console, it appears to happen every time a new tab is created:

WeakMap key must be an object, got undefined  ext-browser.js:271
setId chrome://browser/content/ext-browser.js:271:5
getId chrome://browser/content/ext-browser.js:251:5
wrapTab chrome://browser/content/ext-browser.js:926:47
TabManagerBase/this._tabs< chrome://extensions/content/ext-tabs-base.js:1637:44
get resource://gre/modules/ExtensionUtils.jsm:87:15
getWrapper chrome://extensions/content/ext-tabs-base.js:1708:12
isArticleChangeListener chrome://browser/content/ext-tabs.js:297:23
emit resource://gre/modules/ExtensionUtils.jsm:222:40
receiveMessage

Comment 1

[Bob Silverberg [:bsilverberg]]

Here's what I've discovered:

When isArticleChangeListener [1] fires, it receives a browser object in event.target which is meant to be the browser object for the tab in question. It then calls gBrowser.getTabForBrowser [2] to retrieve the corresponding nativeTab object. When getTabForBrowser goes to retrieve the tab from the this._tabForBrowser map, it does not find a key for this browser, so it returns undefined. It is this undefined value that is passed into tabManager.getWrapper [3] and eventually shows up as an error in setId [4].

Via debugging I have observed that the code in addTab in tab browser.xml [5] which adds a record for a new browser to the this._tabForBrowser map does fire before isArticleChangeListener fires, but still, when isArticleChangeListener fires, the record for the browser is not found in the map. In an effort to see whether the browser added at [5] is different from the browser received at [1], I recorded some values that looked unique. I’m not sure if these are actually usable for this purpose, but what I found is that the browser added at [5] contains the following properties:

_contentRequestContextID : 320082437734494
_innerWindowID : 2147483661
_outerWindowID : 2147483659

whereas the browser received at [1] contains these properties:

_contentRequestContextID : 320082437734518
_innerWindowID : 2147483664
_outerWindowID : 2147483662

Does this prove they are different browser objects and explain why the browser isn’t found in this._tabForBrowser at [2]?

Note that this only happens when a new tab is opened to about:newtab. It does not happen when any other tabs are opened, including other about: tabs (based on the few that I’ve checked, anyway). It also does not happen when navigating to about:newtab - only when opening a new tab. It also does not happen if the newtab page is overridden by an extension. It only happens when opening the actual Firefox New Tab page.

One possible fix for this is to check the return value of gBrowser.getTabForBrowser at [2] and bail out of the listener if it is undefined. Based on my testing, the only time that will happen is when the Firefox New Tab page is used to open a tab, so I think that’s fine.

I’m still not sure what is actually causing this issue, though, so maybe we don’t want to apply a band-aid like that to it.

Do you have any further thoughts, Andrew?

[1] http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/browser/components/extensions/ext-tabs.js#295
[2] http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/base/content/tabbrowser.xml#404
[3] http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/browser/components/extensions/ext-tabs.js#297
[4] http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/components/extensions/ext-browser.js#271
[5] http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/base/content/tabbrowser.xml#2771

Comment 2

[Andrew Swan [:aswan]]

progress on this happened on irc

Comment 3

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1411351 - Fix Console error "WeakMap key must be an object..." that happens when opening a new tab,

When the New Tab page is opened, the browser object that is passed into the isArticleChangeListener
is not a valid browser from the perspective of tabbrowser.xml, so it is not able to find it in its
_tabForBrowser map. This causes undefined to be passed into tabManager.getWrapper(), which causes
the error.

This patch fixes this by not calling tabManager.getWrapper(), and subsequently firing the onUpdate
event, if the return value from gBrowser.getTabForBrowser() is undefined. Testing shows that that
is only the case when the New Tab page is opened, and that page is not, and is unlikely to be
readerable any time in the foreseeable future (confirmed on IRC), so ignoring this case in the
listener is acceptable.

Review commit: https://reviewboard.mozilla.org/r/193512/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/193512/

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8922448 [details]
Bug 1411351 - Fix Console error "WeakMap key must be an object..." that happens when opening a new tab,

https://reviewboard.mozilla.org/r/193512/#review198748

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88a41df87dac
Fix Console error "WeakMap key must be an object..." that happens when opening a new tab, r=aswan

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/88a41df87dac

Comment 7

[Madalin Cotetiu]

Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.

Thanks!