Closed Bug 1263167 Opened 8 years ago Closed 8 years ago

Complete test coverage for browser.extension.inIncognitoContext

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set
normal

Tracking

(firefox49 fixed)

RESOLVED FIXED
mozilla49
Iteration:
48.3 - Apr 25
Tracking Status
firefox49 --- fixed

People

(Reporter: bsilverberg, Assigned: bsilverberg)

References

(Blocks 1 open bug)

Details

(Whiteboard: [testing]triaged)

Attachments

(1 file)

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
Assignee: nobody → bob.silverberg
Status: NEW → ASSIGNED
No longer depends on: 1190322
Whiteboard: [testing] triaged → [testing]
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?
(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 on attachment 8739468 [details]
MozReview Request: Bug 1263167 - Complete test coverage for browser.extension.inIncognitoContext, r?kmag

https://reviewboard.mozilla.org/r/45283/#review42109
Attachment #8739468 - Flags: review?(kmaglione+bmo) → review+
Blocks: 1263900
Keywords: checkin-needed
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!
Flags: needinfo?(bob.silverberg)
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/
Thanks Tomcat. I believe I fixed the conflict in the commit I just pushed.
Flags: needinfo?(bob.silverberg)
(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!
backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8714852&repo=fx-team
Flags: needinfo?(bob.silverberg)
It looks like those failures are on Android. Perhaps this test shouldn't be run on Android? Matt, what do you think?
Flags: needinfo?(bob.silverberg) → needinfo?(mwein)
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.
Flags: needinfo?(mwein)
Whiteboard: [testing] → [testing]triaged
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/
Thanks Matt! I removed the tabs permission, which wasn't actually needed. Let's try landing this again.
Keywords: checkin-needed
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/
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.
Flags: needinfo?(bob.silverberg)
Keywords: checkin-needed
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
Flags: needinfo?(bob.silverberg)
See Also: → 1166351
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.
I'm not seeing the above error in the latest try, so I think we can try landing this again.
Flags: needinfo?(bob.silverberg)
Keywords: checkin-needed
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!
Flags: needinfo?(bob.silverberg)
Keywords: checkin-needed
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/
Sorry about that. I have rebased onto the latest fxteam. Let's try it again.
Flags: needinfo?(bob.silverberg)
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/beb2dbb917ea
Complete test coverage for browser.extension.inIncognitoContext, r=kmag
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/beb2dbb917ea
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.