Closed
Bug 1263167
Opened 8 years ago
Closed 8 years ago
Complete test coverage for browser.extension.inIncognitoContext
Categories
(WebExtensions :: Untriaged, defect)
WebExtensions
Untriaged
Tracking
(firefox49 fixed)
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 | ||
Updated•8 years ago
|
Assignee: nobody → bob.silverberg
Status: NEW → ASSIGNED
No longer depends on: 1190322
Whiteboard: [testing] triaged → [testing]
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/45283/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45283/
Attachment #8739468 -
Flags: review?(kmaglione+bmo)
Assignee | ||
Comment 2•8 years ago
|
||
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•8 years ago
|
||
(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•8 years ago
|
||
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+
Assignee | ||
Comment 5•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1cb59a04a560
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 6•8 years ago
|
||
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)
Assignee | ||
Comment 7•8 years ago
|
||
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/
Assignee | ||
Comment 8•8 years ago
|
||
Thanks Tomcat. I believe I fixed the conflict in the commit I just pushed.
Flags: needinfo?(bob.silverberg)
Comment 10•8 years ago
|
||
(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•8 years ago
|
||
backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8714852&repo=fx-team
Flags: needinfo?(bob.silverberg)
Comment 12•8 years ago
|
||
Backout: https://hg.mozilla.org/integration/fx-team/rev/6b620569c320
Assignee | ||
Comment 13•8 years ago
|
||
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)
Comment 14•8 years ago
|
||
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)
Updated•8 years ago
|
Whiteboard: [testing] → [testing]triaged
Assignee | ||
Comment 15•8 years ago
|
||
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/
Assignee | ||
Comment 16•8 years ago
|
||
Thanks Matt! I removed the tabs permission, which wasn't actually needed. Let's try landing this again.
Keywords: checkin-needed
Comment 17•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0e7e5f7ecc07
Keywords: checkin-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
Flags: needinfo?(bob.silverberg)
Assignee | ||
Comment 19•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=169b37462145
Assignee | ||
Comment 20•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0059ffe0567c
Assignee | ||
Comment 21•8 years ago
|
||
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/
Assignee | ||
Comment 22•8 years ago
|
||
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
Comment 23•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/2c6f68832171
Keywords: checkin-needed
Comment 24•8 years ago
|
||
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)
Assignee | ||
Comment 25•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ded760c7e290
Assignee | ||
Comment 26•8 years ago
|
||
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.
Assignee | ||
Comment 27•8 years ago
|
||
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
Comment 28•8 years ago
|
||
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
Assignee | ||
Comment 29•8 years ago
|
||
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/
Assignee | ||
Comment 30•8 years ago
|
||
Sorry about that. I have rebased onto the latest fxteam. Let's try it again.
Flags: needinfo?(bob.silverberg)
Keywords: checkin-needed
Comment 31•8 years ago
|
||
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
Comment 32•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/beb2dbb917ea
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment 33•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/beb2dbb917ea
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•