Remove SecurityChips experiment and flags

Remove SecurityChips experiment and flags

We are launching security chips/verbose security states to stable in
M55, and thus removing the Finch experiments and command-line flags.

The behavior we are launching is show-all and animate-nonsecure-only
(security verbose states are shown for all states but animated for
non-secure only).

BUG=647762
Committed: https://crrev.com/b6c0b5412248ac3001905d439cd9ebd4b2d0f7ec
Cr-Commit-Position: refs/heads/master@{#438553}

[estark, 2016-12-09 22:40:07]

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

[commit-bot: I haz the power, 2016-12-09 22:41:14]

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

[estark, 2016-12-09 22:42:10]

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

[estark, 2016-12-09 22:42:11]

pkasting, spqchan, PTAL? Thanks!

[commit-bot: I haz the power, 2016-12-09 22:44:51]

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

[commit-bot: I haz the power, 2016-12-09 22:44:52]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[spqchan, 2016-12-09 22:46:50]

https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):

https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:667: return
(security == security_state::EV_SECURE ||
Nit: Don't think we need the round brackets

https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:888: return
(IsSecureConnection(level) ||
Are we animating all? I thought the plan was to animate nonsecure only

https://codereview.chromium.org/2560203003/diff/1/chrome/common/chrome_switch...
File chrome/common/chrome_switches.h (left):

https://codereview.chromium.org/2560203003/diff/1/chrome/common/chrome_switch...
chrome/common/chrome_switches.h:177: extern const char
kSecurityChipShowNonSecureOnly[];
You should also remove line 177 to 181

[estark, 2016-12-09 22:56:22]

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

[estark, 2016-12-09 22:56:25]

https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):

https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:667: return
(security == security_state::EV_SECURE ||
On 2016/12/09 22:46:50, spqchan wrote:
> Nit: Don't think we need the round brackets

Done.

https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:888: return
(IsSecureConnection(level) ||
On 2016/12/09 22:46:50, spqchan wrote:
> Are we animating all? I thought the plan was to animate nonsecure only

Hrm, where's that plan from? The server-side Finch config that is rolled out to
beta/dev/canary right now is animate-all/show-all.

https://codereview.chromium.org/2560203003/diff/1/chrome/common/chrome_switch...
File chrome/common/chrome_switches.h (left):

https://codereview.chromium.org/2560203003/diff/1/chrome/common/chrome_switch...
chrome/common/chrome_switches.h:177: extern const char
kSecurityChipShowNonSecureOnly[];
On 2016/12/09 22:46:50, spqchan wrote:
> You should also remove line 177 to 181

Done.

[commit-bot: I haz the power, 2016-12-09 22:57:25]

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

[spqchan, 2016-12-09 23:04:21]

LGTM if we're aiming for animate-all/show-all on stable

https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):

https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:888: return
(IsSecureConnection(level) ||
On 2016/12/09 22:56:25, estark wrote:
> On 2016/12/09 22:46:50, spqchan wrote:
> > Are we animating all? I thought the plan was to animate nonsecure only
> 
> Hrm, where's that plan from? The server-side Finch config that is rolled out
to
> beta/dev/canary right now is animate-all/show-all.

I remember hearing it from emilyschechter@ that we're leaning towards
animate-only-nonsecure/show-all for stable, and that we're likely sticking to
that. I'm not updated on any recent decisions though so it's possible that this
has changed. You should probably check with her

[estark, 2016-12-09 23:19:05]

On 2016/12/09 23:04:21, spqchan wrote:
> LGTM if we're aiming for animate-all/show-all on stable
> 
>
https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
> File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):
> 
>
https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
> chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:888: return
> (IsSecureConnection(level) ||
> On 2016/12/09 22:56:25, estark wrote:
> > On 2016/12/09 22:46:50, spqchan wrote:
> > > Are we animating all? I thought the plan was to animate nonsecure only
> > 
> > Hrm, where's that plan from? The server-side Finch config that is rolled out
> to
> > beta/dev/canary right now is animate-all/show-all.
> 
> I remember hearing it from emilyschechter@ that we're leaning towards
> animate-only-nonsecure/show-all for stable, and that we're likely sticking to
> that. I'm not updated on any recent decisions though so it's possible that
this
> has changed. You should probably check with her

Ok, checked with emilyschechter, and you're right. So I changed this to
animate-nonsecure-only, and will update the Finch configs.

[estark, 2016-12-09 23:19:27]

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

[commit-bot: I haz the power, 2016-12-09 23:20:13]

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-12-09 23:36:40]

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

[commit-bot: I haz the power, 2016-12-09 23:36:41]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[estark, 2016-12-09 23:43:32]

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

[commit-bot: I haz the power, 2016-12-09 23:44:14]

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

[spqchan, 2016-12-10 00:52:21]

On 2016/12/09 23:19:05, estark wrote:
> On 2016/12/09 23:04:21, spqchan wrote:
> > LGTM if we're aiming for animate-all/show-all on stable
> > 
> >
>
https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
> > File chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm (right):
> > 
> >
>
https://codereview.chromium.org/2560203003/diff/1/chrome/browser/ui/cocoa/loc...
> > chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm:888: return
> > (IsSecureConnection(level) ||
> > On 2016/12/09 22:56:25, estark wrote:
> > > On 2016/12/09 22:46:50, spqchan wrote:
> > > > Are we animating all? I thought the plan was to animate nonsecure only
> > > 
> > > Hrm, where's that plan from? The server-side Finch config that is rolled
out
> > to
> > > beta/dev/canary right now is animate-all/show-all.
> > 
> > I remember hearing it from emilyschechter@ that we're leaning towards
> > animate-only-nonsecure/show-all for stable, and that we're likely sticking
to
> > that. I'm not updated on any recent decisions though so it's possible that
> this
> > has changed. You should probably check with her
> 
> Ok, checked with emilyschechter, and you're right. So I changed this to
> animate-nonsecure-only, and will update the Finch configs.

Cool, the changes LGTM :)

[commit-bot: I haz the power, 2016-12-10 01:46:00]

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

[commit-bot: I haz the power, 2016-12-10 01:46:02]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[Peter Kasting, 2016-12-10 02:36:38]

I suggest updating the CL description to explicitly mention the behavior you're
standardizing on.

c/b/ui/{location_bar,views} LGTM

https://codereview.chromium.org/2560203003/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (left):

https://codereview.chromium.org/2560203003/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/location_bar_view.cc:156:
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
Nit: See if relevant #includes (command_line.h, chrome_switches.h, etc.) can be
removed.

https://codereview.chromium.org/2560203003/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/2560203003/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/location_bar_view.cc:907: return false;
Nit: This conditional can disappear entirely.

[estark, 2016-12-14 17:40:46]

Description was changed from

==========
Remove SecurityChips experiment and flags

We are launching security chips/verbose security states to stable in
M55, and thus removing the Finch experiments and command-line flags.

BUG=647762
==========

to

==========
Remove SecurityChips experiment and flags

We are launching security chips/verbose security states to stable in
M55, and thus removing the Finch experiments and command-line flags.

The behavior we are launching is show-all and animate-nonsecure-only
(security verbose states are shown for all states but animated for
non-secure only).

BUG=647762
==========

[estark, 2016-12-14 17:54:29]

Thanks. Updated the CL description as suggested.

https://codereview.chromium.org/2560203003/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (left):

https://codereview.chromium.org/2560203003/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/location_bar_view.cc:156:
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
On 2016/12/10 02:36:38, Peter Kasting wrote:
> Nit: See if relevant #includes (command_line.h, chrome_switches.h, etc.) can
be
> removed.

Done; removed command_line.h, chrome_switches.h, chrome_features.h.

https://codereview.chromium.org/2560203003/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/2560203003/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/location_bar/location_bar_view.cc:907: return false;
On 2016/12/10 02:36:38, Peter Kasting wrote:
> Nit: This conditional can disappear entirely.

Done.

[estark, 2016-12-14 17:54:49]

The CQ bit was checked by estark@chromium.org

[estark, 2016-12-14 17:54:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
spqchan@chromium.org, pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2560203003/#ps80001
(title: "pkasting comments")

[commit-bot: I haz the power, 2016-12-14 17:55:30]

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-12-14 18:47:14]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481738089102330,
"parent_rev": "59e2b7a0920281dce79a29ceb695aae4a858a45a", "commit_rev":
"dd60b0ef68324a9dc10263b2414cd53766cddd32"}

[commit-bot: I haz the power, 2016-12-14 18:47:40]

Description was changed from

==========
Remove SecurityChips experiment and flags

We are launching security chips/verbose security states to stable in
M55, and thus removing the Finch experiments and command-line flags.

The behavior we are launching is show-all and animate-nonsecure-only
(security verbose states are shown for all states but animated for
non-secure only).

BUG=647762
==========

to

==========
Remove SecurityChips experiment and flags

We are launching security chips/verbose security states to stable in
M55, and thus removing the Finch experiments and command-line flags.

The behavior we are launching is show-all and animate-nonsecure-only
(security verbose states are shown for all states but animated for
non-secure only).

BUG=647762

Review-Url: https://codereview.chromium.org/2560203003
==========

[commit-bot: I haz the power, 2016-12-14 18:47:41]

Committed patchset #6 (id:80001)

[commit-bot: I haz the power, 2016-12-14 18:51:33]

Description was changed from

==========
Remove SecurityChips experiment and flags

We are launching security chips/verbose security states to stable in
M55, and thus removing the Finch experiments and command-line flags.

The behavior we are launching is show-all and animate-nonsecure-only
(security verbose states are shown for all states but animated for
non-secure only).

BUG=647762

Review-Url: https://codereview.chromium.org/2560203003
==========

to

==========
Remove SecurityChips experiment and flags

We are launching security chips/verbose security states to stable in
M55, and thus removing the Finch experiments and command-line flags.

The behavior we are launching is show-all and animate-nonsecure-only
(security verbose states are shown for all states but animated for
non-secure only).

BUG=647762

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

[commit-bot: I haz the power, 2016-12-14 18:51:34]

Patchset 6 (id:??) landed as
https://crrev.com/b6c0b5412248ac3001905d439cd9ebd4b2d0f7ec
Cr-Commit-Position: refs/heads/master@{#438553}