Parse the script_badge manifest section.

Parse the script_badge manifest section.

This isn't wired into much yet.

BUG=132853
TEST=unit_tests --gtest_filter=ExtensionManifestTest*
browser_tests --gtest_filter=ExtensionBrowserTest*
browser_tests --gtest_filter=PageAsBrowserAction*


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

[Jeffrey Yasskin, 2012-06-16 00:04:30]

I haven't actually watched this do something to a running browser, so let me
know if I've headed off in the wrong direction. Or if I'm doing stylistically
silly things anywhere.

[Aaron Boodman, 2012-06-16 01:50:38]

https://chromiumcodereview.appspot.com/10565017/diff/1/chrome/common/extensio...
File chrome/common/extensions/extension.cc (right):

https://chromiumcodereview.appspot.com/10565017/diff/1/chrome/common/extensio...
chrome/common/extensions/extension.cc:843: *error =
ASCIIToUTF16(errors::kScriptBadgeCannotSpecifyIcon);
Same here, I don't think we should throw an error in this case. If you want you
could make it an install_warning.

https://chromiumcodereview.appspot.com/10565017/diff/1/chrome/common/extensio...
chrome/common/extensions/extension.cc:2293: if
(!switch_utils::IsActionBoxEnabled()) {
Note: This is going to become AreScriptBadgesEnabled() soon (kalman has cl
pending).

https://chromiumcodereview.appspot.com/10565017/diff/1/chrome/common/extensio...
chrome/common/extensions/extension.cc:2294: if
(manifest_->HasKey(keys::kScriptBadge)) {
This is not necessary. The manifest key will be locked to dev or canary channel.
We can just ignore the API when it is used w/o script badges enabled.

https://chromiumcodereview.appspot.com/10565017/diff/1/chrome/common/extensio...
chrome/common/extensions/extension.cc:2318: // Script badges always use their
extension's badges so users can
extension's icon

https://chromiumcodereview.appspot.com/10565017/diff/1/chrome/test/data/exten...
File chrome/test/data/extensions/manifest_tests/script_badge_basic.json (right):

https://chromiumcodereview.appspot.com/10565017/diff/1/chrome/test/data/exten...
chrome/test/data/extensions/manifest_tests/script_badge_basic.json:8: "19":
"icon19.png",
Are you going to add support for the 19px size (that would be cool).

[not at google - send to devlin, 2012-06-18 18:51:24]

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
File chrome/browser/extensions/script_badge_controller.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
chrome/browser/extensions/script_badge_controller.cc:30: << "Without this,
extension->script_badge() would return NULL.";
Is this necessary? We only construct these objects when this flag (soon-to-be
enable-script-badges) is true. If anything, (D)CHECK below that script_badge
returns non-NULL.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2293: if
(!switch_utils::IsActionBoxEnabled()) {
On 2012/06/16 01:50:38, Aaron Boodman wrote:
> Note: This is going to become AreScriptBadgesEnabled() soon (kalman has cl
> pending).

It's in the CQ.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2296: return false;
Aaron are you saying to delete this whole block that's guarded by
IsActionBoxEnabled / AreScriptBadgesEnabled?

If so, yeah, I think that's sensible. It's unfortunate that this code
necessitates all that extra code in switch_utils. If we handle the lack of the
switch at a higher level (e.g. only constructing the ScriptBadgeController if
the flag is on, as we currently do) then that simplifies things a lot.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2309: return false;  // Failed to parse
page action definition.
s/page action/script badge/

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension_manifest_constants.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension_manifest_constants.cc:487: const char
kScriptBadgeRequiresActionBox[] =
ditto

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension_manifest_constants.h (right):

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension_manifest_constants.h:325: extern const char
kScriptBadgeRequiresActionBox[];
Should be able to delete this.

[Aaron Boodman, 2012-06-18 21:36:21]

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2296: return false;
On 2012/06/18 18:51:24, kalman wrote:
> Aaron are you saying to delete this whole block that's guarded by
> IsActionBoxEnabled / AreScriptBadgesEnabled?

yes, or rather:

if (!manifest->HasKey(...))
  return true;

[Jeffrey Yasskin, 2012-06-18 22:16:03]

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
File chrome/browser/extensions/script_badge_controller.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
chrome/browser/extensions/script_badge_controller.cc:30: << "Without this,
extension->script_badge() would return NULL.";
On 2012/06/18 18:51:24, kalman wrote:
> Is this necessary? We only construct these objects when this flag (soon-to-be
> enable-script-badges) is true. If anything, (D)CHECK below that script_badge
> returns non-NULL.

The CHECK is intended to tell people reading the code that you only construct
these objects when this flag is true. As the code was, I had to ask you. I could
remove the "Without this..." comment if you want.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:843: *error =
ASCIIToUTF16(errors::kScriptBadgeCannotSpecifyIcon);
On 2012/06/16 01:50:38, Aaron Boodman wrote:
> Same here, I don't think we should throw an error in this case. If you want
you
> could make it an install_warning.

Turned it into an install_warning that only developers see.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2293: if
(!switch_utils::IsActionBoxEnabled()) {
On 2012/06/18 18:51:24, kalman wrote:
> On 2012/06/16 01:50:38, Aaron Boodman wrote:
> > Note: This is going to become AreScriptBadgesEnabled() soon (kalman has cl
> > pending).
> 
> It's in the CQ.

I haven't synced to this yet, but I will soon.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2294: if
(manifest_->HasKey(keys::kScriptBadge)) {
On 2012/06/16 01:50:38, Aaron Boodman wrote:
> This is not necessary. The manifest key will be locked to dev or canary
channel.
> We can just ignore the API when it is used w/o script badges enabled.

I'm a bit worried about developers using the script_badge section and then
getting confused when it's ignored. I'll make this a warning for location==LOAD
like you suggested for default_icon.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2296: return false;
On 2012/06/18 21:36:21, Aaron Boodman wrote:
> On 2012/06/18 18:51:24, kalman wrote:
> > Aaron are you saying to delete this whole block that's guarded by
> > IsActionBoxEnabled / AreScriptBadgesEnabled?
> 
> yes, or rather:
> 
> if (!manifest->HasKey(...))
>   return true;

We need to generate a script_badge object even when there's no script_badge
section in the manifest, to support the ScriptBadgeController, right?

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2309: return false;  // Failed to parse
page action definition.
On 2012/06/18 18:51:24, kalman wrote:
> s/page action/script badge/

Whoops, done.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.cc:2318: // Script badges always use their
extension's badges so users can
On 2012/06/16 01:50:38, Aaron Boodman wrote:
> extension's icon

Done.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension_manifest_constants.h (right):

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension_manifest_constants.h:325: extern const char
kScriptBadgeRequiresActionBox[];
On 2012/06/18 18:51:24, kalman wrote:
> Should be able to delete this.

I still have warnings for developers that uses these constants. If y'all think
those warnings aren't worth having, I'll remove them.

http://codereview.chromium.org/10565017/diff/1/chrome/test/data/extensions/ma...
File chrome/test/data/extensions/manifest_tests/script_badge_basic.json (right):

http://codereview.chromium.org/10565017/diff/1/chrome/test/data/extensions/ma...
chrome/test/data/extensions/manifest_tests/script_badge_basic.json:8: "19":
"icon19.png",
On 2012/06/16 01:50:38, Aaron Boodman wrote:
> Are you going to add support for the 19px size (that would be cool).

I wasn't really planning to, since that increases the potential for hiding the
script badge's identity. This test was actually checking that the code _doesn't_
use the 19px size.

Still, if the extension chooses not to use the 16px badge for anything else,
they could just as easily use that size to spoof another extension. We should
find a general way to block this...

[not at google - send to devlin, 2012-06-18 22:59:41]

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
File chrome/browser/extensions/script_badge_controller.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
chrome/browser/extensions/script_badge_controller.cc:30: << "Without this,
extension->script_badge() would return NULL.";
On 2012/06/18 22:16:03, Jeffrey Yasskin wrote:
> On 2012/06/18 18:51:24, kalman wrote:
> > Is this necessary? We only construct these objects when this flag
(soon-to-be
> > enable-script-badges) is true. If anything, (D)CHECK below that script_badge
> > returns non-NULL.
> 
> The CHECK is intended to tell people reading the code that you only construct
> these objects when this flag is true. As the code was, I had to ask you. I
could
> remove the "Without this..." comment if you want.

Maybe just a comment would be more appropriate then?

// Note: this class will only be constructed from ExtensionTabHelper if
// script badges have been enabled.

or something. And a DCHECK if you really want.

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension_switch_utils.h (right):

http://codereview.chromium.org/10565017/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension_switch_utils.h:18: // interaction with
CommandLine::HasSwitch and doesn't require globals.
yeah, seems like this functionality would be nice to have in command_line.h or
around there.

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:423: if (location() == LOAD)
Install warnings only show up when developer mode has been turned on anyway...
as in, the "developer" is a tautology.

(ny all means have a function for adding a single warning, but only needs to be
called AddInstallWarning)

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:2303: if
(!switch_utils::IsActionBoxEnabled()) {
(has been submitted now btw)

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:2308: return true;
Could we just generate a warning here and not early-exit? I know that it would
result in redundantly generating a script badge if the flag is off, but I don't
think that's a big deal (it's just going to be turned on by default soon anyway)
and it means that we have less logic that's divergent on command line flags (and
logic that's divergent on command line flags is confusing and increasingly hard
to follow). It also means no setting up command line flag stuff for unit tests
as I mentioned earlier.

E.g. how about just:

// So as to not confuse developers if they specify a script badge
// section in the manifest, show a warning if the script badge
// declaration isn't going to have any effect.
if (!switch_utils::AreScriptBadgeEnabled() &&
     manifest_HasKey(keys::kScriptBadge)) {
  AddInstallWarning();
}

[Aaron Boodman, 2012-06-18 23:17:35]

lgtm

http://codereview.chromium.org/10565017/diff/1/chrome/test/data/extensions/ma...
File chrome/test/data/extensions/manifest_tests/script_badge_basic.json (right):

http://codereview.chromium.org/10565017/diff/1/chrome/test/data/extensions/ma...
chrome/test/data/extensions/manifest_tests/script_badge_basic.json:8: "19":
"icon19.png",
On 2012/06/18 22:16:03, Jeffrey Yasskin wrote:
> On 2012/06/16 01:50:38, Aaron Boodman wrote:
> > Are you going to add support for the 19px size (that would be cool).
> 
> I wasn't really planning to, since that increases the potential for hiding the
> script badge's identity. This test was actually checking that the code
_doesn't_
> use the 19px size.
> 
> Still, if the extension chooses not to use the 16px badge for anything else,
> they could just as easily use that size to spoof another extension. We should
> find a general way to block this...

I wrote this before you and I discussed in person.

I think what we ended up with is best path for now:

1) Use 16px icon from manifest if present
2) Fall back to puzzle piece

We can ask the designers to design us a cute treatment for the 16px icon if
necessary (depending on how this ends up looking).

[Jeffrey Yasskin, 2012-06-18 23:58:41]

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
File chrome/browser/extensions/script_badge_controller.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
chrome/browser/extensions/script_badge_controller.cc:30: << "Without this,
extension->script_badge() would return NULL.";
On 2012/06/18 22:59:41, kalman wrote:
> On 2012/06/18 22:16:03, Jeffrey Yasskin wrote:
> > On 2012/06/18 18:51:24, kalman wrote:
> > > Is this necessary? We only construct these objects when this flag
> (soon-to-be
> > > enable-script-badges) is true. If anything, (D)CHECK below that
script_badge
> > > returns non-NULL.
> > 
> > The CHECK is intended to tell people reading the code that you only
construct
> > these objects when this flag is true. As the code was, I had to ask you. I
> could
> > remove the "Without this..." comment if you want.
> 
> Maybe just a comment would be more appropriate then?
> 
> // Note: this class will only be constructed from ExtensionTabHelper if
> // script badges have been enabled.
> 
> or something. And a DCHECK if you really want.

I put the comment inside the DCHECK.

For future reference, what's the convention for deciding between CHECK and
DCHECK when flagging impossible situations?

http://codereview.chromium.org/10565017/diff/1/chrome/test/data/extensions/ma...
File chrome/test/data/extensions/manifest_tests/script_badge_basic.json (right):

http://codereview.chromium.org/10565017/diff/1/chrome/test/data/extensions/ma...
chrome/test/data/extensions/manifest_tests/script_badge_basic.json:8: "19":
"icon19.png",
On 2012/06/18 23:17:35, Aaron Boodman wrote:
> On 2012/06/18 22:16:03, Jeffrey Yasskin wrote:
> > On 2012/06/16 01:50:38, Aaron Boodman wrote:
> > > Are you going to add support for the 19px size (that would be cool).
> > 
> > I wasn't really planning to, since that increases the potential for hiding
the
> > script badge's identity. This test was actually checking that the code
> _doesn't_
> > use the 19px size.
> > 
> > Still, if the extension chooses not to use the 16px badge for anything else,
> > they could just as easily use that size to spoof another extension. We
should
> > find a general way to block this...
> 
> I wrote this before you and I discussed in person.
> 
> I think what we ended up with is best path for now:
> 
> 1) Use 16px icon from manifest if present
> 2) Fall back to puzzle piece

