Add OAuth2 scopes to the ExtensionPermissionSet and granted permissions.

Add OAuth2 scopes to the ExtensionPermissionSet and granted permissions.

BUG=none
TEST=unit_tests


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

[jstritar, 2012-04-12 14:34:10]

This isn't ready for review yet, but it works. You can patch this in if you need
it on your end. I'm going to start another CL, based on this one, that adds the
messages to the prompt.

[jstritar, 2012-04-24 20:43:40]

@yoz, can you take a look?

[Yoyo Zhou, 2012-04-24 23:16:36]

Mostly LGTM, just a few unimportant comments.

http://codereview.chromium.org/10049006/diff/10001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_prefs.cc (right):

http://codereview.chromium.org/10049006/diff/10001/chrome/browser/extensions/...
chrome/browser/extensions/extension_prefs.cc:127: const char kPrefScopes[] =
"scopes";
The ones just above this aren't plural, so consider whether you want this to be
plural or not.

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension.cc:243: ExtensionOAuth2Scopes
Extension::OAuth2Info::GetScopesAsSet() {
nit: newline above this

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
File chrome/common/extensions/extension_permission_set.h (right):

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension_permission_set.h:14: #include
"base/callback.h"
You don't seem to use this.

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension_permission_set.h:284: 
nit: why this extra line?

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension_permission_set.h:450: // A set of oauth2
scopes that are specific to Google Accounts.
Can you elaborate on what these are used for in the comment (looks like the
signed-in sync account)?

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
File chrome/common/extensions/extension_permission_set_unittest.cc (left):

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension_permission_set_unittest.cc:468: const char*
granted_apis[10];
You pruned these because they were getting unmaintainable?

http://codereview.chromium.org/10049006/diff/10001/chrome/test/data/extension...
File chrome/test/data/extensions/allow_silent_upgrade/scopes1_new.json (right):

http://codereview.chromium.org/10049006/diff/10001/chrome/test/data/extension...
chrome/test/data/extensions/allow_silent_upgrade/scopes1_new.json:3: "version":
"1.0",
It's kinda weird that the version number doesn't increase from old to new.

[jstritar, 2012-04-24 23:33:34]

Thanks for the review!

http://codereview.chromium.org/10049006/diff/10001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_prefs.cc (right):

http://codereview.chromium.org/10049006/diff/10001/chrome/browser/extensions/...
chrome/browser/extensions/extension_prefs.cc:127: const char kPrefScopes[] =
"scopes";
On 2012/04/24 23:16:36, Yoyo Zhou wrote:
> The ones just above this aren't plural, so consider whether you want this to
be
> plural or not.

Yeah, this is weird.... I don't remember why we made those singular.
I'm going to stick with "scopes" to be consistent with the manifest field and
other uses of it.

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension.cc:243: ExtensionOAuth2Scopes
Extension::OAuth2Info::GetScopesAsSet() {
On 2012/04/24 23:16:36, Yoyo Zhou wrote:
> nit: newline above this

Done.

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
File chrome/common/extensions/extension_permission_set.h (right):

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension_permission_set.h:14: #include
"base/callback.h"
On 2012/04/24 23:16:36, Yoyo Zhou wrote:
> You don't seem to use this.

Done.

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension_permission_set.h:284: 
On 2012/04/24 23:16:36, Yoyo Zhou wrote:
> nit: why this extra line?

Done.

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension_permission_set.h:450: // A set of oauth2
scopes that are specific to Google Accounts.
On 2012/04/24 23:16:36, Yoyo Zhou wrote:
> Can you elaborate on what these are used for in the comment (looks like the
> signed-in sync account)?

Done.

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
File chrome/common/extensions/extension_permission_set_unittest.cc (left):

http://codereview.chromium.org/10049006/diff/10001/chrome/common/extensions/e...
chrome/common/extensions/extension_permission_set_unittest.cc:468: const char*
granted_apis[10];
On 2012/04/24 23:16:36, Yoyo Zhou wrote:
> You pruned these because they were getting unmaintainable?

They were no longer being used in the actual test case below for some reason. We
created a list of granted APIs and hosts, but didn't do anything with them.

http://codereview.chromium.org/10049006/diff/10001/chrome/test/data/extension...
File chrome/test/data/extensions/allow_silent_upgrade/scopes1_new.json (right):

http://codereview.chromium.org/10049006/diff/10001/chrome/test/data/extension...
chrome/test/data/extensions/allow_silent_upgrade/scopes1_new.json:3: "version":
"1.0",
On 2012/04/24 23:16:36, Yoyo Zhou wrote:
> It's kinda weird that the version number doesn't increase from old to new.

Done.

[commit-bot: I haz the power, 2012-04-25 01:29:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jstritar@chromium.org/10049006/13002

[commit-bot: I haz the power, 2012-04-25 03:20:21]

Change committed as 133858