Extension install prompt: in the delegated permissions prompt, include optional permissions

Extension install prompt: in the delegated permissions prompt, include optional permissions

Followup to https://codereview.chromium.org/850283003.

BUG=397951
Committed: https://crrev.com/7496f63cbefd34b2d791022fbad64a82838a3f3f
Cr-Commit-Position: refs/heads/master@{#319055}

[Marc Treib, 2015-03-02 14:55:59]

treib@chromium.org changed reviewers:
+ kalman@chromium.org

[Marc Treib, 2015-03-02 14:56:00]

Small followup to the showPermissionPrompt... API. PTAL!

[not at google - send to devlin, 2015-03-02 17:52:00]

https://codereview.chromium.org/968183002/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_install_prompt.cc (right):

https://codereview.chromium.org/968183002/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_install_prompt.cc:928: if (prompt_->type()
== DELEGATED_PERMISSIONS_PROMPT) {
Why?

[Marc Treib, 2015-03-02 18:15:41]

https://codereview.chromium.org/968183002/diff/1/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_install_prompt.cc (right):

https://codereview.chromium.org/968183002/diff/1/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_install_prompt.cc:928: if (prompt_->type()
== DELEGATED_PERMISSIONS_PROMPT) {
On 2015/03/02 17:52:00, kalman wrote:
> Why?

In delegated installs, the custodian "pre-approves" all optional permissions
(the supervised user can then enable them themselves). I've added a comment to
explain.

[not at google - send to devlin, 2015-03-02 18:29:11]

Thanks for the comment, that makes sense. Could you add a test here?
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

[Marc Treib, 2015-03-03 09:12:50]

On 2015/03/02 18:29:11, kalman wrote:
> Thanks for the comment, that makes sense. Could you add a test here?
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...

Test added; thanks for the pointer!

[not at google - send to devlin, 2015-03-03 17:33:26]

lgtm

https://codereview.chromium.org/968183002/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/extension_install_prompt_unittest.cc (right):

https://codereview.chromium.org/968183002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/extension_install_prompt_unittest.cc:107:
optional_permissions.Append("location");
Could you inline all of these (I don't know why the previous test doesn't)? The
point of ListBuilder is to inline things.

https://codereview.chromium.org/968183002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/extension_install_prompt_unittest.cc:114:
.Set("permissions", required_permissions)
E.g.


.Set("permissions",
     ListBuilder().Append("tabs"))
.Set("optional_permissions",
     ListBuilder().Append("location"))

[Marc Treib, 2015-03-04 10:33:43]

https://codereview.chromium.org/968183002/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/extension_install_prompt_unittest.cc (right):

https://codereview.chromium.org/968183002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/extension_install_prompt_unittest.cc:107:
optional_permissions.Append("location");
On 2015/03/03 17:33:26, kalman wrote:
> Could you inline all of these (I don't know why the previous test doesn't)?
The
> point of ListBuilder is to inline things.

Done, also in the previous test.

https://codereview.chromium.org/968183002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/extension_install_prompt_unittest.cc:114:
.Set("permissions", required_permissions)
On 2015/03/03 17:33:26, kalman wrote:
> E.g.
> 
> 
> .Set("permissions",
>      ListBuilder().Append("tabs"))
> .Set("optional_permissions",
>      ListBuilder().Append("location"))

Done.

[Marc Treib, 2015-03-04 10:33:48]

The CQ bit was checked by treib@chromium.org

[Marc Treib, 2015-03-04 10:33:49]

The patchset sent to the CQ was uploaded after l-g-t-m from kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/968183002/#ps60001 (title:
"cleanup")

[commit-bot: I haz the power, 2015-03-04 10:34:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/968183002/60001

[commit-bot: I haz the power, 2015-03-04 12:19:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-04 12:19:37]

Patchset 4 (id:??) landed as
https://crrev.com/7496f63cbefd34b2d791022fbad64a82838a3f3f
Cr-Commit-Position: refs/heads/master@{#319055}