Ah, ok. I'll use MATCH_EXACTLY instead.

> We can ask the designers to design us a cute treatment for the 16px icon if
> necessary (depending on how this ends up looking).

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:423: if (location() == LOAD)
On 2012/06/18 22:59:41, kalman wrote:
> Install warnings only show up when developer mode has been turned on anyway...
> as in, the "developer" is a tautology.
> 
> (ny all means have a function for adding a single warning, but only needs to
be
> called AddInstallWarning)

My idea here was that it would show up for extensions that are under development
but not extensions that were merely installed. There's another example of this
for the manifest_version_==1 warning, but I don't see any others.

If it shouldn't filter on location()==LOAD, I don't think we need a helper
function for this, so I've removed the filter.

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:2303: if
(!switch_utils::IsActionBoxEnabled()) {
On 2012/06/18 22:59:41, kalman wrote:
> (has been submitted now btw)

And I've now synced past it.

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:2308: return true;
On 2012/06/18 22:59:41, kalman wrote:
> Could we just generate a warning here and not early-exit? I know that it would
> result in redundantly generating a script badge if the flag is off, but I
don't
> think that's a big deal (it's just going to be turned on by default soon
anyway)
> and it means that we have less logic that's divergent on command line flags
(and
> logic that's divergent on command line flags is confusing and increasingly
hard
> to follow). It also means no setting up command line flag stuff for unit tests
> as I mentioned earlier.
> 
> E.g. how about just:
> 
> // So as to not confuse developers if they specify a script badge
> // section in the manifest, show a warning if the script badge
> // declaration isn't going to have any effect.
> if (!switch_utils::AreScriptBadgeEnabled() &&
>      manifest_HasKey(keys::kScriptBadge)) {
>   AddInstallWarning();
> }

