The default bug view has changed. See this FAQ.

Implement permissions API and optional_permissions manifest property

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
WebExtensions: Frontend
P1
normal
RESOLVED FIXED
2 years ago
a day ago

People

(Reporter: Mindaugas <LA-MJ>, Assigned: aswan)

Tracking

(Blocks: 7 bugs, {dev-doc-needed})

unspecified
mozilla55
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [permissions]triaged[ux], URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(5 attachments)

(Reporter)

Description

2 years ago
https://developer.chrome.com/extensions/permissions

API is listed on https://wiki.mozilla.org/WebExtensions#List_of_APIs_we_will_likely_support_in_the_future

Updated

2 years ago
Blocks: 1161828
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 1208765
Keywords: dev-doc-needed
Tweaking the bug title to make it more findable in searches. The permissions API is intrinsically tied to the optional_permissions manifest property.
Summary: Implement permissions API for open extension API → Implement permissions API and optional_permissions manifest property for open extension API

Updated

2 years ago
Whiteboard: [permissions]

Updated

2 years ago
Blocks: 1214433

Updated

a year ago
Flags: blocking-webextensions+
Depends on: 1227462
Blocks: 1227460
Blocks: 1227462
No longer depends on: 1227462
Blocks: 1227453
Blocks: 1227451

Updated

a year ago
Whiteboard: [permissions] → [permissions]triaged

Updated

a year ago
Priority: -- → P1
Whiteboard: [permissions]triaged → [permissions]triaged[ux]

Updated

a year ago
Depends on: 1203233

Updated

