Update of the extension install UI:

Update of the extension install UI:

- Give the user more information about which hosts an
  extension can access.
- Remove the red severe warning because it doesn't play well
  with themes and because it adds nothing when the other 
  text is more specific.
- Make the image a bit smaller.

Also integrate this new idea with the silent/not-silent update flow.

BUG=12129, 12140, 19582
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=24599

[Matt Perry, 2009-08-26 17:48:15]

looking good

http://codereview.chromium.org/173463/diff/1001/1002
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/173463/diff/1001/1002#newcode2110
Line 2110: <message name="IDS_EXTENSION_PROMPT_WARNING_NEW_BROWSER"
desc="Warning displayed in body of extension dialog when the extension requires
access to browser APIs.">
Is it not localizable to make this paramaterized, like "This extension will have
access to $1." and "This extension will have access to $1 and $2." ?

http://codereview.chromium.org/173463/diff/1001/1003
File chrome/browser/extensions/extension_install_ui.cc (right):

http://codereview.chromium.org/173463/diff/1001/1003#newcode34
Line 34: return l10n_util::GetString(IDS_EXTENSION_PROMPT_WARNING_NEW_BROWSER);
FULL_ACCESS

http://codereview.chromium.org/173463/diff/1001/1003#newcode64
Line 64: IDS_EXTENSION_PROMPT_WARNING_NEW_MULTIPLE_HOSTS_AND_BROWSER,
SINGLE_HOST

http://codereview.chromium.org/173463/diff/1001/1005
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/173463/diff/1001/1005#newcode387
Line 387: if (!Extension::AllowSilentUpgrade(old, extension)) {
old is already deleted and NULL here

http://codereview.chromium.org/173463/diff/1001/1007
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/173463/diff/1001/1007#newcode533
Line 533: // to remember the max set of permissions we ever granted a single
extension.
yeah.. but how often is that going to happen?

http://codereview.chromium.org/173463/diff/1001/1007#newcode556
Line 556: std::set_difference(new_hosts.begin(), new_hosts.end(),
nice

http://codereview.chromium.org/173463/diff/1001/1009
File chrome/common/extensions/extension_unittest.cc (right):

http://codereview.chromium.org/173463/diff/1001/1009#newcode488
Line 488: static Extension* LoadExtension(const std::string& dir,
nit: I think LoadManifest would be clearer

http://codereview.chromium.org/173463/diff/1001/1009#newcode602
Line 602: { "hosts1", true }, // http://a,http://b -> http://a,http://b
nit: 2 spaces before comment

http://codereview.chromium.org/173463/diff/1001/1009#newcode603
Line 603: { "hosts2", true }, // http://a,http://b -> https://a,http://*.
nit: http://*.b

http://codereview.chromium.org/173463/diff/1001/1009#newcode611
Line 611: { "plugin2", true}, // plugin ->
nit: plugin -> none

[Aaron Boodman, 2009-08-26 21:02:24]

http://codereview.chromium.org/173463/diff/1001/1002
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/173463/diff/1001/1002#newcode2110
Line 2110: <message name="IDS_EXTENSION_PROMPT_WARNING_NEW_BROWSER"
desc="Warning displayed in body of extension dialog when the extension requires
access to browser APIs.">
On 2009/08/26 17:48:15, Matt Perry wrote:
> Is it not localizable to make this paramaterized, like "This extension will
have
> access to $1." and "This extension will have access to $1 and $2." ?

For now I prefer having them all be distinct, even though it is more work and
more error prone, because it allows the wording to be tweaked to be perfect for
each case. For example, the wording in the full access case is slightly
different.

Also, I think it will be easier for the translators to understand what's going
on.

http://codereview.chromium.org/173463/diff/1001/1003
File chrome/browser/extensions/extension_install_ui.cc (right):

http://codereview.chromium.org/173463/diff/1001/1003#newcode34
Line 34: return l10n_util::GetString(IDS_EXTENSION_PROMPT_WARNING_NEW_BROWSER);
On 2009/08/26 17:48:15, Matt Perry wrote:
> FULL_ACCESS

Whoops, I had fixed this but forgot to upload the new snapshot.

http://codereview.chromium.org/173463/diff/1001/1003#newcode64
Line 64: IDS_EXTENSION_PROMPT_WARNING_NEW_MULTIPLE_HOSTS_AND_BROWSER,
On 2009/08/26 17:48:15, Matt Perry wrote:
> SINGLE_HOST

Ah, thanks for the careful check.

http://codereview.chromium.org/173463/diff/1001/1005
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/173463/diff/1001/1005#newcode387
Line 387: if (!Extension::AllowSilentUpgrade(old, extension)) {
On 2009/08/26 17:48:15, Matt Perry wrote:
> old is already deleted and NULL here

Whoops, I totally forgot to test this. Thanks for reminding me.

http://codereview.chromium.org/173463/diff/1001/1007
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/173463/diff/1001/1007#newcode533
Line 533: // to remember the max set of permissions we ever granted a single
extension.
On 2009/08/26 17:48:15, Matt Perry wrote:
> yeah.. but how often is that going to happen?

Agree, just noting it down to acknowledge to future readers that it is a known
issue.

[Matt Perry, 2009-08-26 21:10:32]

LGTM

http://codereview.chromium.org/173463/diff/1056/1060
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/173463/diff/1056/1060#newcode382
Line 382: bool allow_silent_upgrade = Extension::AllowSilentUpgrade(old,
extension);
80 cols