Sure. Then script_badge_ should never be null.

[not at google - send to devlin, 2012-06-19 00:27:32]

lgtm, don't want to hold you up any more, but do consider reverting those
changes that make it necessary to scope command line flags. I don't think that's
the right level to be enforcing these things.

(as I said, all the --enable-script-badges flag is meant to do is turn it on/off
in the UI, it doesn't need to be watertight feature checking at the Extension
level if it's going to complicate testing).

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
File chrome/browser/extensions/script_badge_controller.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
chrome/browser/extensions/script_badge_controller.cc:30: << "Without this,
extension->script_badge() would return NULL.";
On 2012/06/18 23:58:41, Jeffrey Yasskin wrote:
> On 2012/06/18 22:59:41, kalman wrote:
> > On 2012/06/18 22:16:03, Jeffrey Yasskin wrote:
> > > On 2012/06/18 18:51:24, kalman wrote:
> > > > Is this necessary? We only construct these objects when this flag
> > (soon-to-be
> > > > enable-script-badges) is true. If anything, (D)CHECK below that
> script_badge
> > > > returns non-NULL.
> > > 
> > > The CHECK is intended to tell people reading the code that you only
> construct
> > > these objects when this flag is true. As the code was, I had to ask you. I
> > could
> > > remove the "Without this..." comment if you want.
> > 
> > Maybe just a comment would be more appropriate then?
> > 
> > // Note: this class will only be constructed from ExtensionTabHelper if
> > // script badges have been enabled.
> > 
> > or something. And a DCHECK if you really want.
> 
> I put the comment inside the DCHECK.
> 
> For future reference, what's the convention for deciding between CHECK and
> DCHECK when flagging impossible situations?

I think of DCHECK as documentation and CHECK as bug-catching.

So in this case, if ScriptBadgeController was instantiated without that flag, no
biggie. But it can't possibly be because forces outside its control never do so.

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10565017/diff/8001/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:423: if (location() == LOAD)
On 2012/06/18 23:58:42, Jeffrey Yasskin wrote:
> On 2012/06/18 22:59:41, kalman wrote:
> > Install warnings only show up when developer mode has been turned on
anyway...
> > as in, the "developer" is a tautology.
> > 
> > (ny all means have a function for adding a single warning, but only needs to
> be
> > called AddInstallWarning)
> 
> My idea here was that it would show up for extensions that are under
development
> but not extensions that were merely installed. There's another example of this
> for the manifest_version_==1 warning, but I don't see any others.
> 
> If it shouldn't filter on location()==LOAD, I don't think we need a helper
> function for this, so I've removed the filter.

Yeah, makes sense. I'm just saying that this happens automatically kind of,
because most people don't have developer mode turned on.

http://codereview.chromium.org/10565017/diff/8017/chrome/common/extensions/ex...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10565017/diff/8017/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:2296:
manifest_->HasKey(keys::kScriptBadge)) {
I meant just make this branch a log, no logic... avoid logic (besides log
statements) guarded by command-line switches... doesn't matter. I preferred what
you had before.

As in, all --enable-script-badges needs to do is control them in the UI.
Anything beyond that is implementation detail, and trying to protect ourselves
against all these cases might just end up tying ourselves in knots.

http://codereview.chromium.org/10565017/diff/8017/chrome/common/extensions/ma...
File
chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc
(right):

http://codereview.chromium.org/10565017/diff/8017/chrome/common/extensions/ma...
chrome/common/extensions/manifest_tests/extension_manifests_scriptbadge_unittest.cc:19:
switch_utils::ScopedSetScriptBadgeForTest _(switch_utils::ENABLED);
the point of those comments was to get rid of needing these
enable-switch-blocks. I don't think this should be part of what Extension needs
to worry about.

[Jeffrey Yasskin, 2012-06-19 21:43:08]

I think I see the overall strategy for command-line switches now. This version
should be closer. I'll try to commit now, but a false positive in the presubmit
is likely to stop me, so I'll need to ask you to patch in the change and commit
it for me.

Thanks for patiently explaining this. :)

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
File chrome/browser/extensions/script_badge_controller.cc (right):

