Enable CredentialManagerBrowserTest.ObsoleteHttpCredentialMovedOnMigrationToHstsSite

Enable CredentialManagerBrowserTest.ObsoleteHttpCredentialMovedOnMigrationToHstsSite

The CL does
- introduce a new observer for the password UI state,
- uses it for ObsoleteHttpCredentialMovedOnMigrationToHstsSite,
- fixes all other tests broken by PasswordManagerBrowserTestBase changes.

BUG=703305
Review-Url: https://codereview.chromium.org/2759353004
Cr-Commit-Position: refs/heads/master@{#458721}
Committed: https://chromium.googlesource.com/chromium/src/+/9a3e6e43c4bdce4bdb64cbeb3f5bcf61d35e2ff7

[vasilii, 2017-03-21 14:23:07]

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

[commit-bot: I haz the power, 2017-03-21 14:23:29]

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

[vasilii, 2017-03-21 14:25:04]

vasilii@chromium.org changed reviewers:
+ vabr@chromium.org

[vasilii, 2017-03-21 14:25:04]

Hi Vaclav,

please review the CL.

[commit-bot: I haz the power, 2017-03-21 14:54:13]

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

[commit-bot: I haz the power, 2017-03-21 14:54:13]

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

[vasilii, 2017-03-21 15:29:00]

Many tests are still unhappy, but you can review the general idea so far.

[vasilii, 2017-03-21 17:02:10]

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

[commit-bot: I haz the power, 2017-03-21 17:03:16]

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

[vasilii, 2017-03-21 18:01:37]

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

[commit-bot: I haz the power, 2017-03-21 18:02:18]

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

[vabr (Chromium), 2017-03-21 18:46:52]

LGTM with nits, thanks for the robust solution!

BTW., if you learned something interesting about the initial sequence of how
PasswordAutofillAgent and the PasswordManagerDrivers come to life, I'd be happy
to hear next week. As you noticed, it is a mostly mysterious process to me.

Cheers,
Vaclav

https://codereview.chromium.org/2759353004/diff/30001/chrome/browser/password...
File chrome/browser/password_manager/credential_manager_browsertest.cc (right):

https://codereview.chromium.org/2759353004/diff/30001/chrome/browser/password...
chrome/browser/password_manager/credential_manager_browsertest.cc:198:
WaitForPasswordStore();
It would still be helpful to comment (in the code) on what PasswordStore query
this represents. There must have been one to produce the account chooser
already, so please comment on what the current query is for.

https://codereview.chromium.org/2759353004/diff/30001/chrome/browser/password...
File chrome/browser/password_manager/password_manager_browsertest.cc (right):

https://codereview.chromium.org/2759353004/diff/30001/chrome/browser/password...
chrome/browser/password_manager/password_manager_browsertest.cc:332:
BubbleObserver prompt_observer(WebContents());
optional nit: This could be as well called |bubble_observer|. The "prompt" (and
the past unique_ptr) comes from times when we had bubbles and infobars on
desktop, and needed a term capturing both.

[commit-bot: I haz the power, 2017-03-21 18:49:35]

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

[commit-bot: I haz the power, 2017-03-21 18:49:36]

Dry run: This issue passed the CQ dry run.

[vasilii, 2017-03-22 09:28:58]

I didn't learn about PasswordAutofillAgent because it is in the renderer.
PasswordManagerDriver wasn't created for me because I didn't cause the
navigation. I created a tab but didn't load anything into it. Thus, the fix was
to load some html first and then retrieve the PasswordManagerDriver.

https://codereview.chromium.org/2759353004/diff/30001/chrome/browser/password...
File chrome/browser/password_manager/credential_manager_browsertest.cc (right):

https://codereview.chromium.org/2759353004/diff/30001/chrome/browser/password...
chrome/browser/password_manager/credential_manager_browsertest.cc:198:
WaitForPasswordStore();
On 2017/03/21 18:46:51, vabr (Chromium) wrote:
> It would still be helpful to comment (in the code) on what PasswordStore query
> this represents. There must have been one to produce the account chooser
> already, so please comment on what the current query is for.

Done.

https://codereview.chromium.org/2759353004/diff/30001/chrome/browser/password...
File chrome/browser/password_manager/password_manager_browsertest.cc (right):

https://codereview.chromium.org/2759353004/diff/30001/chrome/browser/password...
chrome/browser/password_manager/password_manager_browsertest.cc:332:
BubbleObserver prompt_observer(WebContents());
On 2017/03/21 18:46:52, vabr (Chromium) wrote:
> optional nit: This could be as well called |bubble_observer|. The "prompt"
(and
> the past unique_ptr) comes from times when we had bubbles and infobars on
> desktop, and needed a term capturing both.

Done.

[vasilii, 2017-03-22 09:29:17]

Description was changed from

==========
Enable
CredentialManagerBrowserTest.ObsoleteHttpCredentialMovedOnMigrationToHstsSite

The CL does
- introduce a new observer for the password UI state,
- uses it for ObsoleteHttpCredentialMovedOnMigrationToHstsSite,
- fixes all other tests broken by PasswordManagerBrowserTestBase changes.

BUG=703305
==========

to

==========
Enable
CredentialManagerBrowserTest.ObsoleteHttpCredentialMovedOnMigrationToHstsSite

The CL does
- introduce a new observer for the password UI state,
- uses it for ObsoleteHttpCredentialMovedOnMigrationToHstsSite,
- fixes all other tests broken by PasswordManagerBrowserTestBase changes.

BUG=703305
==========

[vasilii, 2017-03-22 09:30:10]

The CQ bit was checked by vasilii@chromium.org

[vasilii, 2017-03-22 09:30:11]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org
Link to the patchset: https://codereview.chromium.org/2759353004/#ps50001
(title: "comments")

[commit-bot: I haz the power, 2017-03-22 09:30:38]

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-22 09:53:28]

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

