Add origin permissions to the extension permissions API.

Add origins to the extension permissions API.

BUG=48119, 84507
TEST=*Extension*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95704

[jstritar, 2011-08-03 19:09:45]

app engine error... trying to send again.

[Matt Perry, 2011-08-03 22:08:14]

http://codereview.chromium.org/7508029/diff/2001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_permissions_api.cc (right):

http://codereview.chromium.org/7508029/diff/2001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_permissions_api.cc:104:
origin.Parse(pattern, URLPattern::USE_PORTS);
shouldn't we use IGNORE_PORTS to match what we do for manifests?

http://codereview.chromium.org/7508029/diff/2001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_permissions_api.cc:213: host->Send(new
ExtensionMsg_UpdatePermissions(
Since we're only sending updates to renderers, what happens to renderers that
have been created after one of these messages is sent? Won't they will only see
the updates without having the full initial permission set?

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/permissions/optional/background.html
(right):

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/permissions/optional/background.html:55:
return checkEqualSets(set1.origins, set2.origins);
2 return statements in a row :)

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/permissions/optional/background.html:251:
doReq('http://c.com', true);
I believe the behavior of the pass/fail functions is that once all of them
finish, they call chrome.test.runNextTest(). If that's true, there's a race
between them calling runNextTest and doReq completing and calling it. I think
you need to wrap doReq's onload and onerror functions in pass/fail calls.

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
File
chrome/test/data/extensions/api_test/permissions/optional_deny/background.html
(right):

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/permissions/optional_deny/background.html:67:
doReq('http://b.c.com/', false);
ditto comments from other test

[jstritar, 2011-08-04 16:35:15]

http://codereview.chromium.org/7508029/diff/2001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_permissions_api.cc (right):

http://codereview.chromium.org/7508029/diff/2001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_permissions_api.cc:104:
origin.Parse(pattern, URLPattern::USE_PORTS);
On 2011/08/03 22:08:14, Matt Perry wrote:
> shouldn't we use IGNORE_PORTS to match what we do for manifests?

yes, thanks. done.

http://codereview.chromium.org/7508029/diff/2001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_permissions_api.cc:213: host->Send(new
ExtensionMsg_UpdatePermissions(
On 2011/08/03 22:08:14, Matt Perry wrote:
> Since we're only sending updates to renderers, what happens to renderers that
> have been created after one of these messages is sent? Won't they will only
see
> the updates without having the full initial permission set?

The is handled by the ExtensionMsg_Loaded message, implemented in a CL from when
you were away:

http://codereview.chromium.org/7432006

(We chose to keep the runtime data on Extension for now because so many places
access permissions through Extension that the refractoring would have become a
mess with an already large CL. The runtime data is kept on a struct within
Extension, so we can move it off in the future.)

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/permissions/optional/background.html
(right):

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/permissions/optional/background.html:55:
return checkEqualSets(set1.origins, set2.origins);
On 2011/08/03 22:08:14, Matt Perry wrote:
> 2 return statements in a row :)

Woops! Done.

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/permissions/optional/background.html:251:
doReq('http://c.com', true);
On 2011/08/03 22:08:14, Matt Perry wrote:
> I believe the behavior of the pass/fail functions is that once all of them
> finish, they call chrome.test.runNextTest(). If that's true, there's a race
> between them calling runNextTest and doReq completing and calling it. I think
> you need to wrap doReq's onload and onerror functions in pass/fail calls.

Thanks for catching that. I had copied the doReq function from the cross origin
XHR test, but I didn't use it as originally intended. I've updated doReq to
accept a callback so the caller can use the callbackPass function.

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
File
chrome/test/data/extensions/api_test/permissions/optional_deny/background.html
(right):

http://codereview.chromium.org/7508029/diff/2001/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/permissions/optional_deny/background.html:67:
doReq('http://b.c.com/', false);
On 2011/08/03 22:08:14, Matt Perry wrote:
> ditto comments from other test

Done.

[Matt Perry, 2011-08-04 18:06:42]

lgtm

[jstritar, 2011-08-04 19:07:34]

There's something strange about this CL. The try bots keep failing when patching
the background.html pages, which haven't changed in weeks. I also can't patch
this into a local clean branch with the same base revision. Any recommendations?

[Matt Perry, 2011-08-04 20:30:15]

On 2011/08/04 19:07:34, jstritar wrote:
> There's something strange about this CL. The try bots keep failing when
patching
> the background.html pages, which haven't changed in weeks. I also can't patch
> this into a local clean branch with the same base revision. Any
recommendations?

Hmm.. try pulling the background.html changes out of the patch, patch it into a
clean branch, then copy over your new background.html files? If that doesn't
work, ask maurel :)

[commit-bot: I haz the power, 2011-08-06 00:57:15]

Change committed as 95704