[Password Generation] Fix a bug where we always upload ACCOUNT_CREATION_PASSWORD

[Password Generation] Fix a bug where we always upload ACCOUNT_CREATION_PASSWORD

Update test to catch the problem. Previous test mocked out
bad behavior.

BUG=419228
R=isherman@chromium.org

Committed: https://crrev.com/44c5e4ebc358b6e804ae7af97fd240ca5682743f
Cr-Commit-Position: refs/heads/master@{#297580}

[Garrett Casto, 2014-10-01 01:02:18]

gcasto@chromium.org changed reviewers:
+ isherman@chromium.org

[Ilya Sherman, 2014-10-01 02:17:29]

Non-test changes LGTM.  I suggest splitting the test changes off into a separate
CL, both for ease of merging and so you're not blocked on review comments for
them.

[Ilya Sherman, 2014-10-01 02:21:46]

Comments on test changes:

https://chromiumcodereview.appspot.com/614313002/diff/1/components/password_m...
File components/password_manager/core/browser/password_form_manager_unittest.cc
(right):

https://chromiumcodereview.appspot.com/614313002/diff/1/components/password_m...
components/password_manager/core/browser/password_form_manager_unittest.cc:57:
const autofill::ServerFieldType&));
nit: Please leave a blank line after this one.

https://chromiumcodereview.appspot.com/614313002/diff/1/components/password_m...
components/password_manager/core/browser/password_form_manager_unittest.cc:58:
private:
nit: DISALLOW_COPY_AND_ASSIGN

https://chromiumcodereview.appspot.com/614313002/diff/1/components/password_m...
components/password_manager/core/browser/password_form_manager_unittest.cc:77:
test_personal_data_manager_.reset();
nit: These four lines are unnecessary.

https://chromiumcodereview.appspot.com/614313002/diff/1/components/password_m...
components/password_manager/core/browser/password_form_manager_unittest.cc:97:
scoped_ptr<NiceMock<MockAutofillManager> > mock_autofill_manager_;
nit: Do these all need to be scoped_ptrs?  Seems like they can be directly
allocated instead.

https://chromiumcodereview.appspot.com/614313002/diff/1/components/password_m...
components/password_manager/core/browser/password_form_manager_unittest.cc:185:
client_.reset(new TestPasswordManagerClient(NULL /*password_store*/));
nit: Why did this move, out of curiosity?

https://chromiumcodereview.appspot.com/614313002/diff/1/components/password_m...
components/password_manager/core/browser/password_form_manager_unittest.cc:191:
client_.reset();
nit: Is it a problem if this doesn't happen until the destructor runs?

https://chromiumcodereview.appspot.com/614313002/diff/1/components/password_m...
components/password_manager/core/browser/password_form_manager_unittest.cc:551:
// Need a MessageLoop for callbacks.
nit: Should this comment be removed as well?

[Garrett Casto, 2014-10-01 02:25:23]

Removed testing changes for now. Will upload testing changes in a different CL.
Thanks for the quick review.

[commit-bot: I haz the power, 2014-10-01 02:27:26]

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

[Garrett Casto, 2014-10-01 02:27:27]

Committed patchset #2 (id:20001) manually as
44c5e4ebc358b6e804ae7af97fd240ca5682743f (presubmit successful).