[commit-bot: I haz the power, 2017-03-22 09:53:29]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[vasilii, 2017-03-22 11:40:12]

The CQ bit was checked by vasilii@chromium.org

[commit-bot: I haz the power, 2017-03-22 11:40:21]

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-22 11:44:37]

CQ is committing da patch.

Bot data: {"patchset_id": 50001, "attempt_start_ts": 1490182812964630,
"parent_rev": "fb687dc277807b111a99eba3c3bf9740c30ea431", "commit_rev":
"9a3e6e43c4bdce4bdb64cbeb3f5bcf61d35e2ff7"}

[commit-bot: I haz the power, 2017-03-22 11:45:24]

Description was changed from

==========
Enable
CredentialManagerBrowserTest.ObsoleteHttpCredentialMovedOnMigrationToHstsSite

The CL does
- introduce a new observer for the password UI state,
- uses it for ObsoleteHttpCredentialMovedOnMigrationToHstsSite,
- fixes all other tests broken by PasswordManagerBrowserTestBase changes.

BUG=703305
==========

to

==========
Enable
CredentialManagerBrowserTest.ObsoleteHttpCredentialMovedOnMigrationToHstsSite

The CL does
- introduce a new observer for the password UI state,
- uses it for ObsoleteHttpCredentialMovedOnMigrationToHstsSite,
- fixes all other tests broken by PasswordManagerBrowserTestBase changes.

BUG=703305

Review-Url: https://codereview.chromium.org/2759353004
Cr-Commit-Position: refs/heads/master@{#458721}
Committed:
https://chromium.googlesource.com/chromium/src/+/9a3e6e43c4bdce4bdb64cbeb3f5b...
==========

[commit-bot: I haz the power, 2017-03-22 11:45:25]

Committed patchset #4 (id:50001) as
https://chromium.googlesource.com/chromium/src/+/9a3e6e43c4bdce4bdb64cbeb3f5b...