MD Settings: test passwordsPrivate.get{SavedPassword,PasswordExceptions}List

MD Settings: test passwordsPrivate.get{SavedPassword,PasswordExceptions}List

Also, separate list initialization (blocks on less things).

R=rdevlin.cronin@chromium.org
BUG=657215
Committed: https://crrev.com/83bb086878243e77279db7ae9d973649a0453f45
Cr-Commit-Position: refs/heads/master@{#429183}

[Dan Beam, 2016-11-01 22:53:29]

Description was changed from

==========
MD Settings: test passwordsPrivate.get{SavedPassword,PasswordExceptions}List

Also, splits up initialization to allow each list to function separately

R=rdevlin.cronin@chromium.org
BUG=657215
==========

to

==========
MD Settings: test passwordsPrivate.get{SavedPassword,PasswordExceptions}List

Also, separate list initialization (blocks on less things).

R=rdevlin.cronin@chromium.org
BUG=657215
==========

[Dan Beam, 2016-11-01 22:54:49]

Patchset #1 (id:1) has been deleted

[Dan Beam, 2016-11-01 22:59:23]

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

[commit-bot: I haz the power, 2016-11-01 23:00:11]

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

[Devlin, 2016-11-01 23:45:08]

https://codereview.chromium.org/2466273004/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc
(right):

https://codereview.chromium.org/2466273004/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc:198:
bool run_initialization_callbacks = !current_entries_initialized_;
Couldn't we also do:
DCHECK(!current_entries_initialized_ ||
       get_saved_passwords_list_callbacks_.empty());
And then unconditionally run all of them (which would be zero if it's already
init'd)?

https://codereview.chromium.org/2466273004/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc
(right):

https://codereview.chromium.org/2466273004/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc:42:
CallbackTracker<PasswordsPrivateDelegate::UiEntries> tracker;
I worry a bit that this is verging on testing code rather than behavior... but I
also don't have a good concrete suggestion of what else to do.  At the very
least, though, it might be good to validate that we get the proper responses in
the callback?  The issue before wasn't so much that the callback was or wasn't
getting called as it was getting called with improper data.

[commit-bot: I haz the power, 2016-11-02 00:27:58]

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

[commit-bot: I haz the power, 2016-11-02 00:27:58]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-11-02 00:51:49]

https://codereview.chromium.org/2466273004/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc
(right):

https://codereview.chromium.org/2466273004/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc:198:
bool run_initialization_callbacks = !current_entries_initialized_;
On 2016/11/01 23:45:07, Devlin wrote:
> Couldn't we also do:
> DCHECK(!current_entries_initialized_ ||
>        get_saved_passwords_list_callbacks_.empty());
> And then unconditionally run all of them (which would be zero if it's already
> init'd)?

Done.

https://codereview.chromium.org/2466273004/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc
(right):

https://codereview.chromium.org/2466273004/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc:42:
CallbackTracker<PasswordsPrivateDelegate::UiEntries> tracker;
On 2016/11/01 23:45:07, Devlin wrote:
> I worry a bit that this is verging on testing code rather than behavior... but
I
> also don't have a good concrete suggestion of what else to do.  At the very
> least, though, it might be good to validate that we get the proper responses
in
> the callback?  The issue before wasn't so much that the callback was or wasn't
> getting called as it was getting called with improper data.

Done.(-ish)

[Dan Beam, 2016-11-02 00:52:04]

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

[commit-bot: I haz the power, 2016-11-02 00:52:44]

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

[Devlin, 2016-11-02 00:56:11]

lgtm

https://codereview.chromium.org/2466273004/diff/60001/chrome/browser/extensio...
File
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc
(right):

https://codereview.chromium.org/2466273004/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc:59:
list.push_back(std::move(form));
nitty nit:
list.push_back(base::MakeUnique<autofill::PasswordForm>());

(I'm not sure if
PasswordFormList list(1, base::MakeUnique<autofill::PasswordForm>());
would work, but I suspect not)

https://codereview.chromium.org/2466273004/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc:79:
list.push_back(std::move(form));
ditto

[Dan Beam, 2016-11-02 01:01:44]

https://codereview.chromium.org/2466273004/diff/60001/chrome/browser/extensio...
File
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc
(right):

https://codereview.chromium.org/2466273004/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc:59:
list.push_back(std::move(form));
On 2016/11/02 00:56:11, Devlin wrote:
> nitty nit:
> list.push_back(base::MakeUnique<autofill::PasswordForm>());
> 
> (I'm not sure if
> PasswordFormList list(1, base::MakeUnique<autofill::PasswordForm>());
> would work, but I suspect not)

Done.

https://codereview.chromium.org/2466273004/diff/60001/chrome/browser/extensio...
chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc:79:
list.push_back(std::move(form));
On 2016/11/02 00:56:11, Devlin wrote:
> ditto

Done.

[Dan Beam, 2016-11-02 01:01:50]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2016-11-02 01:01:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2466273004/#ps80001
(title: "asdf")

[commit-bot: I haz the power, 2016-11-02 01:02:27]

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-11-02 02:08:37]

Description was changed from

==========
MD Settings: test passwordsPrivate.get{SavedPassword,PasswordExceptions}List

Also, separate list initialization (blocks on less things).

R=rdevlin.cronin@chromium.org
BUG=657215
==========

to

==========
MD Settings: test passwordsPrivate.get{SavedPassword,PasswordExceptions}List

Also, separate list initialization (blocks on less things).

R=rdevlin.cronin@chromium.org
BUG=657215
==========

[commit-bot: I haz the power, 2016-11-02 02:08:39]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-11-02 02:10:58]

Description was changed from

==========
MD Settings: test passwordsPrivate.get{SavedPassword,PasswordExceptions}List

Also, separate list initialization (blocks on less things).

R=rdevlin.cronin@chromium.org
BUG=657215
==========

to

==========
MD Settings: test passwordsPrivate.get{SavedPassword,PasswordExceptions}List

Also, separate list initialization (blocks on less things).

R=rdevlin.cronin@chromium.org
BUG=657215

Committed: https://crrev.com/83bb086878243e77279db7ae9d973649a0453f45
Cr-Commit-Position: refs/heads/master@{#429183}
==========

[commit-bot: I haz the power, 2016-11-02 02:10:59]

Patchset 4 (id:??) landed as
https://crrev.com/83bb086878243e77279db7ae9d973649a0453f45
Cr-Commit-Position: refs/heads/master@{#429183}