a year ago
Assignee: nobody → mjaritz
Yesterday you mentioned this having 2 steps, where the first one would not require UX. Is that correct?
What is the timeline for UX on this?
How do we surface required permissions? (https://developer.chrome.com/extensions/permission_warnings)
What is the advantage for the user?
What is the incentive for the Dev to use optional permissions?
What is an example on Chrome that uses optional permissions?
Flags: needinfo?(amckay)
(In reply to Markus Jaritz [:maritz] (UX) from comment #2)
> Yesterday you mentioned this having 2 steps, where the first one would not
> require UX. Is that correct?

After review, I don't think so. There was talk about using some built in API for this, I don't think they'll work.

Based on the fact that we don't have UX for this and its a big bug, I'm going to take this off tracking 48 and we'll see when UX has had opportunity to review it.

> What is the timeline for UX on this?

I think we should do this sooner rather than later. The ability of WebExtensions to explicitly ask for permissions is a good things for users and something we've never had with add-ons before.

> How do we surface required permissions?
> (https://developer.chrome.com/extensions/permission_warnings)
> What is the advantage for the user?
> What is the incentive for the Dev to use optional permissions?
> What is an example on Chrome that uses optional permissions?
Flags: needinfo?(amckay)
Flags: blocking-webextensions-
Flags: blocking-webextensions+

Updated

a year ago
Depends on: 1229230
(In reply to Andy McKay [:andym] from comment #3)
> (In reply to Markus Jaritz [:maritz] (UX) from comment #2)
> > Yesterday you mentioned this having 2 steps, where the first one would not
> > require UX. Is that correct?
> 
> After review, I don't think so. There was talk about using some built in API
> for this, I don't think they'll work.
> 
> Based on the fact that we don't have UX for this and its a big bug, I'm
> going to take this off tracking 48 and we'll see when UX has had opportunity
> to review it.

We have some UX for permission prompts that are designed for web 
pages. I think we'd most likely be able to reuse most of that, 
but we'd need some changes to make it clear that the message is 
related to an extension rather than a web page.

At some point we'll also need a UI to manage and revoke options 
permissions. I think that's going to have to depend on bug 
1212685.

Comment 5

a year ago
> What is the incentive for the Dev to use optional permissions?

I'll try with a simple scenario:
Create a simple feed reader which displays a feed list in a popup/action. The user should be able to customize what feed/url to fetch.

As far as I know there is no way to request a custom url given by the user as the permission does not allow it.

Updated

10 months ago
Blocks: 1269288
Work on this bug is happening on GitHub:
https://github.com/mozilla/addons/issues/51

Updated

10 months ago

Updated

10 months ago
Blocks: 1278100

Updated

7 months ago
Blocks: 1234150
Blocks: 1303905

Updated

6 months ago
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Flags: blocking-webextensions-

Updated

6 months ago
Summary: Implement permissions API and optional_permissions manifest property for open extension API → (tracking} Implement permissions API and optional_permissions manifest property for open extension API

Updated

6 months ago
Depends on: 1201896

Comment 7

6 months ago
Un-assigning since this has morphed from a UX bug to a tracking bug.
Assignee: mjaritz → nobody

Updated

6 months ago
Summary: (tracking} Implement permissions API and optional_permissions manifest property for open extension API → (tracking) Implement permissions API and optional_permissions manifest property for open extension API

Updated

6 months ago
Blocks: 1246236
(Assignee)

Updated

5 months ago
No longer blocks: 1234150

Updated

4 months ago
Blocks: 1283650

Updated

4 months ago
Assignee: nobody → aswan

Updated

2 months ago
webextensions: --- → +
Blocks: 1332408
(Assignee)

Updated

2 months ago
Depends on: 1333235
(Assignee)

Updated

2 months ago
No longer depends on: 1203233
(Assignee)

Updated

2 months ago
Depends on: 1333477
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 10

a month ago
Kris, this needs tests of course and I haven't yet touched content scripts but can you take a look and let me know how you want to connect what's here with bug 1333477?  I can put in message manager dispatching/handling here or would you rather handle that in 1333477?  Note that we also need to update whiteListedHosts in various contexts, as the bugs are worded that would make more sense here but I dont' really care where we do it.
Flags: needinfo?(kmaglione+bmo)
(Assignee)

Updated

15 days ago
Flags: needinfo?(kmaglione+bmo)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

12 days ago
Summary: (tracking) Implement permissions API and optional_permissions manifest property for open extension API → Implement permissions API and optional_permissions manifest property
(Assignee)

Updated

12 days ago
Attachment #8841600 - Flags: review?(kmaglione+bmo)
Attachment #8841601 - Flags: review?(kmaglione+bmo)
Attachment #8848272 - Flags: review?(kmaglione+bmo)
Attachment #8848273 - Flags: review?(kmaglione+bmo)

Comment 15

11 days ago
mozreview-review
Comment on attachment 8841600 [details]
Bug 1197420 Part 1 Schema groundwork for optional permissions

https://reviewboard.mozilla.org/r/115752/#review123618

::: toolkit/components/extensions/schemas/manifest.json:238
(Diff revision 2)
> +          { "$ref": "OptionalPermission" },
> +          {
> +            "type": "string",
> +            "enum": [
> +              "alarms",
> +              "geolocation",

Hm. Seems like at least geolocation should work as an optional permission...
Attachment #8841600 - Flags: review?(kmaglione+bmo) → review+

Comment 16

11 days ago
mozreview-review
Comment on attachment 8841601 [details]
Bug 1197420 Part 3 Initial browser.permissions api support

https://reviewboard.mozilla.org/r/115754/#review123620

Looks good, but I think we need a better solution for the user input handling.

::: browser/modules/ExtensionsUI.jsm:214
(Diff revision 2)
> +      let strings = this._buildStrings({
> +        type: "optional",
> +        addon: {name},
> +        permissions,
> +      });
> +      this.showPermissionsPrompt(browser, strings, icon).then(resolve);

Nit: `resolve(this.showPermissionsPrompt(...))`

That way we pass along any rejections, too.

::: toolkit/components/extensions/ExtensionPermissions.jsm:24
(Diff revision 2)
> +async function lazyInit() {
> +  if (prefs) {
> +    return;
> +  }
> +
> +  let profileDir = Services.dirsvc.get("ProfD", Ci.nsIFile).path;

`OS.Constants.Path.profileDir`

::: toolkit/components/extensions/ExtensionPermissions.jsm:28
(Diff revision 2)
> +
> +  let profileDir = Services.dirsvc.get("ProfD", Ci.nsIFile).path;
> +  prefs = new JSONFile({path: OS.Path.join(profileDir, FILE_NAME)});
> +
> +  await prefs.load();
> +  prefs.ensureDataReady();

Not necessary. Awaiting on `load()` guarantees this.

::: toolkit/components/extensions/ExtensionPermissions.jsm:38
(Diff revision 2)
> +}
> +
> +this.ExtensionPermissions = {
> +  async get(extension) {
> +    await lazyInit();
> +    return prefs.data[extension.id] || emptyPermissions();

We should probably clone or freeze this, so we don't wind up with changes that don't get saved right away.

::: toolkit/components/extensions/ExtensionPermissions.jsm:53
(Diff revision 2)
> +      if (!prefs.data[extension.id].permissions.includes(perm)) {
> +        added.permissions.push(perm);
> +        prefs.data[extension.id].permissions.push(perm);
> +      }

Please do something like `let perms = prefs.data[extension.id].permissions` outside of the loop. Same for origins and `.remove()`

::: toolkit/components/extensions/ExtensionPermissions.jsm:84
(Diff revision 2)
> +
> +    let removed = emptyPermissions();
> +
> +    for (let perm of permissions.permissions) {
> +      let i = prefs.data[extension.id].permissions.indexOf(perm);
> +      if (i != -1) {

`if (i > 0)` please... Checking explicitly against -1 always makes me nervous.

::: toolkit/components/extensions/ext-c-permissions.js:10
(Diff revision 2)
> +        let winUtils = context.contentWindow.getInterface(Ci.nsIDOMWindowUtils);
> +        if (!winUtils.isHandlingUserInput) {
> +          throw new ExtensionError("May only request permissions from a user input handler");
> +        }

I'd rather we send this information to the parent at the schema level. One of mixedpuppy's patches also requires this information in the parent in several places, so it would simplify a lot of things.

::: toolkit/components/extensions/ext-permissions.js:22
(Diff revision 2)
> +const {
> +  classifyPermission,
> +  ExtensionError,
> +} = ExtensionUtils;
> +
> +XPCOMUtils.defineLazyPreferenceGetter(this, "PROMPTS", "extensions.webextOptionalPermissionPrompts");

Please no ALL_CAPS for non-constant values.

::: toolkit/components/extensions/ext-permissions.js:39
(Diff revision 2)
> +            let type = classifyPermission(perm);
> +            if (type.origin) {
> +              origins.push(perm);
> +            }
> +          }
> +          optionalOrigins = new MatchPattern(origins);

Can we just make this a lazy getter on the Extension instead?

::: toolkit/components/extensions/ext-permissions.js:42
(Diff revision 2)
> +        let {permissions, origins} = perms;
> +        permissions = permissions || [];
> +        origins = origins || [];

Please just set `"default": []` for these in the schema. Or `let {permissions = [], origins = []}` if that's not an option for some reason.

::: toolkit/components/extensions/ext-permissions.js:81
(Diff revision 2)
> +        await ExtensionPermissions.add(context.extension, perms);
> +        return true;
> +      },
> +
> +      async getAll() {
> +        let perms = context.extension.userPermissions();

Please just make `userPermissions` a getter rather than a method.

::: toolkit/components/extensions/ext-permissions.js:84
(Diff revision 2)
> +
> +      async getAll() {
> +        let perms = context.extension.userPermissions();
> +        return {
> +          permissions: perms.permissions,
> +          origins: perms.hosts,

Why hosts in one place and origins in another?

::: toolkit/components/extensions/ext-permissions.js:89
(Diff revision 2)
> +        for (let perm of permissions.permissions) {
> +          if (!context.extension.hasPermission(perm)) {
> +            return false;
> +          }
> +        }
> +
> +        for (let origin of permissions.origins) {

What happens if either of these is null?

::: toolkit/components/extensions/schemas/permissions.json:53
(Diff revision 2)
> +        "description": "Get a list of all the extension's permissions.",
> +        "parameters": [
> +          {
> +            "name": "callback",
> +            "type": "function",
> +            "optional": true,

No `"optional": true`, please.
Attachment #8841601 - Flags: review?(kmaglione+bmo)

Comment 17

11 days ago
mozreview-review
Comment on attachment 8841601 [details]
Bug 1197420 Part 3 Initial browser.permissions api support

https://reviewboard.mozilla.org/r/115754/#review123624

::: toolkit/components/extensions/ExtensionPermissions.jsm:20
(Diff revision 2)
> +  if (prefs) {
> +    return;
> +  }

Oh, also, we need to store a promise the first time this is called, and return that if we're not fully initialized yet. Otherwise, when there are concurrent callers, some will wind up with non-fully-initialized pref stores.

Comment 18

11 days ago
mozreview-review
Comment on attachment 8848272 [details]
Bug 1197420 Part 4 Apply dynamic permission changes

https://reviewboard.mozilla.org/r/121204/#review123622

::: toolkit/components/extensions/Extension.jsm:690
(Diff revision 1)
> +    this.on("remove-permissions", (ignoreEvent, permissions) => {
> +      for (let perm of permissions.permissions) {
> +        this.permissions.delete(perm);
> +      }
> +
> +      if (permissions.origins.length > 0) {

No need for the `if`.

::: toolkit/components/extensions/Extension.jsm:927
(Diff revision 1)
>          return;
>        }
>  
>        GlobalManager.init(this);
>  
> +      return ExtensionPermissions.get(this);

We should do this earlier, as part of a `Promies.all`, or at least start the load earlier, and then wait on the promise here.

::: toolkit/components/extensions/ExtensionChild.jsm:786
(Diff revision 1)
>    hasPermission(permission) {
>      return this.context.extension.hasPermission(permission);
>    }
> +
> +  isPermissionRevokable(permission) {
> +    return ChildAPIManager.OPTIONAL_PERMISSIONS.includes(permission);

I think we should check against the manifest's set of optional permissions here, instead. If it's not an optional permission for this particular extension, it'll never be lazily instantiated or revoked.

::: toolkit/components/extensions/ExtensionChild.jsm:790
(Diff revision 1)
> +    // XXX when does this get cleaned up
> +    this.context.extension.permissionChangedCallbacks.add(callback);

I think we probably want a separate set of these per context, and then the context can do `extension.on("add-permissions", ...)` and such, and remove the listeners on unload. Otherwise we need to store the set of listeners specific to this context, and remove them from the extension instance on unload.

::: toolkit/components/extensions/ExtensionChild.jsm:796
(Diff revision 1)
> +  let type = Schemas.rootNamespace
> +                    .get("manifest")
> +                    .get("OptionalPermission");
> +  let permissions = [];
> +  for (let choice of type.choices) {
> +    if (choice.enumeration) {
> +      permissions = permissions.concat(...choice.enumeration);
> +    }
> +  }
> +  return permissions;

No need for this if we check against the extension's own set of optional permissions. They're already validated against this.

::: toolkit/components/extensions/ExtensionContent.jsm:831
(Diff revision 1)
> +      if (permissions.permissions.length > 0) {
> +        for (let perm of permissions.permissions) {
> +          this.permissions.add(perm);
> +        }
> +        for (let callback of this.permissionChangedCallbacks) {
> +          callback();

Please catch errors here. Not catching errors from callbacks always makes me nervous... Same below.
Attachment #8848272 - Flags: review?(kmaglione+bmo)

Comment 19

11 days ago
mozreview-review
Comment on attachment 8848273 [details]
Bug 1197420 Part 5 Tests for optional permissions

https://reviewboard.mozilla.org/r/121206/#review123626

::: toolkit/components/extensions/test/mochitest/chrome.ini:21
(Diff revision 1)
> +[test_chrome_ext_downloads_saveAs.html]
> +[test_chrome_ext_eventpage_warning.html]
>  [test_chrome_ext_hybrid_addons.html]
> +[test_chrome_ext_idle.html]
> +[test_chrome_ext_identity.html]
> +skip-if = os == 'android' # unsupported.
> +[test_chrome_ext_permissions.html]
> +[test_chrome_ext_storage_cleanup.html]
> +[test_chrome_ext_trackingprotection.html]
>  [test_chrome_ext_trustworthy_origin.html]
>  [test_chrome_ext_webnavigation_resolved_urls.html]
> +[test_chrome_ext_webrequest_background_events.html]
> +[test_chrome_ext_webrequest_host_permissions.html]

Thank you.

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_permissions.html:15
(Diff revision 1)
> +<body>
> +
> +<script type="text/javascript">
> +"use strict";
> +
> +add_task(async function test_permissions() {

+1

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_permissions.html:18
(Diff revision 1)
> +      permissions: ["cookies"],
> +      origins: ["*://example.com/"],

Please also add tests for only "permissions", only "origins", and probably also with neither, for all of the relevant methods.

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_permissions.html:35
(Diff revision 1)
> +      if (msg == "set-cookie") {
> +        try {
> +          browser.cookies.set({
> +            url: "http://example.com/",
> +            name: "COOKIE",
> +            value: "NOM NOM",

Hm...

::: toolkit/components/extensions/test/xpcshell/test_ext_permissions.js:160
(Diff revision 1)
> +  // Restart, verify permissions are still present
> +  await AddonTestUtils.promiseRestartManager();
> +  await extension.awaitStartup();

Would be good to test initial startup (with the settings store uninitialized) with multiple current extensions.
Attachment #8848273 - Flags: review?(kmaglione+bmo) → review+
(Assignee)

Comment 20

9 days ago
mozreview-review-reply
Comment on attachment 8841601 [details]
Bug 1197420 Part 3 Initial browser.permissions api support

https://reviewboard.mozilla.org/r/115754/#review123620

> Why hosts in one place and origins in another?

`hosts` is never externally visible and that code was written before any of the optional permissions code.  In the optional permissions API, `origins` is part of the extension-facing api and so it needs to be this way to be compatible with Chrome (and of course it it more sensible this way).  I was wary of changing to much at once but I think I should switch everything over to origins, will do that in a revision.
(Assignee)

Comment 21

9 days ago
mozreview-review-reply
Comment on attachment 8841600 [details]
Bug 1197420 Part 1 Schema groundwork for optional permissions

https://reviewboard.mozilla.org/r/115752/#review123618

> Hm. Seems like at least geolocation should work as an optional permission...

I just copied the existing list from Chrome: https://developer.chrome.com/extensions/permissions
(Assignee)

Comment 22

9 days ago
mozreview-review-reply
Comment on attachment 8841601 [details]
Bug 1197420 Part 3 Initial browser.permissions api support

https://reviewboard.mozilla.org/r/115754/#review123620

> We should probably clone or freeze this, so we don't wind up with changes that don't get saved right away.

I'm not sure I follow, are you concerned about the caller writing to the returned object?  This is only called from one place (extension startup) where I don't think there's much danger of somebody adding code that changes the object.  I can add a guard if you feel strongly though...

> I'd rather we send this information to the parent at the schema level. One of mixedpuppy's patches also requires this information in the parent in several places, so it would simplify a lot of things.

Do you mind landing this as-is and doing that work in a follow-up?  I'm happy to do it and will do it during the 55 cycle.

> Please just set `"default": []` for these in the schema. Or `let {permissions = [], origins = []}` if that's not an option for some reason.

Whoops these do have `default: []` in the schema, this was just old code

> What happens if either of these is null?

They have a default value of `[]` so they can't be null
(Assignee)

Comment 23

9 days ago
mozreview-review-reply
Comment on attachment 8841601 [details]
Bug 1197420 Part 3 Initial browser.permissions api support

https://reviewboard.mozilla.org/r/115754/#review123624

> Oh, also, we need to store a promise the first time this is called, and return that if we're not fully initialized yet. Otherwise, when there are concurrent callers, some will wind up with non-fully-initialized pref stores.

whoops thanks for catching that.  it seems very likely to happen at startup to anybody with more than one webextension installed.
(Assignee)

Comment 24

9 days ago
mozreview-review-reply
Comment on attachment 8848272 [details]
Bug 1197420 Part 4 Apply dynamic permission changes

https://reviewboard.mozilla.org/r/121204/#review123622

> I think we probably want a separate set of these per context, and then the context can do `extension.on("add-permissions", ...)` and such, and remove the listeners on unload. Otherwise we need to store the set of listeners specific to this context, and remove them from the extension instance on unload.

Okay, but the callbacks are going to call `hasPermission()` which consults the Extension.  So we need to make sure that the extension is updated before we call any of the callbacks.  We can do that by relying on EventEmitter handlers being called in the order they were originally added, but that makes me nervous.  Maybe it shouldn't though...
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

8 days ago
Attachment #8849285 - Flags: review?(kmaglione+bmo)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 31

6 days ago
mozreview-review
Comment on attachment 8849285 [details]
Bug 1197420 Part 2 Extension cleanups for optional permissions

https://reviewboard.mozilla.org/r/122098/#review125638
Attachment #8849285 - Flags: review?(kmaglione+bmo) → review+

Comment 32

6 days ago
mozreview-review
Comment on attachment 8841601 [details]
Bug 1197420 Part 3 Initial browser.permissions api support

https://reviewboard.mozilla.org/r/115754/#review125640

::: toolkit/components/extensions/ExtensionPermissions.jsm:24
(Diff revision 3)
> +    return;
> +  }
> +
> +  prefs = new JSONFile({path: OS.Path.join(OS.Constants.Path.profileDir, FILE_NAME)});
> +
> +  await prefs.load();

I think we need to store this load promise, and await on it the next time someone calls `lazyInit()`. Otherwise they may wind up with a not-fully-initialized copy.
Attachment #8841601 - Flags: review?(kmaglione+bmo)

Comment 33

6 days ago
mozreview-review
Comment on attachment 8841601 [details]
Bug 1197420 Part 3 Initial browser.permissions api support

https://reviewboard.mozilla.org/r/115754/#review125646
Attachment #8841601 - Flags: review+

Comment 34

5 days ago
mozreview-review
Comment on attachment 8848272 [details]
Bug 1197420 Part 4 Apply dynamic permission changes

https://reviewboard.mozilla.org/r/121204/#review125648

::: toolkit/components/extensions/Extension.jsm:912
(Diff revision 3)
> -  startup() {
> +  async startup() {
>      let started = false;
> -    return this.loadManifest().then(() => {
> +
> +    try {
> +      let perms;
> +      [, perms] = await Promise.all([this.loadManifest(), ExtensionPermissions.get(this)]);

Nit: `let [, perms] = ...`

::: toolkit/components/extensions/Extension.jsm:1039
(Diff revision 3)
>  
> -    return this.permissions.has(perm);
> +    if (this.permissions.has(perm)) {
> +      return true;
> +    }
> +
> +    if (includeOptional && (this.manifest.optional_permissions || []).includes(perm)) {

Please just use `"default": []` for `optional_permissions` in the manifest schema, and drop the `|| []`
Attachment #8848272 - Flags: review?(kmaglione+bmo) → review+
(Assignee)

Comment 35

5 days ago
mozreview-review-reply
Comment on attachment 8841601 [details]
Bug 1197420 Part 3 Initial browser.permissions api support

https://reviewboard.mozilla.org/r/115754/#review123620

> Do you mind landing this as-is and doing that work in a follow-up?  I'm happy to do it and will do it during the 55 cycle.

Filed bug 1350151 for this
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 39

5 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=515c908833e25324b311dc659bcc63f448ece8d0
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 45

5 days ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 88db4c72a9d8 -d 8528543cc258: rebasing 383932:88db4c72a9d8 "Bug 1197420 Part 1 Schema groundwork for optional permissions r=kmag"
rebasing 383933:6bf3abe0db8b "Bug 1197420 Part 2 Extension cleanups for optional permissions r=kmag"
merging browser/base/content/test/webextensions/browser_extension_sideloading.js
merging toolkit/components/extensions/Extension.jsm
merging toolkit/mozapps/extensions/content/extensions.js
merging toolkit/mozapps/extensions/internal/XPIProvider.jsm
rebasing 383934:46451e501104 "Bug 1197420 Part 3 Initial browser.permissions api support r=kmag"
merging browser/locales/en-US/chrome/browser/browser.properties
merging toolkit/components/extensions/Extension.jsm
merging toolkit/components/extensions/ExtensionUtils.jsm
rebasing 383935:5fe8342b1042 "Bug 1197420 Part 4 Apply dynamic permission changes r=kmag"
merging toolkit/components/extensions/Extension.jsm
merging toolkit/components/extensions/ExtensionChild.jsm
merging toolkit/components/extensions/ExtensionCommon.jsm
merging toolkit/components/extensions/ExtensionContent.jsm
rebasing 383936:4fb68e7d2f3d "Bug 1197420 Part 5 Tests for optional permissions r=kmag" (tip)
merging toolkit/components/extensions/test/mochitest/chrome.ini
merging toolkit/components/extensions/test/xpcshell/xpcshell.ini
warning: conflicts while merging toolkit/components/extensions/test/mochitest/chrome.ini! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 51

5 days ago
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c4c29c0b05a
Part 1 Schema groundwork for optional permissions r=kmag
https://hg.mozilla.org/integration/autoland/rev/3de2de388ac9
Part 2 Extension cleanups for optional permissions r=kmag
https://hg.mozilla.org/integration/autoland/rev/7df6cc66a2eb
Part 3 Initial browser.permissions api support r=kmag
https://hg.mozilla.org/integration/autoland/rev/cb352ddee812
Part 4 Apply dynamic permission changes r=kmag
https://hg.mozilla.org/integration/autoland/rev/5750ae148c78
Part 5 Tests for optional permissions r=kmag
Backed out for failing test_ext_all_apis.html:

https://hg.mozilla.org/integration/autoland/rev/32b07901a07087717a53ab3e7fd15e8baca7925e
https://hg.mozilla.org/integration/autoland/rev/d16b5157f406d77b592857b5c1d70ab74a3cd535
https://hg.mozilla.org/integration/autoland/rev/7f34c9359eb8beb3c945031b0eefba82c6d13954
https://hg.mozilla.org/integration/autoland/rev/1e0885d9302764b0e8952ddf723d891f35e57b64
https://hg.mozilla.org/integration/autoland/rev/83cea5c428a741b01564bad066c4721139983b9d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5750ae148c78047de794a6cb242b1c9a6069c2f6&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=86294637&repo=autoland

12:11:36     INFO - TEST-PASS | browser/components/extensions/test/mochitest/test_ext_all_apis.html | content script APIs 
12:11:36     INFO - SpawnTask.js | Leaving test test_enumerate_content_script_apis
12:11:36     INFO - SpawnTask.js | Entering test test_enumerate_background_script_apis
12:11:36     INFO - Extension loaded
12:11:36     INFO - Buffered messages finished
12:11:36     INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/mochitest/test_ext_all_apis.html | background script APIs -     Structures begin differing at:
12:11:36     INFO - got[20] = "browser.permissions.contains"
12:11:36     INFO - expected[20] = "browser.runtime.OnInstalledReason"
12:11:36     INFO - 
12:11:36     INFO -     SimpleTest.isDeeply@SimpleTest/SimpleTest.js:1595:9
12:11:36     INFO -     test_enumerate_background_script_apis@browser/components/extensions/test/mochitest/test_ext_all_apis.js:160:3
12:11:36     INFO -     onFulfilled@SimpleTest/SpawnTask.js:69:15
12:11:36     INFO -     promise callback*next@SimpleTest/SpawnTask.js:104:45
12:11:36     INFO -     onFulfilled@SimpleTest/SpawnTask.js:73:7
12:11:36     INFO -     promise callback*next@SimpleTest/SpawnTask.js:104:45
12:11:36     INFO -     onFulfilled@SimpleTest/SpawnTask.js:73:7
12:11:36     INFO -     co/<@SimpleTest/SpawnTask.js:58:5
12:11:36     INFO -     co@SimpleTest/SpawnTask.js:54:10
12:11:36     INFO -     toPromise@SimpleTest/SpawnTask.js:122:60
12:11:36     INFO -     next@SimpleTest/SpawnTask.js:103:19
12:11:36     INFO -     onFulfilled@SimpleTest/SpawnTask.js:73:7
12:11:36     INFO -     promise callback*next@SimpleTest/SpawnTask.js:104:45
12:11:36     INFO -     onFulfilled@SimpleTest/SpawnTask.js:73:7
12:11:36     INFO -     co/<@SimpleTest/SpawnTask.js:58:5
12:11:36     INFO -     co@SimpleTest/SpawnTask.js:54:10
12:11:36     INFO -     add_task/<@SimpleTest/SpawnTask.js:270:9
12:11:36     INFO -     setTimeout handler*SimpleTest_setTimeoutShim@SimpleTest/SimpleTest.js:672:12
12:11:36     INFO -     add_task@SimpleTest/SpawnTask.js:269:7
12:11:36     INFO -     @browser/components/extensions/test/mochitest/test_ext_all_apis.js:127:1
Flags: needinfo?(aswan)
Also failing xpcshell test test_ext_permissions.js: https://treeherder.mozilla.org/logviewer.html#?job_id=86296671&repo=autoland
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 59

4 days ago
hg error in cmd: hg identify upstream -r tip:

Comment 60

4 days ago
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e690bbe8b5a
Part 1 Schema groundwork for optional permissions r=kmag
https://hg.mozilla.org/integration/autoland/rev/440bab141509
Part 2 Extension cleanups for optional permissions r=kmag
https://hg.mozilla.org/integration/autoland/rev/46e135035f10
Part 3 Initial browser.permissions api support r=kmag
https://hg.mozilla.org/integration/autoland/rev/925e3a9499ee
Part 4 Apply dynamic permission changes r=kmag
https://hg.mozilla.org/integration/autoland/rev/8a0125e00903
Part 5 Tests for optional permissions r=kmag
Backed out in https://hg.mozilla.org/integration/autoland/rev/0fa8a7ec4b42 for timing out on Android, https://treeherder.mozilla.org/logviewer.html#?job_id=86373358&repo=autoland and https://treeherder.mozilla.org/logviewer.html#?job_id=86373354&repo=autoland
(Assignee)

Updated

4 days ago
Blocks: 1350559
Comment hidden (mozreview-request)

Comment 63

4 days ago
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b6af7d779efc
Part 1 Schema groundwork for optional permissions r=kmag
https://hg.mozilla.org/integration/autoland/rev/680dd7916a23
Part 2 Extension cleanups for optional permissions r=kmag
https://hg.mozilla.org/integration/autoland/rev/d1628b66e5f8
Part 3 Initial browser.permissions api support r=kmag
https://hg.mozilla.org/integration/autoland/rev/f4fbd8e60288
Part 4 Apply dynamic permission changes r=kmag
https://hg.mozilla.org/integration/autoland/rev/b56c89bfeb0e
Part 5 Tests for optional permissions r=kmag

Comment 64

4 days ago
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/2b54d882c024
Backed out changeset b56c89bfeb0e 
https://hg.mozilla.org/integration/autoland/rev/94e7f0558069
Backed out changeset f4fbd8e60288 
https://hg.mozilla.org/integration/autoland/rev/0b0d2e8ada83
Backed out changeset d1628b66e5f8 
https://hg.mozilla.org/integration/autoland/rev/a1667683da70
Backed out changeset 680dd7916a23 
https://hg.mozilla.org/integration/autoland/rev/66fe37afe8f1
Backed out changeset b6af7d779efc on request of developer. r=backout
(Assignee)

Comment 65

4 days ago
New try run, good on Android:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=40f4bed27c01baa24815a37ef1328ff4e135c528
Flags: needinfo?(aswan)
Comment hidden (mozreview-request)

Comment 67

4 days ago
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0a1daa74303c
Part 1 Schema groundwork for optional permissions r=kmag
https://hg.mozilla.org/integration/autoland/rev/52c2b953c811
Part 2 Extension cleanups for optional permissions r=kmag
https://hg.mozilla.org/integration/autoland/rev/a9a6dad3b0b7
Part 3 Initial browser.permissions api support r=kmag
https://hg.mozilla.org/integration/autoland/rev/2ba59dc585a3
Part 4 Apply dynamic permission changes r=kmag
https://hg.mozilla.org/integration/autoland/rev/4e319a074b49
Part 5 Tests for optional permissions r=kmag

Comment 68

3 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0a1daa74303c
https://hg.mozilla.org/mozilla-central/rev/52c2b953c811
https://hg.mozilla.org/mozilla-central/rev/a9a6dad3b0b7
https://hg.mozilla.org/mozilla-central/rev/2ba59dc585a3
https://hg.mozilla.org/mozilla-central/rev/4e319a074b49
Status: NEW → RESOLVED
Last Resolved: 3 days ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.