1664522 - tabs.captureVisibleTab() fails with "this.nativeTab.ownerGlobal is undefined"

Status: RESOLVED FIXED

(WebExtensions :: Android, defect, P2)

Description

[Dennis Schubert [:denschub]]

On a Gecko 82 build, calling tabs.captureVisibleTab() fails with the following exception:

[JavaScript Error: "this.nativeTab.ownerGlobal is undefined" {file: "chrome://extensions/content/parent/ext-tabs-base.js" line: 126}]
capture@chrome://extensions/content/parent/ext-tabs-base.js:126:45
captureVisibleTab@chrome://geckoview/content/ext-tabs.js:480:22
Async*recvAPICall/result</<@resource://gre/modules/ExtensionParent.jsm:951:68
withPendingBrowser@resource://gre/modules/ExtensionParent.jsm:489:26
recvAPICall/result<@resource://gre/modules/ExtensionParent.jsm:951:24
callAndLog@resource://gre/modules/ExtensionParent.jsm:913:14
recvAPICall@resource://gre/modules/ExtensionParent.jsm:950:25
Async*_recv@resource://gre/modules/ConduitsChild.jsm:78:20
receiveMessage@resource://gre/modules/ConduitsParent.jsm:371:20
JSActor query*_send@resource://gre/modules/ConduitsChild.jsm:63:11
_send@resource://gre/modules/ConduitsChild.jsm:111:18
callParentAsyncFunction@resource://gre/modules/ExtensionChild.jsm:857:18
callAsyncFunction@resource://gre/modules/ExtensionChild.jsm:621:33
stub@resource://gre/modules/Schema

This works on a beta (81) build, and :agi mentioned that there have been some code changes to that API recently, so this is a new regression.

Comment 1

[[ex-Mozilla] Agi Sferro | :agi]

I think all we need to do here is providing a getter for ownerGlobal here: https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewTab.jsm#29

Comment 2

[Tomislav Jovanovic :zombie]

Oops, sorry about that. The nativeGlobal there is the chrome window on Desktop, not sure what it would be on android.

This was regressed by the first patch from bug 1636508 that landed 17 days ago -- the line from the call stack is now a comment after the second patch landed yesterday.

Another thing that might be different on Fenix is the ZoomManager, not sure if that's fully wired up on android. Also, I was updating mobile tests, expecting they were running and actually testing things :/ could you please also verify what's going on with that?

Let me know if I can be of any help!

Comment 3

[[ex-Mozilla] Agi Sferro | :agi]

No worries!

this.browser.ownerGlobal is the equivalent for mobile.

ZoomManager is not available yeah, I think we can find a replacement for it, I'll put up a patch soon.

Comment 4

[[ex-Mozilla] Agi Sferro | :agi]

The test passes somehow 🤷‍♂️ looking into what's going on there

Comment 5

[[ex-Mozilla] Agi Sferro | :agi]

I think this patch might have broken the test: https://hg.mozilla.org/mozilla-central/rev/56c287941f2a60a7ea18baf45ffc526c4cd137e2

Comment 6

[[ex-Mozilla] Agi Sferro | :agi]

Ok so it turns out getting screenshots of desktop viewport pages is broken, but mobile viewport works as expected. Since the mobile viewport case is more important I just opened Bug 1664522

Comment 7

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1664522 - Fix tabs.captureVisibleTab for mobile.

There are two things wrong with captureVisibleTab on mobile:

• ownerGlobal is not available on nativeTab, this patch adds a new getter that
  returns the browser's ownerGlobal.
• ZoomManager is not available on mobile, this patch adds a fallback.

The test was also not actually running due to a refuse in
https://hg.mozilla.org/mozilla-central/rev/56c287941f2a60a7ea18baf45ffc526c4cd137e2,
this patch fixes that by moving the runTest function to be async instead of a
generator.

While working on this patch I noticed that the captureVisibleTab API only works
for mobile viewports, so I added a test and opened a bug for that.

Comment 8

[Rob Wu [:robwu]]

(In reply to Agi Sferro | :agi | ⏰ PST | he/him from comment #6)

Ok so it turns out getting screenshots of desktop viewport pages is broken, but mobile viewport works as expected. Since the mobile viewport case is more important I just opened Bug 1664522

bug 1664594 *

Comment 9

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

Set release status flags based on info from the regressing bug 1636508

Comment 10

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1664522 - Implement tabs.captureTab on mobile.

Comment 11

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1664522 - Fix tabs.captureVisibleTab for mobile.

There are two things wrong with captureVisibleTab on mobile:

* ownerGlobal is not available on nativeTab, this patch uses
  this.browser.ownerGlobal instead.
* ZoomManager is not available on mobile, this patch moves that code to
  browser/../ext-tabs.

Comment 12

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1664522 - Move ext_tabs_captureTab test to toolkit.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c951834c99c7
Implement tabs.captureTab on mobile. r=snorp
https://hg.mozilla.org/integration/autoland/rev/04b61d3d734a
Fix tabs.captureVisibleTab for mobile. r=robwu,zombie,snorp
https://hg.mozilla.org/integration/autoland/rev/30b8a0e3514b
Move ext_tabs_captureTab test to toolkit. r=robwu,snorp

Comment 14

[Dorel Luca [:dluca]]

Backed out 3 changesets (bug 1664522) for Mochitest failures in extensions/test/mochitest/test_ext_tabs_captureTab.html. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=315876703&repo=autoland&lineNumber=14574

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=30b8a0e3514b302ff26436090835422f32c3e369

Backout:
https://hg.mozilla.org/integration/autoland/rev/a31efbcf3ff5bb7a0c0c35f9f4f09f2b2db73a6a

Comment 15

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/515cce30a345
Backed out 3 changesets for Mochitest failures in extensions/test/mochitest/test_ext_tabs_captureTab.html. CLOSED TREE

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af27b96245a0
Implement tabs.captureTab on mobile. r=snorp
https://hg.mozilla.org/integration/autoland/rev/810036a8a9f7
Fix tabs.captureVisibleTab for mobile. r=robwu,zombie,snorp
https://hg.mozilla.org/integration/autoland/rev/ad4be03cc660
Move ext_tabs_captureTab test to toolkit. r=robwu,snorp

Comment 17

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/af27b96245a0
https://hg.mozilla.org/mozilla-central/rev/810036a8a9f7
https://hg.mozilla.org/mozilla-central/rev/ad4be03cc660

Comment 18

[[ex-Mozilla] Agi Sferro | :agi]

Thanks.