1859490 - "TypeError: can't access property "title", tab is undefined" due to tabManager.getWrapper not finding a wrapper

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect)

Description

[Henri Sivonen (:hsivonen)]

1. Compile with the patch from bug 543435
2. Run ./mach test dom/base/test/browser_bug1058164.js

The test times out due to:

 0:36.67 INFO Console message: [JavaScript Error: "TypeError: can't access property "title", tab is undefined" {file: "chrome://browser/content/parent/ext-tabs.js" line: 468}]
listener@chrome://browser/content/parent/ext-tabs.js:468:11
_tabAttrModified@chrome://browser/content/tabbrowser.js:1562:12
_setTabLabel@chrome://browser/content/tabbrowser.js:1744:14
setTabTitle@chrome://browser/content/tabbrowser.js:1690:19
swapBrowsersAndCloseOther@chrome://browser/content/tabbrowser.js:4612:12
swapBrowsers@chrome://browser/content/browser.js:1766:20
EventListener.handleEvent*onLoad@chrome://browser/content/browser.js:1778:25
EventHandlerNonNull*@chrome://browser/content/browser.xhtml:119:39

What mechanism is supposed to populate the wrapper map upon tab swap and why might that not happen before this line runs?

Comment 1

[Henri Sivonen (:hsivonen)]

browser/base/content/test/tabs/browser_adoptTab_failure.js appears to fail for the same reason.

Comment 2

[Henri Sivonen (:hsivonen)]

browser/components/extensions/test/browser/browser_ext_sessions_getRecentlyClosed_tabs.js is similar but not the same.

Comment 3

[Rob Wu [:robwu]]

FYI I'm compiling with the patch from bug 543435 to look into this issue (and bug 1792685). I'll get back to you by next week.

Comment 4

[Rob Wu [:robwu]]

Results below obtained with the nov 3 version of the patch, at https://phabricator.services.mozilla.com/D155376?id=782938

2. Run ./mach test dom/base/test/browser_bug1058164.js

Test passes for me. I do see the reported error message in the console though.

(In reply to Henri Sivonen (:hsivonen) from comment #1)

browser/base/content/test/tabs/browser_adoptTab_failure.js appears to fail for the same reason.

I also see the same "error message" without the patch. When I run the test with the patch, I see the following error messages, and there is no obvious relation between these messages and the referenced extension code:

browser/base/content/test/tabs/browser_adoptTab_failure.js
  FAIL undefined assertion name - Got 3, expected 2
Stack trace:
chrome://mochikit/content/browser-test.js:test_is:1611
chrome://mochitests/content/browser/browser/base/content/test/tabs/browser_adoptTab_failure.js:test_replaceTabsWithWindow:47
chrome://mochikit/content/browser-test.js:handleTask:1134
chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1206
chrome://mochikit/content/browser-test.js:Tester_execTest:1348
chrome://mochikit/content/browser-test.js:nextTest/<:1123
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1058
  FAIL undefined assertion name - Got "data:text/plain,nonAdoptableTab", expected "data:text/plain,auxiliaryTab"
Stack trace:
chrome://mochikit/content/browser-test.js:test_is:1611
chrome://mochitests/content/browser/browser/base/content/test/tabs/browser_adoptTab_failure.js:test_replaceTabsWithWindow:48
chrome://mochikit/content/browser-test.js:handleTask:1134
chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1206
chrome://mochikit/content/browser-test.js:Tester_execTest:1348
chrome://mochikit/content/browser-test.js:nextTest/<:1123
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1058
  FAIL undefined assertion name - Got "data:text/plain,auxiliaryTab", expected "data:text/plain,selectedTab"
Stack trace:
chrome://mochikit/content/browser-test.js:test_is:1611
chrome://mochitests/content/browser/browser/base/content/test/tabs/browser_adoptTab_failure.js:test_replaceTabsWithWindow:49
chrome://mochikit/content/browser-test.js:handleTask:1134
chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1206
chrome://mochikit/content/browser-test.js:Tester_execTest:1348
chrome://mochikit/content/browser-test.js:nextTest/<:1123
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1058

I'm going to take a further look at the specific console message reported in the bug, but from what I have seen so far, there is no relation between the error message from extension code, and the observed test failures. Can you confirm whether my assessment is correct? And if it is not, some further details on why your observed failures are caused by extension code?

Comment 5

[Rob Wu [:robwu]]

Attachment: Bug 1859490 - Ignore tab events when isAdoptingTab

While a tab is being adopted, it may trigger tab events. This currently
triggers errors in the console becausetabManager.getWrapper returns null
in this situation, and the next line dereferences the return value
without null-checking.

To prevent this error from happening, exit early in this situation. This
avoids logspam whenever swapBrowsersAndCloseOther is called before
_clearTabToAdopt, at https://searchfox.org/mozilla-central/rev/76b13c585e575cf273fd05ab3b48a1532c1d1faa/browser/base/content/browser.js#1766-1772

Once _clearTabToAdopt is called, isAdoptingTab() becomes false.

Comment 6

[Henri Sivonen (:hsivonen)]

Confirming that this addresses the ext-tabs.js problem. Thank you. I'll take a new look at the other failure mode, so keeping myself needinfoed.

Comment 7

[Rob Wu [:robwu]]

Thanks for confirming. Given the issue and my patch, I'll update the title to more closely reflect what's going on. I recommend to file separate bugs for other issues.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6baf5fcef2d5
Ignore tab events when isAdoptingTab r=zombie

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6baf5fcef2d5