[omnibox] Support new flags for security chip

[omnibox] Support new flags for security chip

Have the Omnibox security chip honor the new flags. The new flags
say whether to animate the security chip, or display it at all.
For desktops except Mac.

BUG=647347
Committed: https://crrev.com/5f34c35d0c98b88cf71e25e5215288d97b258735
Cr-Commit-Position: refs/heads/master@{#419970}

[Kevin Bailey, 2016-09-20 17:49:51]

Description was changed from

==========
[omnibox] Support new flags for security chip

Have the Omnibox security chip honor the new flags. The new flags
say whether to animate the security chip, or display it at all.
For desktops except Mac.

BUG=647347
==========

to

==========
[omnibox] Support new flags for security chip

Have the Omnibox security chip honor the new flags. The new flags
say whether to animate the security chip, or display it at all.
For desktops except Mac.

BUG=647347
==========

[Kevin Bailey, 2016-09-20 17:49:51]

krb@chromium.org changed reviewers:
+ pkasting@chromium.org

[Kevin Bailey, 2016-09-20 17:50:46]

On 2016/09/20 17:49:51, Kevin Bailey wrote:
> mailto:krb@chromium.org changed reviewers:
> + mailto:pkasting@chromium.org

Hi Peter,

Just aligning flag support with Mac stuff.

[Peter Kasting, 2016-09-20 20:52:09]

LGTM

https://codereview.chromium.org/2351003003/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.h (right):

https://codereview.chromium.org/2351003003/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.h:488: bool
should_animate_secure_state_;
Nit: I'd just put these two together with a block comment like:

  // These allow toggling the verbose security state behavior via flags.

The actual functions are self-explanatory from the names.

https://codereview.chromium.org/2351003003/diff/1/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/2351003003/diff/1/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:634: const char kMaterialSecurityVerbose[] =
"material-security-verbose";
Nit: If we're mucking with this stuff anyway and it's still not in widespread
use, can we change the constant names and flag name to not use "material"? 
We're in the midst of deleting all the pre-MD code and this behavior isn't
really MD-gated anyway, so it's not really something that should be part of the
names.

[Kevin Bailey, 2016-09-21 02:40:14]

The CQ bit was checked by krb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-21 02:40:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-21 03:20:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-21 03:20:46]

Dry run: This issue passed the CQ dry run.

[Kevin Bailey, 2016-09-21 04:05:01]

https://codereview.chromium.org/2351003003/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.h (right):

https://codereview.chromium.org/2351003003/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.h:488: bool
should_animate_secure_state_;
On 2016/09/20 20:52:09, Peter Kasting wrote:
> Nit: I'd just put these two together with a block comment like:
> 
>   // These allow toggling the verbose security state behavior via flags.
> 
> The actual functions are self-explanatory from the names.

Done.

https://codereview.chromium.org/2351003003/diff/1/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/2351003003/diff/1/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:634: const char kMaterialSecurityVerbose[] =
"material-security-verbose";
On 2016/09/20 20:52:09, Peter Kasting wrote:
> Nit: If we're mucking with this stuff anyway and it's still not in widespread
> use, can we change the constant names and flag name to not use "material"? 
> We're in the midst of deleting all the pre-MD code and this behavior isn't
> really MD-gated anyway, so it's not really something that should be part of
the
> names.

Works for me. I'll put in separate CL to coordinate w/spqchan (for the variable
names, not the strings.)

[Kevin Bailey, 2016-09-21 04:05:06]

The CQ bit was checked by krb@chromium.org

[Kevin Bailey, 2016-09-21 04:05:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2351003003/#ps20001
(title: "Simplify class field definitions")

[commit-bot: I haz the power, 2016-09-21 04:05:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-21 04:11:03]

Description was changed from

==========
[omnibox] Support new flags for security chip

Have the Omnibox security chip honor the new flags. The new flags
say whether to animate the security chip, or display it at all.
For desktops except Mac.

BUG=647347
==========

to

==========
[omnibox] Support new flags for security chip

Have the Omnibox security chip honor the new flags. The new flags
say whether to animate the security chip, or display it at all.
For desktops except Mac.

BUG=647347
==========

[commit-bot: I haz the power, 2016-09-21 04:11:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-21 04:12:48]

Description was changed from

==========
[omnibox] Support new flags for security chip

Have the Omnibox security chip honor the new flags. The new flags
say whether to animate the security chip, or display it at all.
For desktops except Mac.

BUG=647347
==========

to

==========
[omnibox] Support new flags for security chip

Have the Omnibox security chip honor the new flags. The new flags
say whether to animate the security chip, or display it at all.
For desktops except Mac.

BUG=647347

Committed: https://crrev.com/5f34c35d0c98b88cf71e25e5215288d97b258735
Cr-Commit-Position: refs/heads/master@{#419970}
==========

[commit-bot: I haz the power, 2016-09-21 04:12:49]

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