Add a new flag for HTTP-bad Phase 1 development work

Add a new flag for HTTP-bad Phase 1 development work

This extends the #mark-non-secure-as flag to use for the HTTP-bad phase
1 (passwords and CC form detection) work. It currently behaves exactly
like the existing kMarkNonSecureAsNeutral switch, although presumably
that will change as we make progress.

BUG=646221
Committed: https://crrev.com/c7a6a6f68877cc8346cb7fa6bfa9e5eab7148385
Cr-Commit-Position: refs/heads/master@{#418860}

[felt, 2016-09-13 13:23:02]

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

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

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

[felt, 2016-09-13 13:23:32]

Description was changed from

==========
Add a new flag for HTTP-bad Phase 1 development work

This extends the #mark-non-secure-as flag to use for the HTTP-bad phase
1 (passwords and CC form detection) work. It currently behaves exactly
like the existing kMarkNonSecureAsNeutral switch, although presumably
that will change as we make progress.

BUG=646221
==========

to

==========
Add a new flag for HTTP-bad Phase 1 development work

This extends the #mark-non-secure-as flag to use for the HTTP-bad phase
1 (passwords and CC form detection) work. It currently behaves exactly
like the existing kMarkNonSecureAsNeutral switch, although presumably
that will change as we make progress.

BUG=646221
==========

[felt, 2016-09-13 13:23:32]

felt@chromium.org changed reviewers:
+ elawrence@chromium.org

[felt, 2016-09-13 13:23:40]

elawrence, PTAL?

[elawrence, 2016-09-13 17:56:31]

Looks good to me. A few nit'y questions.

https://codereview.chromium.org/2337103002/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2337103002/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:5258: +        Change the UI treatment for
non-secure (HTTP) origins.
Including periods in dropdown items is unusual on Windows.

That said, you are keeping the existing precedent for this flag, and this isn't
the only flag in chrome://flags to include a period (though most don't).

https://codereview.chromium.org/2337103002/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2337103002/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:204:
{IDS_MARK_NON_SECURE_AS_NON_SECURE_WITH_PASSWORDS,
Grammatically, IDS_MARK_NON_SECURE_AS_NON_SECURE_WITH_PASSWORDS feels a bit odd,
vs. IDS_MARK_NON_SECURE_WITH_PASSWORDS_AS_NON_SECURE, but maybe this was
intentional for alphebetized grouping purposes?

https://codereview.chromium.org/2337103002/diff/20001/components/security_sta...
File components/security_state/switches.cc (right):

https://codereview.chromium.org/2337103002/diff/20001/components/security_sta...
components/security_state/switches.cc:14: const char
kMarkNonSecureWhenPasswordOrCc[] = "non-secure-passwords";
Is this a temporary flag, or do we plan to keep it as the feature evolves? Even
in this first draft, it's somewhat misleading; maybe we could use
"non-secure-if-sensitive" or something and keep that as the experiment evolves
to other triggers?

https://codereview.chromium.org/2337103002/diff/20001/components/security_sta...
File components/security_state/switches.h (right):

https://codereview.chromium.org/2337103002/diff/20001/components/security_sta...
components/security_state/switches.h:14: extern const char
kMarkNonSecureWhenPasswordOrCc[];
Maybe kMarkNonSecureWithPasswordOrCc for consistency with variable naming?

[felt, 2016-09-13 18:39:47]

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

[felt, 2016-09-13 18:39:51]

https://codereview.chromium.org/2337103002/diff/20001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2337103002/diff/20001/chrome/app/generated_re...
chrome/app/generated_resources.grd:5258: +        Change the UI treatment for
non-secure (HTTP) origins.
On 2016/09/13 17:56:31, elawrence wrote:
> Including periods in dropdown items is unusual on Windows.
> 
> That said, you are keeping the existing precedent for this flag, and this
isn't
> the only flag in chrome://flags to include a period (though most don't).

Done.

https://codereview.chromium.org/2337103002/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2337103002/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:204:
{IDS_MARK_NON_SECURE_AS_NON_SECURE_WITH_PASSWORDS,
On 2016/09/13 17:56:31, elawrence wrote:
> Grammatically, IDS_MARK_NON_SECURE_AS_NON_SECURE_WITH_PASSWORDS feels a bit
odd,
> vs. IDS_MARK_NON_SECURE_WITH_PASSWORDS_AS_NON_SECURE, but maybe this was
> intentional for alphebetized grouping purposes?

Was just for consistency's sake, no strong preference tho. Updated.

https://codereview.chromium.org/2337103002/diff/20001/components/security_sta...
File components/security_state/switches.cc (right):

https://codereview.chromium.org/2337103002/diff/20001/components/security_sta...
components/security_state/switches.cc:14: const char
kMarkNonSecureWhenPasswordOrCc[] = "non-secure-passwords";
On 2016/09/13 17:56:31, elawrence wrote:
> Is this a temporary flag, or do we plan to keep it as the feature evolves?
Even
> in this first draft, it's somewhat misleading; maybe we could use
> "non-secure-if-sensitive" or something and keep that as the experiment evolves
> to other triggers?

If we do other triggers beyond passwords and CC, we should use a separate flag
level for that one to keep it separate. This flag will be specific to the
rollout for phase 1 and shouldn't be repurposed.

https://codereview.chromium.org/2337103002/diff/20001/components/security_sta...
File components/security_state/switches.h (right):

https://codereview.chromium.org/2337103002/diff/20001/components/security_sta...
components/security_state/switches.h:14: extern const char
kMarkNonSecureWhenPasswordOrCc[];
On 2016/09/13 17:56:31, elawrence wrote:
> Maybe kMarkNonSecureWithPasswordOrCc for consistency with variable naming?

Done.

[commit-bot: I haz the power, 2016-09-13 18:40:20]

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-13 19:40:52]

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

[commit-bot: I haz the power, 2016-09-13 19:40:53]

Dry run: This issue passed the CQ dry run.

[felt, 2016-09-14 20:15:55]

elawrence, sorry to rush but this CL is blocking other people's work. any other
changes you'd like, or looks good to you?

[elawrence, 2016-09-14 20:30:48]

On 2016/09/14 20:15:55, felt wrote:
> elawrence, sorry to rush but this CL is blocking other people's work. any
other
> changes you'd like, or looks good to you?

Oh, this totally LGTM but I believe I was told not to use LGTM until I have
committer privileges. Maybe I'm misremembering.

[felt, 2016-09-15 00:01:44]

On 2016/09/14 20:30:48, elawrence wrote:
> On 2016/09/14 20:15:55, felt wrote:
> > elawrence, sorry to rush but this CL is blocking other people's work. any
> other
> > changes you'd like, or looks good to you?
> 
> Oh, this totally LGTM but I believe I was told not to use LGTM until I have
> committer privileges. Maybe I'm misremembering.

For posterity, reviewers-in-training can say "looks good" or "l-g-t-m" to let
the reviewee know you're done, without officially triggering the magical
approval logic. Thanks!

[felt, 2016-09-15 00:01:56]

felt@chromium.org changed reviewers:
+ estark@chromium.org

[felt, 2016-09-15 00:02:08]

estark, ptal?

[estark, 2016-09-15 04:30:30]

lgtm

https://codereview.chromium.org/2337103002/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2337103002/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:5266: +      <message
name="IDS_MARK_NON_SECURE_WITH_PASSWORDS_AS_NON_SECURE">
optional nit: WITH_PASSWORDS => WITH_PASSWORDS_OR_CC (or is that too annoyingly
long?)

https://codereview.chromium.org/2337103002/diff/40001/components/security_sta...
File components/security_state/switches.cc (right):

https://codereview.chromium.org/2337103002/diff/40001/components/security_sta...
components/security_state/switches.cc:14: const char
kMarkNonSecureWithPasswordsAsNonSecure[] = "non-secure-passwords";
same optional nit here about using "PasswordsOrCC" or some such instead of
"passwords"

https://codereview.chromium.org/2337103002/diff/40001/components/security_sta...
File components/security_state/switches.h (right):

https://codereview.chromium.org/2337103002/diff/40001/components/security_sta...
components/security_state/switches.h:14: extern const char
kMarkNonSecureWithPasswordsAsNonSecure[];
nit: either add another newline after this line, or delete the one on line 10

[felt, 2016-09-15 12:55:30]

https://codereview.chromium.org/2337103002/diff/40001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2337103002/diff/40001/chrome/app/generated_re...
chrome/app/generated_resources.grd:5266: +      <message
name="IDS_MARK_NON_SECURE_WITH_PASSWORDS_AS_NON_SECURE">
On 2016/09/15 04:30:29, estark wrote:
> optional nit: WITH_PASSWORDS => WITH_PASSWORDS_OR_CC (or is that too
annoyingly
> long?)

I was debating the length but I'm feeling plucky this morning so let's go with
the longer one

https://codereview.chromium.org/2337103002/diff/40001/components/security_sta...
File components/security_state/switches.cc (right):

https://codereview.chromium.org/2337103002/diff/40001/components/security_sta...
components/security_state/switches.cc:14: const char
kMarkNonSecureWithPasswordsAsNonSecure[] = "non-secure-passwords";
On 2016/09/15 04:30:29, estark wrote:
> same optional nit here about using "PasswordsOrCC" or some such instead of
> "passwords"

Done.

https://codereview.chromium.org/2337103002/diff/40001/components/security_sta...
File components/security_state/switches.h (right):

https://codereview.chromium.org/2337103002/diff/40001/components/security_sta...
components/security_state/switches.h:14: extern const char
kMarkNonSecureWithPasswordsAsNonSecure[];
On 2016/09/15 04:30:29, estark wrote:
> nit: either add another newline after this line, or delete the one on line 10

Done.

[felt, 2016-09-15 12:55:39]

The CQ bit was checked by felt@chromium.org

[felt, 2016-09-15 12:55:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
elawrence@chromium.org, estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2337103002/#ps80001
(title: "Clean up dirty tree")

[commit-bot: I haz the power, 2016-09-15 12:55:50]

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-15 14:09:46]

Description was changed from

==========
Add a new flag for HTTP-bad Phase 1 development work

This extends the #mark-non-secure-as flag to use for the HTTP-bad phase
1 (passwords and CC form detection) work. It currently behaves exactly
like the existing kMarkNonSecureAsNeutral switch, although presumably
that will change as we make progress.

BUG=646221
==========

to

==========
Add a new flag for HTTP-bad Phase 1 development work

This extends the #mark-non-secure-as flag to use for the HTTP-bad phase
1 (passwords and CC form detection) work. It currently behaves exactly
like the existing kMarkNonSecureAsNeutral switch, although presumably
that will change as we make progress.

BUG=646221
==========

[commit-bot: I haz the power, 2016-09-15 14:09:47]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-15 14:11:28]

Description was changed from

==========
Add a new flag for HTTP-bad Phase 1 development work

This extends the #mark-non-secure-as flag to use for the HTTP-bad phase
1 (passwords and CC form detection) work. It currently behaves exactly
like the existing kMarkNonSecureAsNeutral switch, although presumably
that will change as we make progress.

BUG=646221
==========

to

==========
Add a new flag for HTTP-bad Phase 1 development work

This extends the #mark-non-secure-as flag to use for the HTTP-bad phase
1 (passwords and CC form detection) work. It currently behaves exactly
like the existing kMarkNonSecureAsNeutral switch, although presumably
that will change as we make progress.

BUG=646221

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

[commit-bot: I haz the power, 2016-09-15 14:11:29]

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