1563500 - JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 2440: TypeError: tabbrowser.getTabForBrowser is not a function

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Seen in various reftests as run via web-platform-tests like:

https://treeherder.mozilla.org/logviewer.html#?job_id=254712754&repo=mozilla-central
https://taskcluster-artifacts.net/XHgmUfhFSaeC5s_ZRx6NhA/0/public/logs/live_backing.log

Relevant code, which is part of onMayChangeProcess:

https://searchfox.org/mozilla-central/rev/11712bd3ce7454923e5931fa92eaf9c01ef35a0a/browser/components/sessionstore/SessionStore.jsm#2440

I assume this has been caused by a new feature and is not a regression.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:mikedeboer, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Nika Layzell [:nika] (ni? for response)]

This seems relatively easy to fix, the check on the line before for tabbrowser could be extended to also check the getTabForbrowser method also exists.

Looking at the linked log, I see that there are a lot of assertions around gBrowser not looking like expected as well coming from BrowserTabParent.jsm. ni? :mconley in case they know why gBrowser would be in a "broken" state during this test.

06:27:59     INFO - PID 6480 | JavaScript error: resource:///actors/BrowserTabParent.jsm, line 27: TypeError: gBrowser.announceWindowCreated is not a function
06:27:59     INFO - PID 6480 | JavaScript error: , line 0: NS_ERROR_UNEXPECTED:
06:27:59     INFO - PID 6480 | 1562221679654	Marionette	INFO	Testing http://web-platform.test:8000/2dcontext/compositing/canvas_compositing_globalcompositeoperation_001.htm == http://web-platform.test:8000/2dcontext/compositing/canvas_compositing_globalcompositeoperation_001-ref.htm
06:27:59     INFO - PID 6480 | JavaScript error: resource:///actors/BrowserTabParent.jsm, line 27: TypeError: gBrowser.announceWindowCreated is not a function
06:27:59     INFO - PID 6480 | JavaScript error: , line 0: NS_ERROR_UNEXPECTED:
06:27:59     INFO - PID 6480 | JavaScript error: resource:///actors/BrowserTabParent.jsm, line 32: TypeError: browser.ownerGlobal.gBrowserInit is undefined
06:27:59     INFO - PID 6480 | JavaScript error: , line 0: NS_ERROR_UNEXPECTED:
06:27:59     INFO - PID 6480 | JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 2440: TypeError: tabbrowser.getTabForBrowser is not a function
06:27:59     INFO - PID 6480 | JavaScript error: resource:///actors/BrowserTabParent.jsm, line 27: TypeError: gBrowser.announceWindowCreated is not a function
06:27:59     INFO - PID 6480 | JavaScript error: , line 0: NS_ERROR_UNEXPECTED:
06:27:59     INFO - PID 6480 | JavaScript error: resource:///actors/BrowserTabParent.jsm, line 32: TypeError: browser.ownerGlobal.gBrowserInit is undefined
06:27:59     INFO - PID 6480 | JavaScript error: , line 0: NS_ERROR_UNEXPECTED:
06:28:00     INFO - PID 6480 | JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 2440: TypeError: tabbrowser.getTabForBrowser is not a function
06:28:00     INFO - PID 6480 | JavaScript error: resource:///actors/BrowserTabParent.jsm, line 27: TypeError: gBrowser.announceWindowCreated is not a function
06:28:00     INFO - PID 6480 | JavaScript error: , line 0: NS_ERROR_UNEXPECTED:
06:28:00     INFO - PID 6480 | JavaScript error: resource:///actors/BrowserTabParent.jsm, line 32: TypeError: browser.ownerGlobal.gBrowserInit is undefined
06:28:00     INFO - PID 6480 | JavaScript error: , line 0: NS_ERROR_UNEXPECTED:

Comment 3

[Mike Conley (:mconley) (:⚙️)]

I think a lot of those errors were fixed by the patch in bug 1561970 (see bug 1563495 and bug 1563498).

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Note that this failure is still present in recent mozilla-central builds like:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=258720683&repo=mozilla-central&lineNumber=1017

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Hi Mike, any thoughts on newer failures? Also, do we know if there's any real-world impact from these errors? Lost of session or anything?

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1563500 - Avoid accessing bogus reftest gBrowser in SessionStore.onMayChangeProcess. r?nika

Comment 7

[Mike Conley (:mconley) (:⚙️)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #5)

Hi Mike, any thoughts on newer failures? Also, do we know if there's any real-world impact from these errors? Lost of session or anything?

No, I think we're only hitting this because the refTest harness spoofs the gBrowser on a browser element to point to another browser:

https://searchfox.org/mozilla-central/rev/b38e3beb658b80e1ed03e0fdf64d225bd4a40327/testing/marionette/reftest.js#187

It's doing this for reasons that are beyond me. I've attached a patch that causes the SessionStore code to use the <browser>.getTabBrowser() method to avoid the spoof. I really wonder why refTest needs to spoof it though. shrug

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #7)

No, I think we're only hitting this because the refTest harness spoofs the gBrowser on a browser element to point to another browser:

https://searchfox.org/mozilla-central/rev/b38e3beb658b80e1ed03e0fdf64d225bd4a40327/testing/marionette/reftest.js#187

It's doing this for reasons that are beyond me. I've attached a patch that causes the SessionStore code to use the <browser>.getTabBrowser() method to avoid the spoof. I really wonder why refTest needs to spoof it though. shrug

James, do you have an idea why this code is used?

Comment 9

[James Graham [:jgraham]]

Essentially because the reftests run in a pretty strange window without the normal chrome (no tabs etc.) We need a reference to the browser element to call getBoundingClientRect on it (although we could probably call getElementsByTagNameNs or something). The specific approach of setting gBrowser on the window to point at the Browser element was stolen from the reftest harness, but it looks like that code subsequently changed to just stash the browser in a global.

So, we can probably change the details here if necessary.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6395e0de040
Avoid accessing bogus reftest gBrowser in SessionStore.onMayChangeProcess. r=nika

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

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