Disable autocomplete with unknown autocomplete attribute value.

Disable autocomplete with unknown autocomplete attribute value.

This is for consistent behaviour between autofill and autocomplete.

Improved autofill_manager_unittest readability with a convenience method
for using a MockAutofillHistoryManager and test naming consistency.

BUG=632737
R=mathp,sebsg
TEST=AutofillManagerTest*

Committed: https://crrev.com/a652213cc933de7eed9adc9199117281a2965f08
Cr-Commit-Position: refs/heads/master@{#409935}

[asredzki, 2016-08-04 19:55:35]

Description was changed from

==========
Disable autocomplete with unknown autocomplete attribute value.

This is for consistent behaviour between autofill and autocomplete.

Improved autofill_manager_unittest readability with a convenience method
for using a MockAutofillHistoryManager and test naming consistency.

BUG=632737
R=mathp,sebsg
TEST=AutofillManagerTest*
==========

to

==========
Disable autocomplete with unknown autocomplete attribute value.

This is for consistent behaviour between autofill and autocomplete.

Improved autofill_manager_unittest readability with a convenience method
for using a MockAutofillHistoryManager and test naming consistency.

BUG=632737
R=mathp,sebsg
TEST=AutofillManagerTest*
==========

[asredzki, 2016-08-04 19:55:36]

asredzki@google.com changed reviewers:
+ mathp@chromium.org
- mathp@google.com

[Mathieu, 2016-08-04 20:57:19]

very cool lgtm with nit

https://codereview.chromium.org/2211193002/diff/1/components/autofill/core/br...
File components/autofill/core/browser/autofill_manager_unittest.cc (right):

https://codereview.chromium.org/2211193002/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager_unittest.cc:978:
autofill_manager_->autocomplete_history_manager_.reset(
to avoid the cast, you can:

MockAutocompleteHistoryManager* manager = new
MockAutocompleteHistoryManager(autofill_driver_.get(),
                                            autofill_manager_->client());
autofill_manager_->autocomplete_history_manager_.reset(manager);
return manager;

|manager| is valid as long as autocomplete_history_manager_ is not reset.

https://codereview.chromium.org/2211193002/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager_unittest.cc:982:
autofill_manager_->autocomplete_history_manager_.get());
indent is off here?

https://codereview.chromium.org/2211193002/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager_unittest.cc:1110:
MockAutocompleteHistoryManager* m = UseMockAutocompleteHistoryManager();
I don't like "Use", perhaps "Recreate"?

[sebsg, 2016-08-04 21:07:34]

Awesome! Thanks for changing the test names too :) LGTM

[asredzki, 2016-08-04 21:10:11]

Addressed nits

https://codereview.chromium.org/2211193002/diff/1/components/autofill/core/br...
File components/autofill/core/browser/autofill_manager_unittest.cc (right):

https://codereview.chromium.org/2211193002/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager_unittest.cc:978:
autofill_manager_->autocomplete_history_manager_.reset(
On 2016/08/04 20:57:19, Mathieu Perreault wrote:
> to avoid the cast, you can:
> 
> MockAutocompleteHistoryManager* manager = new
> MockAutocompleteHistoryManager(autofill_driver_.get(),
>                                             autofill_manager_->client());
> autofill_manager_->autocomplete_history_manager_.reset(manager);
> return manager;
> 
> |manager| is valid as long as autocomplete_history_manager_ is not reset.

Done.

https://codereview.chromium.org/2211193002/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager_unittest.cc:982:
autofill_manager_->autocomplete_history_manager_.get());
On 2016/08/04 20:57:19, Mathieu Perreault wrote:
> indent is off here?

Done.

https://codereview.chromium.org/2211193002/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager_unittest.cc:1110:
MockAutocompleteHistoryManager* m = UseMockAutocompleteHistoryManager();
On 2016/08/04 20:57:18, Mathieu Perreault wrote:
> I don't like "Use", perhaps "Recreate"?

Done.

[asredzki, 2016-08-04 21:10:31]

The CQ bit was checked by asredzki@google.com

[asredzki, 2016-08-04 21:10:32]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org,
sebsg@chromium.org
Link to the patchset: https://codereview.chromium.org/2211193002/#ps20001
(title: "Addressed nits")

[commit-bot: I haz the power, 2016-08-04 21:10:52]

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-08-04 22:41:45]

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

[commit-bot: I haz the power, 2016-08-04 22:41:47]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[asredzki, 2016-08-04 23:06:20]

The CQ bit was checked by asredzki@google.com

[commit-bot: I haz the power, 2016-08-04 23:06:44]

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-08-04 23:39:23]

Description was changed from

==========
Disable autocomplete with unknown autocomplete attribute value.

This is for consistent behaviour between autofill and autocomplete.

Improved autofill_manager_unittest readability with a convenience method
for using a MockAutofillHistoryManager and test naming consistency.

BUG=632737
R=mathp,sebsg
TEST=AutofillManagerTest*
==========

to

==========
Disable autocomplete with unknown autocomplete attribute value.

This is for consistent behaviour between autofill and autocomplete.

Improved autofill_manager_unittest readability with a convenience method
for using a MockAutofillHistoryManager and test naming consistency.

BUG=632737
R=mathp,sebsg
TEST=AutofillManagerTest*
==========

[commit-bot: I haz the power, 2016-08-04 23:39:24]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-04 23:41:07]

Description was changed from

==========
Disable autocomplete with unknown autocomplete attribute value.

This is for consistent behaviour between autofill and autocomplete.

Improved autofill_manager_unittest readability with a convenience method
for using a MockAutofillHistoryManager and test naming consistency.

BUG=632737
R=mathp,sebsg
TEST=AutofillManagerTest*
==========

to

==========
Disable autocomplete with unknown autocomplete attribute value.

This is for consistent behaviour between autofill and autocomplete.

Improved autofill_manager_unittest readability with a convenience method
for using a MockAutofillHistoryManager and test naming consistency.

BUG=632737
R=mathp,sebsg
TEST=AutofillManagerTest*

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

[commit-bot: I haz the power, 2016-08-04 23:41:09]

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