Add a separate warning message for topSites permission.

Add a separate warning message for topSites permission.

The current warning is the generic "Read your browsing history"
warning. The new warning for topSites will be suppressed by
read only and read/write history warnings, and all hosts warning.

BUG=404334
Committed: https://crrev.com/a859ef1cc6d20bd833c84cd2d126cf49440b16f7
Cr-Commit-Position: refs/heads/master@{#292545}

[not at google - send to devlin, 2014-08-18 22:59:42]

While I consider the bikeshed, a question.

https://codereview.chromium.org/482953003/diff/1/chrome/common/extensions/per...
File chrome/common/extensions/permissions/chrome_permission_message_provider.cc
(right):

https://codereview.chromium.org/482953003/diff/1/chrome/common/extensions/per...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:84:
if (permissions->HasEffectiveFullAccess()) {
Why did you need to move this up?

[not at google - send to devlin, 2014-08-18 23:04:46]

This working lgtm.

[meacer, 2014-08-18 23:11:07]

https://codereview.chromium.org/482953003/diff/1/chrome/common/extensions/per...
File chrome/common/extensions/permissions/chrome_permission_message_provider.cc
(right):

https://codereview.chromium.org/482953003/diff/1/chrome/common/extensions/per...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:84:
if (permissions->HasEffectiveFullAccess()) {
On 2014/08/18 22:59:42, kalman wrote:
> Why did you need to move this up?

Early return. The suppress list isn't needed if we already have full access.

[not at google - send to devlin, 2014-08-18 23:17:30]

https://codereview.chromium.org/482953003/diff/1/chrome/common/extensions/per...
File chrome/common/extensions/permissions/chrome_permission_message_provider.cc
(right):

https://codereview.chromium.org/482953003/diff/1/chrome/common/extensions/per...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:84:
if (permissions->HasEffectiveFullAccess()) {
On 2014/08/18 23:11:07, Mustafa Emre Acer wrote:
> On 2014/08/18 22:59:42, kalman wrote:
> > Why did you need to move this up?
> 
> Early return. The suppress list isn't needed if we already have full access.

Ah right. Full as in ***FULL*** not like all-urls. Got it.

aside:

Seems like this is only "plugin" - but I thought automation.desktop was also
going to have that, given it can drive your whole desktop? That means that this
GetPermissionMessages method is too primitive for the general case. You'd need
to wire that into the automation manifest handling somehow.

[meacer, 2014-08-18 23:31:13]

https://codereview.chromium.org/482953003/diff/1/chrome/common/extensions/per...
File chrome/common/extensions/permissions/chrome_permission_message_provider.cc
(right):

https://codereview.chromium.org/482953003/diff/1/chrome/common/extensions/per...
chrome/common/extensions/permissions/chrome_permission_message_provider.cc:84:
if (permissions->HasEffectiveFullAccess()) {
On 2014/08/18 23:17:30, kalman wrote:
> On 2014/08/18 23:11:07, Mustafa Emre Acer wrote:
> > On 2014/08/18 22:59:42, kalman wrote:
> > > Why did you need to move this up?
> > 
> > Early return. The suppress list isn't needed if we already have full access.
> 
> Ah right. Full as in ***FULL*** not like all-urls. Got it.
> 
> aside:
> 
> Seems like this is only "plugin" - but I thought automation.desktop was also
> going to have that, given it can drive your whole desktop? That means that
this
> GetPermissionMessages method is too primitive for the general case. You'd need
> to wire that into the automation manifest handling somehow.

That looks like a bug to me with the automation API message generation? If I
have "automation" and all sites, I get two warnings for full access and
all-urls.

[not at google - send to devlin, 2014-08-18 23:34:07]

> > Seems like this is only "plugin" - but I thought automation.desktop was also
> > going to have that, given it can drive your whole desktop? That means that
> this
> > GetPermissionMessages method is too primitive for the general case. You'd
need
> > to wire that into the automation manifest handling somehow.
> 
> That looks like a bug to me with the automation API message generation? If I
> have "automation" and all sites, I get two warnings for full access and
> all-urls.

Yeah that sounds like a bug, weird that you would get *2*? I would expect the
suppression not to work since the permissions there are done separately, but...
2?

[meacer, 2014-08-28 00:00:22]

I filed crbug.com/408387 for automation. I'm also going to go with the string in
this CL ("Read a list of your most frequently visited websites")

[meacer, 2014-08-28 21:35:13]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2014-08-28 21:36:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/meacer@chromium.org/482953003/20001

[commit-bot: I haz the power, 2014-08-29 01:54:57]

Committed patchset #2 (id:20001) as 20c69310966d680e328cf9756ba9e9f210f93663

[commit-bot: I haz the power, 2014-09-10 03:05:09]

Patchset 2 (id:??) landed as
https://crrev.com/a859ef1cc6d20bd833c84cd2d126cf49440b16f7
Cr-Commit-Position: refs/heads/master@{#292545}