[Views][Mac] Security Chip Default Value

[Views][Mac] Security Chip Default Value

- Hide the chip by default
- Show the EV certificate for all flag option

BUG=657498
Committed: https://crrev.com/eb99caf19d7d759e76b07a00f224c88571e30e4c
Cr-Commit-Position: refs/heads/master@{#426270}

[spqchan, 2016-10-19 16:37:55]

Description was changed from

==========
[Views][Mac] Always show the EV cert

BUG=
==========

to

==========
[Views][Mac] Security Chip Default Value

- Hide the chip by default
- Show the EV certificate for all flag option

BUG=657498
==========

[spqchan, 2016-10-19 16:37:55]

spqchan@chromium.org changed reviewers:
+ pkasting@chromium.org, rsesek@chromium.org

[spqchan, 2016-10-19 16:45:32]

PTAL

+rsesek for cocoa
+pkasting for views

[Peter Kasting, 2016-10-19 18:02:37]

LGTM

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

https://codereview.chromium.org/2435493003/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1016:
security_state::SecurityStateModel::EV_SECURE);
Nit: This function is only called in one place, and given its overlap with
ShouldShowSecurityChip() (it's conceptually confusing to me that both would
return true for EV_SECURE, which implies we're showing two different things), I
suggest just inlining it into its lone caller.

https://codereview.chromium.org/2435493003/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1024: else if (level
== SecurityLevel::EV_SECURE)
Nit: No else after return

Trivial, but I'd swap this conditional with the one above so that logically
these move from more to less secure.  (Of course, this makes me want to flip the
comparison order in the lines below, as well as in the two comparisons in the
next function.  Argh, OCD.)

[Robert Sesek, 2016-10-19 18:20:25]

lgtm

[spqchan, 2016-10-19 19:03:50]

thanks!

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

https://codereview.chromium.org/2435493003/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1016:
security_state::SecurityStateModel::EV_SECURE);
On 2016/10/19 18:02:36, Peter Kasting wrote:
> Nit: This function is only called in one place, and given its overlap with
> ShouldShowSecurityChip() (it's conceptually confusing to me that both would
> return true for EV_SECURE, which implies we're showing two different things),
I
> suggest just inlining it into its lone caller.

Done.

https://codereview.chromium.org/2435493003/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1024: else if (level
== SecurityLevel::EV_SECURE)
On 2016/10/19 18:02:36, Peter Kasting wrote:
> Nit: No else after return
> 
> Trivial, but I'd swap this conditional with the one above so that logically
> these move from more to less secure.  (Of course, this makes me want to flip
the
> comparison order in the lines below, as well as in the two comparisons in the
> next function.  Argh, OCD.)

Done.

[spqchan, 2016-10-19 19:03:55]

The CQ bit was checked by spqchan@chromium.org

[spqchan, 2016-10-19 19:03:55]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2435493003/#ps20001
(title: "Fix for pkasting")

[commit-bot: I haz the power, 2016-10-19 19:04:36]

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-10-19 20:34:01]

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

[commit-bot: I haz the power, 2016-10-19 20:34:06]

Failed to apply the patch.
On branch working_branch
Your branch is up-to-date with 'origin/refs/pending/heads/master'.
nothing to commit, working tree clean

[spqchan, 2016-10-19 21:28:36]

On 2016/10/19 20:34:06, commit-bot: I haz the power wrote:
> Failed to apply the patch.
> On branch working_branch
> Your branch is up-to-date with 'origin/refs/pending/heads/master'.
> nothing to commit, working tree clean

Weird, this totally landed. I'm going to just go ahead and close this...

The following revision refers to this bug:
 
https://chromium.googlesource.com/chromium/src.git/+/eb99caf19d7d759e76b07a00...

commit eb99caf19d7d759e76b07a00f224c88571e30e4c
Author: spqchan <spqchan@chromium.org>
Date: Wed Oct 19 20:31:06 2016

[Views][Mac] Security Chip Default Value

- Hide the chip by default
- Show the EV certificate for all flag option

BUG=657498

Review-Url: https://chromiumcodereview.appspot.com/2435493003
Cr-Commit-Position: refs/heads/master@{#426270}

[modify]
https://crrev.com/eb99caf19d7d759e76b07a00f224c88571e30e4c/chrome/browser/ui/...
[modify]
https://crrev.com/eb99caf19d7d759e76b07a00f224c88571e30e4c/chrome/browser/ui/...
[modify]
https://crrev.com/eb99caf19d7d759e76b07a00f224c88571e30e4c/chrome/browser/ui/...
[modify]
https://crrev.com/eb99caf19d7d759e76b07a00f224c88571e30e4c/chrome/browser/ui/...

[commit-bot: I haz the power, 2016-10-21 13:11:03]

Description was changed from

==========
[Views][Mac] Security Chip Default Value

- Hide the chip by default
- Show the EV certificate for all flag option

BUG=657498
==========

to

==========
[Views][Mac] Security Chip Default Value

- Hide the chip by default
- Show the EV certificate for all flag option

BUG=657498

Committed: https://crrev.com/eb99caf19d7d759e76b07a00f224c88571e30e4c
Cr-Commit-Position: refs/heads/master@{#426270}
==========