Disable Form-Not-Secure warning when |autofill_client_| is null

Disable Form-Not-Secure warning when |autofill_client_| is null

This is a temporary fix for an M57 crasher when Form-Not-Secure is
enabled.

PasswordAutofillManager doesn't generally seem to allow for the possibility that
|autofill_client_| is null, yet it's possible for it to be null, in the case of
the SimpleWebViewDialog ChromeOS captive portal login. In a follow-up, we should
figure out whether PasswordAutofillManager is intended to support a null
|autofill_client_| and also whether SimpleWebViewDialog is supposed to work
with password autofill.

BUG=678149
TBR=engedy@chromium.org

Review-Url: https://codereview.chromium.org/2733043005
Cr-Commit-Position: refs/heads/master@{#455213}
Committed: https://chromium.googlesource.com/chromium/src/+/88c753c103d76fd3697f7fcef962011588e3975f

[estark, 2017-03-07 18:30:53]

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

[commit-bot: I haz the power, 2017-03-07 18:31:40]

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

[estark, 2017-03-07 18:32:40]

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

[estark, 2017-03-07 18:32:41]

meacer, I realized we probably need to merge this to M57 which goes to stable
soon, so I think we should do the simplest possible fix as a temporary patch.
WDYT?

[meacer, 2017-03-07 18:39:22]

On 2017/03/07 18:32:41, estark wrote:
> meacer, I realized we probably need to merge this to M57 which goes to stable
> soon, so I think we should do the simplest possible fix as a temporary patch.
> WDYT?

This sounds reasonable, but let me patch this locally and see if we hit any
other crashes.

[meacer, 2017-03-07 19:30:07]

Okay, patch looking good, I don't get any crashes. Lgtm.

https://codereview.chromium.org/2733043005/diff/1/components/password_manager...
File components/password_manager/core/browser/password_autofill_manager.cc
(right):

https://codereview.chromium.org/2733043005/diff/1/components/password_manager...
components/password_manager/core/browser/password_autofill_manager.cc:265: if
(!autofill_client_)
You might want to file a new bug to re-enable Form not secure and mention it
here, maybe in the form of TODO(crbug.com/<newbug>).

[estark, 2017-03-07 19:51:34]

Thanks for doing the testing!

https://codereview.chromium.org/2733043005/diff/1/components/password_manager...
File components/password_manager/core/browser/password_autofill_manager.cc
(right):

https://codereview.chromium.org/2733043005/diff/1/components/password_manager...
components/password_manager/core/browser/password_autofill_manager.cc:265: if
(!autofill_client_)
On 2017/03/07 19:30:07, Mustafa Emre Acer wrote:
> You might want to file a new bug to re-enable Form not secure and mention it
> here, maybe in the form of TODO(crbug.com/<newbug>).

Filed a bug at https://crbug.com/699217. I feel a little weird talking about the
specifics of ChromeOS captive portal login here, so I expanded my existing
comment here to say that ideally we'd have non-null autofill_client_ in all
cases and linked to that new bug.

[estark, 2017-03-07 19:54:20]

Description was changed from

==========
Disable Form-Not-Secure warning when |autofill_client_| is null

This is a temporary fix for an M57 crasher when Form-Not-Secure is
enabled.

PasswordAutofillManager doesn't generally seem to allow for the possibility that
|autofill_client_| is null, yet it's possible for it to be null, in the case of
the SimpleWebViewDialog ChromeOS captive portal login. In a follow-up, we should
figure out whether PasswordAutofillManager is intended to support a null
|autofill_client_| and also whether SimpleWebViewDialog is supposed to work
with password autofill.

BUG=678149
==========

to

==========
Disable Form-Not-Secure warning when |autofill_client_| is null

This is a temporary fix for an M57 crasher when Form-Not-Secure is
enabled.

PasswordAutofillManager doesn't generally seem to allow for the possibility that
|autofill_client_| is null, yet it's possible for it to be null, in the case of
the SimpleWebViewDialog ChromeOS captive portal login. In a follow-up, we should
figure out whether PasswordAutofillManager is intended to support a null
|autofill_client_| and also whether SimpleWebViewDialog is supposed to work
with password autofill.

BUG=678149
TBR=engedy@chromium.org
==========

[estark, 2017-03-07 19:54:45]

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

[estark, 2017-03-07 19:54:46]

engedy: TBRing this small patch as it fixes a crasher and we want to merge to
M57 ASAP. Thanks!

[estark, 2017-03-07 19:54:55]

The CQ bit was checked by estark@chromium.org

[estark, 2017-03-07 19:54:55]

The patchset sent to the CQ was uploaded after l-g-t-m from meacer@chromium.org
Link to the patchset: https://codereview.chromium.org/2733043005/#ps20001
(title: "meacer comment")

[commit-bot: I haz the power, 2017-03-07 19:56:17]

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, 2017-03-07 20:56:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488916495081150,
"parent_rev": "d342a948f3809c5f65941cf454c068771718cf0a", "commit_rev":
"88c753c103d76fd3697f7fcef962011588e3975f"}

[commit-bot: I haz the power, 2017-03-07 20:57:22]

Description was changed from

==========
Disable Form-Not-Secure warning when |autofill_client_| is null

This is a temporary fix for an M57 crasher when Form-Not-Secure is
enabled.

PasswordAutofillManager doesn't generally seem to allow for the possibility that
|autofill_client_| is null, yet it's possible for it to be null, in the case of
the SimpleWebViewDialog ChromeOS captive portal login. In a follow-up, we should
figure out whether PasswordAutofillManager is intended to support a null
|autofill_client_| and also whether SimpleWebViewDialog is supposed to work
with password autofill.

BUG=678149
TBR=engedy@chromium.org
==========

to

==========
Disable Form-Not-Secure warning when |autofill_client_| is null

This is a temporary fix for an M57 crasher when Form-Not-Secure is
enabled.

PasswordAutofillManager doesn't generally seem to allow for the possibility that
|autofill_client_| is null, yet it's possible for it to be null, in the case of
the SimpleWebViewDialog ChromeOS captive portal login. In a follow-up, we should
figure out whether PasswordAutofillManager is intended to support a null
|autofill_client_| and also whether SimpleWebViewDialog is supposed to work
with password autofill.

BUG=678149
TBR=engedy@chromium.org

Review-Url: https://codereview.chromium.org/2733043005
Cr-Commit-Position: refs/heads/master@{#455213}
Committed:
https://chromium.googlesource.com/chromium/src/+/88c753c103d76fd3697f7fcef962...
==========

[commit-bot: I haz the power, 2017-03-07 20:57:23]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/88c753c103d76fd3697f7fcef962...

[engedy, 2017-03-08 12:35:04]

LGTM.