1263167 - Complete test coverage for browser.extension.inIncognitoContext

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Description

[Bob Silverberg [:bsilverberg]]

Coverage is missing [1] for browser.extension.inIncognitoContext [2] for both the cases where it will return `true` and where it will return `false`.

Copying a conversation from bug 1190322:
----------------------------------------

(In reply to Bob Silverberg [:bsilverberg] from comment #4)
> 1. What is the case in which we would expect `inIncognitoContext` to return
> `true`.

That's a question without a clear answer. I'd expect it to return true for:

 - Content scripts running in tabs in private windows.
 - Web extension pages loaded into tabs in private windows.
 - Web extension pages loaded into popups in private windows.

I'm not sure, though, which of those currently work, or what Chrome does for
any of them.

> 2. How can I create that scenario in the context of a test? I've tried
> opening private windows and tabs, but I haven't been able to get it to work.
> Is there a way to open a private window via the `window.open` command that I
> am using on line 89?

window.open({incognito: true});
----------------------------------------

[1] https://people.mozilla.org/~kmaglione/webextension-test-coverage/toolkit/components/extensions/ext-extension.js.html
[2] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/Extension/inIncognitoContext

Comment 1

[Bob Silverberg [:bsilverberg]]

Attachment: MozReview Request: Bug 1263167 - Complete test coverage for browser.extension.inIncognitoContext, r?kmag

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

Comment 2

[Bob Silverberg [:bsilverberg]]

Note that I have added a test for one of the scenarios listed above, i.e.,

- Web extension pages loaded into tabs in private windows.

Should we endeavour to have all 3 cases tested?

Comment 3

[Kris Maglione [:kmag]]

(In reply to Bob Silverberg [:bsilverberg] from comment #2)
> Note that I have added a test for one of the scenarios listed above, i.e.,
> 
> - Web extension pages loaded into tabs in private windows.
> 
> Should we endeavour to have all 3 cases tested?

I think so, yes, but the other two can be follow-ups.

Comment 4

[Kris Maglione [:kmag]]

Comment on attachment 8739468 [details]
MozReview Request: Bug 1263167 - Complete test coverage for browser.extension.inIncognitoContext, r?kmag

https://reviewboard.mozilla.org/r/45283/#review42109

Comment 5

[Bob Silverberg [:bsilverberg]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1cb59a04a560

Comment 6

[Carsten Book [:Tomcat]]

fails to apply:

applying 35e4544a6c3a
patching file toolkit/components/extensions/test/mochitest/test_ext_extension.html
Hunk #1 FAILED at 8
1 out of 1 hunks FAILED -- saving rejects to file toolkit/components/extensions/test/mochitest/test_ext_extension.html.rej

could you take a look, thanks!

Comment 7

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8739468 [details]
MozReview Request: Bug 1263167 - Complete test coverage for browser.extension.inIncognitoContext, r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45283/diff/1-2/

Comment 8

[Bob Silverberg [:bsilverberg]]

Thanks Tomcat. I believe I fixed the conflict in the commit I just pushed.

Comment 9

[Pulsebot]

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

Comment 10

[Carsten Book [:Tomcat]]

(In reply to Bob Silverberg [:bsilverberg] from comment #8)
> Thanks Tomcat. I believe I fixed the conflict in the commit I just pushed.

yeah worked great, thanks!

Comment 11

[Carsten Book [:Tomcat]]

backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8714852&repo=fx-team

Comment 12

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/6b620569c320

Comment 13

[Bob Silverberg [:bsilverberg]]

It looks like those failures are on Android. Perhaps this test shouldn't be run on Android? Matt, what do you think?

Comment 14

[Matthew Wein [:mattw]]

From the logcat output: `Reading manifest: Error processing permissions.0: Unknown permission "tabs"`

The tabs API isn't implemented on android yet. You can disable this test on android until that API is supported, or maybe just remove the "tabs" permission if it isn't needed.

Comment 15

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8739468 [details]
MozReview Request: Bug 1263167 - Complete test coverage for browser.extension.inIncognitoContext, r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45283/diff/2-3/

Comment 16

[Bob Silverberg [:bsilverberg]]

Thanks Matt! I removed the tabs permission, which wasn't actually needed. Let's try landing this again.

Comment 17

[Pulsebot]

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

Comment 18

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

Backed out in https://hg.mozilla.org/integration/fx-team/rev/dfda93ed99d5 for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8791125&repo=fx-team

Comment 19

[Bob Silverberg [:bsilverberg]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=169b37462145

Comment 20

[Bob Silverberg [:bsilverberg]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0059ffe0567c

Comment 21

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8739468 [details]
MozReview Request: Bug 1263167 - Complete test coverage for browser.extension.inIncognitoContext, r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45283/diff/3-4/

Comment 22

[Bob Silverberg [:bsilverberg]]

This should be fixed now. One of the tests was invalid for Android so I moved it into a separate file and marked it to skip on Android.

Comment 23

[Pulsebot]

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

Comment 24

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

Had to back this out for e10s mochitest failure in test_ext_runtime_connect.html. Please make a Try run including at least all mochitests and platforms before the next push.

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

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=8881751&repo=fx-team

08:52:33     INFO -  1112 INFO TEST-START | toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html
08:52:33     INFO -  JavaScript error: chrome://global/content/bindings/browser.xml, line 437: TypeError: this.docShell is null
08:52:33     INFO -  [Parent 2716] ###!!! ABORT: __delete__()d actor: file /builds/slave/fx-team-m64-000000000000000000/build/src/ipc/glue/ProtocolUtils.cpp, line 377
08:52:33     INFO -  [Parent 2716] ###!!! ABORT: __delete__()d actor: file /builds/slave/fx-team-m64-000000000000000000/build/src/ipc/glue/ProtocolUtils.cpp, line 377
08:52:33     INFO -  TEST-INFO | started process screencapture
08:52:34     INFO -  TEST-INFO | screencapture: exit 0
08:52:34     INFO -  1113 INFO SpawnTask.js | Entering test test_contentscript
08:52:34     INFO -  1114 INFO Extension loaded
08:52:34     INFO -  1115 INFO extension loaded
08:52:34     INFO -  1116 INFO TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html | TypeError: win is null
08:52:34     INFO -      add_task/</<@SimpleTest/SpawnTask.js:282:15
08:52:34     INFO -      onRejected@SimpleTest/SpawnTask.js:85:15
08:52:34     INFO -      promise callback*next@SimpleTest/SpawnTask.js:104:45
08:52:34     INFO -      promise initializer*co@SimpleTest/SpawnTask.js:54:10
08:52:34     INFO -      add_task/<@SimpleTest/SpawnTask.js:270:9
08:52:34     INFO -      setTimeout handler*SimpleTest_setTimeoutShim@SimpleTest/SimpleTest.js:622:12
08:52:34     INFO -      add_task@SimpleTest/SpawnTask.js:269:7
08:52:34     INFO -      @toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html:66:1
08:52:34     INFO -  Not taking screenshot here: see the one that was previously logged
08:52:34     INFO -  1117 INFO TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html | Extension left running at test shutdown
08:52:34     INFO -      ExtensionTestUtils.loadExtension/<@SimpleTest/ExtensionTestUtils.js:88:7
08:52:34     INFO -      SimpleTest.finish/executeCleanupFunction@SimpleTest/SimpleTest.js:1106:19
08:52:34     INFO -      SimpleTest.finish@SimpleTest/SimpleTest.js:1119:5
08:52:34     INFO -      add_task/</<@SimpleTest/SpawnTask.js:288:11
08:52:34     INFO -      onRejected@SimpleTest/SpawnTask.js:85:15
08:52:34     INFO -      promise callback*next@SimpleTest/SpawnTask.js:104:45
08:52:34     INFO -      promise initializer*co@SimpleTest/SpawnTask.js:54:10
08:52:34     INFO -      add_task/<@SimpleTest/SpawnTask.js:270:9
08:52:34     INFO -      setTimeout handler*SimpleTest_setTimeoutShim@SimpleTest/SimpleTest.js:622:12
08:52:34     INFO -      add_task@SimpleTest/SpawnTask.js:269:7
08:52:34     INFO -      @toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html:66:1
08:52:34     INFO -  ###!!! [Child][DispatchAsyncMessage] Error: (msgtype=0x2C0082,name=???) Route error: message sent to unknown actor ID
08:52:34     INFO -  [Child 2717] ###!!! ABORT: Content child abort due to IPC error: file /builds/slave/fx-team-m64-000000000000000000/build/src/dom/ipc/ContentChild.cpp, line 2304
08:52:34     INFO -  [Child 2717] ###!!! ABORT: Content child abort due to IPC error: file /builds/slave/fx-team-m64-000000000000000000/build/src/dom/ipc/ContentChild.cpp, line 2304

Comment 25

[Bob Silverberg [:bsilverberg]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ded760c7e290

Comment 26

[Bob Silverberg [:bsilverberg]]

It looks like the bug that was blocking this from landing is marked as fixed, so I've submitted another try and will see if this can land if the try looks good.

Comment 27

[Bob Silverberg [:bsilverberg]]

I'm not seeing the above error in the latest try, so I think we can try landing this again.

Comment 28

[Carsten Book [:Tomcat]]

Hey Bob, seems this has now problems to apply and maybe need a rebase 

patching file toolkit/components/extensions/test/mochitest/mochitest.ini
Hunk #1 FAILED at 25
1 out of 1 hunks FAILED -- saving rejects to file toolkit/components/extensions/test/mochitest/mochitest.ini.rej
patching file toolkit/components/extensions/test/mochitest/test_ext_extension.html
Hunk #1 FAILED at 8
1 out of 1 hunks FAILED -- saving rejects to file toolkit/components/extensions/test/mochitest/test_ext_extension.html.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue

could you take a look, thanks!

Comment 29

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8739468 [details]
MozReview Request: Bug 1263167 - Complete test coverage for browser.extension.inIncognitoContext, r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45283/diff/4-5/

Comment 30

[Bob Silverberg [:bsilverberg]]

Sorry about that. I have rebased onto the latest fxteam. Let's try it again.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/beb2dbb917ea
Complete test coverage for browser.extension.inIncognitoContext, r=kmag

Comment 32

[Carsten Book [:Tomcat]]

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

Comment 33

[Carsten Book [:Tomcat]]

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