http://codereview.chromium.org/10565017/diff/1/chrome/browser/extensions/scri...
chrome/browser/extensions/script_badge_controller.cc:30: << "Without this,
extension->script_badge() would return NULL.";
On 2012/06/19 00:27:32, kalman wrote:
> So in this case, if ScriptBadgeController was instantiated without that flag,
no
> biggie. But it can't possibly be because forces outside its control never do
so.

Now that script_badge() always returns non-NULL, I think the CHECK isn't needed,
and it requires the switch-overriding machinery, so it's gone.

http://codereview.chromium.org/10565017/diff/8017/chrome/common/extensions/ex...
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/10565017/diff/8017/chrome/common/extensions/ex...
chrome/common/extensions/extension.cc:2296:
manifest_->HasKey(keys::kScriptBadge)) {
On 2012/06/19 00:27:32, kalman wrote:
> I meant just make this branch a log, no logic... avoid logic (besides log
> statements) guarded by command-line switches... doesn't matter. I preferred
what
> you had before.
> 
> As in, all --enable-script-badges needs to do is control them in the UI.
> Anything beyond that is implementation detail, and trying to protect ourselves
> against all these cases might just end up tying ourselves in knots.

I think I see what you're going for and why. Now the flag only affects an
install warning.

[commit-bot: I haz the power, 2012-06-19 21:46:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jyasskin@chromium.org/10565017/17001

[commit-bot: I haz the power, 2012-06-19 23:12:57]

Change committed as 143075