1190322 - Test coverage for |extension| extension API

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Comment 1

[Kris Maglione [:kmag]]

This is still missing coverage for:

* The |getViews| API method with any properties object at all, and in particular with |type| or |windowId| properties.


https://people.mozilla.org/~kmaglione/webextension-test-coverage/toolkit/components/extensions/ext-extension.js.html

Comment 2

[Bob Silverberg [:bsilverberg]]

According to [1], this also now needs coverage for:

* The |inIncognitoContext| property.

[1] https://people.mozilla.org/~kmaglione/webextension-test-coverage/toolkit/components/extensions/ext-extension.js.html

Comment 3

[Bob Silverberg [:bsilverberg]]

Attachment: MozReview Request: Bug 1190322 - Test coverage for |extension| extension API, r?kmag

Add coverage for:

* The |getViews| API method with a |type| property.
* The |getViews| API method with a |windowId| property.
* The |inIncognitoContext| property.

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

Comment 4

[Bob Silverberg [:bsilverberg]]

https://reviewboard.mozilla.org/r/45019/#review41523

::: toolkit/components/extensions/test/mochitest/test_ext_extension.html:50
(Diff revision 1)
> +  yield extension.startup();
> +  yield extension.awaitFinish("inIncognitoContext");
> +  yield extension.unload();
> +});
> +
> +add_task(function* test_in_incognito_context_true() {

This is my attempt to write a test that would expect `inIncognitoContext` to be `true`. The test proceeds as expected, but `inIncognitoContext` is not `true` because the code is not running in a private window. I have a couple of questions about this:

1. What is the case in which we would expect `inIncognitoContext` to return `true`.
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?

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8739007 [details]
MozReview Request: Bug 1190322 - Test coverage for |extension| extension API, r?kmag

https://reviewboard.mozilla.org/r/45019/#review41601

Please file a different bug for the `isIncognitoContext` tests. The others look fine.

::: browser/components/extensions/test/browser/browser_ext_getViews.js:117
(Diff revision 1)
>    function* openTab(winId) {
>      extension.sendMessage("background-open-tab", winId);
>      yield extension.awaitMessage("tab-ready");
>    }
>  
> -  function* checkViews(kind, tabCount, popupCount) {
> +  function* checkViews(kind, tabCount, popupCount, kindCount, windowId, windowCount = 0) {

Please add `undefined` as a default value for `windowId` to make it clear that it's optional.

Comment 6

[Kris Maglione [:kmag]]

(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});

Comment 7

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8739007 [details]
MozReview Request: Bug 1190322 - Test coverage for |extension| extension API, r?kmag

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

Comment 8

[Bob Silverberg [:bsilverberg]]

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

Comment 9

[Bob Silverberg [:bsilverberg]]

https://reviewboard.mozilla.org/r/45019/#review41601

Ok, I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1263167

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing.

Comment 11

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8739007 [details]
MozReview Request: Bug 1190322 - Test coverage for |extension| extension API, r?kmag

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

Comment 12

[Bob Silverberg [:bsilverberg]]

Ok, rebased.

Comment 13

[Pulsebot]

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

Comment 14

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

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

Comment 15

[Carsten Book [:Tomcat]]

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