Prevent unnecessary prompts when unpacked extensions use chrome.permissions.request.

Prevent unnecessary prompts when unpacked extensions use chrome.permissions.request.

We now record what permissions have been granted to unpacked extensions to make developing against the permissions API simpler. With this change, chrome.permissions.request will generate the same prompts for packed and unpacked extensions. This also fixes an issue where we were not prompting for unpacked extensions with plugins at installation time.

BUG=108797
TEST=ExtensionApiTest.OptionalPermissions*


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

[jstritar, 2012-01-24 17:54:44]

Hey- can you take a look?

http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
chrome/common/extensions/extension.h:404: bool
CanSilentlyIncreasePermissionsWhileRunning() const;
Are these methods confusing?

[Yoyo Zhou, 2012-01-24 19:42:22]

http://codereview.chromium.org/9222013/diff/5001/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/permissions/permissions_apitest.cc (right):

http://codereview.chromium.org/9222013/diff/5001/chrome/browser/extensions/ap...
chrome/browser/extensions/api/permissions/permissions_apitest.cc:68:
IN_PROC_BROWSER_TEST_F(ExtensionApiTest, OptionalPermissionsGranted) {
Does this test exercise the new functionality? It does look like it's loaded
unpacked.

http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
chrome/common/extensions/extension.h:400: // development. We prompt at install
time for packaged extensions and unpacked
packaged -> packed

http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
chrome/common/extensions/extension.h:404: bool
CanSilentlyIncreasePermissionsWhileRunning() const;
On 2012/01/24 17:54:44, jstritar wrote:
> Are these methods confusing?

Maybe "upgrade" instead of "install"? (Does it make sense to increase something
during the first install?)

[Yoyo Zhou, 2012-01-24 19:45:31]

On 2012/01/24 19:42:22, Yoyo Zhou wrote:
>
http://codereview.chromium.org/9222013/diff/5001/chrome/browser/extensions/ap...
> File chrome/browser/extensions/api/permissions/permissions_apitest.cc (right):
> 
>
http://codereview.chromium.org/9222013/diff/5001/chrome/browser/extensions/ap...
> chrome/browser/extensions/api/permissions/permissions_apitest.cc:68:
> IN_PROC_BROWSER_TEST_F(ExtensionApiTest, OptionalPermissionsGranted) {
> Does this test exercise the new functionality? It does look like it's loaded
> unpacked.
> 
>
http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
> File chrome/common/extensions/extension.h (right):
> 
>
http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
> chrome/common/extensions/extension.h:400: // development. We prompt at install
> time for packaged extensions and unpacked
> packaged -> packed
> 
>
http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
> chrome/common/extensions/extension.h:404: bool
> CanSilentlyIncreasePermissionsWhileRunning() const;
> On 2012/01/24 17:54:44, jstritar wrote:
> > Are these methods confusing?
> 
> Maybe "upgrade" instead of "install"? (Does it make sense to increase
something
> during the first install?)

Oh, I see, it's for a re-install. And unpacked extensions can be downgraded...

[Yoyo Zhou, 2012-01-24 19:56:41]

LGTM, I guess I don't have better naming suggestions.

[jstritar, 2012-01-25 17:34:17]

http://codereview.chromium.org/9222013/diff/5001/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/permissions/permissions_apitest.cc (right):

http://codereview.chromium.org/9222013/diff/5001/chrome/browser/extensions/ap...
chrome/browser/extensions/api/permissions/permissions_apitest.cc:68:
IN_PROC_BROWSER_TEST_F(ExtensionApiTest, OptionalPermissionsGranted) {
Yes, because it will only succeed if we've stored granted permissions for the
extension. I also just added a unit test to verify the prompt behavior for LOAD
extensions that have plugins.

http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
chrome/common/extensions/extension.h:404: bool
CanSilentlyIncreasePermissionsWhileRunning() const;
I ended up going back to just one CanSilentlyIncreasePermissions method.

[commit-bot: I haz the power, 2012-01-25 20:40:54]

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

[Yoyo Zhou, 2012-01-25 20:41:34]

On 2012/01/25 17:34:17, jstritar wrote:
>
http://codereview.chromium.org/9222013/diff/5001/chrome/browser/extensions/ap...
> File chrome/browser/extensions/api/permissions/permissions_apitest.cc (right):
> 
>
http://codereview.chromium.org/9222013/diff/5001/chrome/browser/extensions/ap...
> chrome/browser/extensions/api/permissions/permissions_apitest.cc:68:
> IN_PROC_BROWSER_TEST_F(ExtensionApiTest, OptionalPermissionsGranted) {
> Yes, because it will only succeed if we've stored granted permissions for the
> extension. I also just added a unit test to verify the prompt behavior for
LOAD
> extensions that have plugins.
> 
>
http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
> File chrome/common/extensions/extension.h (right):
> 
>
http://codereview.chromium.org/9222013/diff/5001/chrome/common/extensions/ext...
> chrome/common/extensions/extension.h:404: bool
> CanSilentlyIncreasePermissionsWhileRunning() const;
> I ended up going back to just one CanSilentlyIncreasePermissions method.

I see. Still LGTM.

[commit-bot: I haz the power, 2012-01-25 23:00:18]

Change committed as 119135