Convert more test helpers to base::RunLoop, fix page title checks. (Reland)

Convert more test helpers to base::RunLoop, fix page title checks. (Reland)

This CL removes the "deferred quit" from most of the touched helpers.

The exclusions are UrlCommitObserver and TestFrameNavigationObserver.
They were already using immediate quit mode, so there is no change in
their behavior, just cleanup.

This is a reland of https://codereview.chromium.org/2601843002. It
caused flakiness in a pair of ClickModifierTests, because I was wrong
when removed the WaitForLoadStop call. After that the TitleWatcher could
be created after the load start, but before the title arrives. In this
case WebContents::GetTitle basically returns the url, and TitleWatcher
was modified to simply return it.
Turns out that waiting this way (without specific expectations) for a
title can easily be done wrong, so I removed changes related to it.
These tests now use TestNavigationObserver, which should be robust
(unless there is some unrelated feature that can create WebContents at
a random time, which I believe is not the case in chromium browser tests).

The first patch set contains the original CL, the second one includes
the fix.

BUG=668707, 681035
Review-Url: https://codereview.chromium.org/2635933003
Cr-Commit-Position: refs/heads/master@{#444674}
Committed: https://chromium.googlesource.com/chromium/src/+/29276c3a4db55690390a4a2c7d072f21cd7b34d9

[Alexander Semashko, 2017-01-16 20:37:51]

The CQ bit was checked by ahest@yandex-team.ru to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 20:38:00]

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

[Alexander Semashko, 2017-01-16 20:39:17]

Description was changed from

==========
Convert more test helpers to base::RunLoop, fix page title checks. (Reland)

This CL removes the "deferred quit" from most of the touched helpers.

The exclusions are UrlCommitObserver and TestFrameNavigationObserver.
They were already using immediate quit mode, so there is no change in
their behavior, just cleanup.

This is a reland of https://codereview.chromium.org/2601843002. It
caused flakiness in a pair of ClickModifierTests, because I was wrong
when removed the WaitForLoadStop call. After that the TitleWatcher could
be created after the load start, but before the title arrives. In this
case WebContents::GetTitle basically returns the url, and TitleWatcher
was modified to simply return it.
Turns out that waiting this way (without specific expectations) for a
title can easily be done wrong, so I removed changes related to it.
These tests now use TestNavigationObserver, which should be robust
(unless there is some unrelated feature that can create WebContents at
a random time, which I believe is not the case in chromium browser tests).

The first patch set contains the original CL, the second one includes
the fix.

BUG=668707, 681035
==========

to

==========
Convert more test helpers to base::RunLoop, fix page title checks. (Reland)

This CL removes the "deferred quit" from most of the touched helpers.

The exclusions are UrlCommitObserver and TestFrameNavigationObserver.
They were already using immediate quit mode, so there is no change in
their behavior, just cleanup.

This is a reland of https://codereview.chromium.org/2601843002. It
caused flakiness in a pair of ClickModifierTests, because I was wrong
when removed the WaitForLoadStop call. After that the TitleWatcher could
be created after the load start, but before the title arrives. In this
case WebContents::GetTitle basically returns the url, and TitleWatcher
was modified to simply return it.
Turns out that waiting this way (without specific expectations) for a
title can easily be done wrong, so I removed changes related to it.
These tests now use TestNavigationObserver, which should be robust
(unless there is some unrelated feature that can create WebContents at
a random time, which I believe is not the case in chromium browser tests).

The first patch set contains the original CL, the second one includes
the fix.

BUG=668707, 681035
==========

[Alexander Semashko, 2017-01-16 20:39:18]

ahest@yandex-team.ru changed reviewers:
+ nasko@chromium.org, pkasting@chromium.org

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

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

[commit-bot: I haz the power, 2017-01-16 21:26:18]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2017-01-17 20:39:38]

LGTM

[Alexander Semashko, 2017-01-18 17:08:15]

nasko, please take a look

[nasko, 2017-01-18 23:44:19]

Surprised that only one test was using the title watcher for arbitrary title.
Less code is always good, so thanks a lot!

LGTM

[Alexander Semashko, 2017-01-19 06:54:08]

The CQ bit was checked by ahest@yandex-team.ru

[commit-bot: I haz the power, 2017-01-19 06:54:24]

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-01-19 07:36:35]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484808848908840,
"parent_rev": "04ff6239687ee823e9aa66254e4d506958366c7d", "commit_rev":
"29276c3a4db55690390a4a2c7d072f21cd7b34d9"}

[commit-bot: I haz the power, 2017-01-19 07:37:06]

Description was changed from

==========
Convert more test helpers to base::RunLoop, fix page title checks. (Reland)

This CL removes the "deferred quit" from most of the touched helpers.

The exclusions are UrlCommitObserver and TestFrameNavigationObserver.
They were already using immediate quit mode, so there is no change in
their behavior, just cleanup.

This is a reland of https://codereview.chromium.org/2601843002. It
caused flakiness in a pair of ClickModifierTests, because I was wrong
when removed the WaitForLoadStop call. After that the TitleWatcher could
be created after the load start, but before the title arrives. In this
case WebContents::GetTitle basically returns the url, and TitleWatcher
was modified to simply return it.
Turns out that waiting this way (without specific expectations) for a
title can easily be done wrong, so I removed changes related to it.
These tests now use TestNavigationObserver, which should be robust
(unless there is some unrelated feature that can create WebContents at
a random time, which I believe is not the case in chromium browser tests).

The first patch set contains the original CL, the second one includes
the fix.

BUG=668707, 681035
==========

to

==========
Convert more test helpers to base::RunLoop, fix page title checks. (Reland)

This CL removes the "deferred quit" from most of the touched helpers.

The exclusions are UrlCommitObserver and TestFrameNavigationObserver.
They were already using immediate quit mode, so there is no change in
their behavior, just cleanup.

This is a reland of https://codereview.chromium.org/2601843002. It
caused flakiness in a pair of ClickModifierTests, because I was wrong
when removed the WaitForLoadStop call. After that the TitleWatcher could
be created after the load start, but before the title arrives. In this
case WebContents::GetTitle basically returns the url, and TitleWatcher
was modified to simply return it.
Turns out that waiting this way (without specific expectations) for a
title can easily be done wrong, so I removed changes related to it.
These tests now use TestNavigationObserver, which should be robust
(unless there is some unrelated feature that can create WebContents at
a random time, which I believe is not the case in chromium browser tests).

The first patch set contains the original CL, the second one includes
the fix.

BUG=668707, 681035

Review-Url: https://codereview.chromium.org/2635933003
Cr-Commit-Position: refs/heads/master@{#444674}
Committed:
https://chromium.googlesource.com/chromium/src/+/29276c3a4db55690390a4a2c7d07...
==========

[commit-bot: I haz the power, 2017-01-19 07:37:07]

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

[Alexander Semashko, 2017-01-19 10:28:57]

On 2017/01/18 23:44:19, nasko wrote:
> Surprised that only one test was using the title watcher for arbitrary title.
> Less code is always good, so thanks a lot!
> 
> LGTM

Actually, I just added it in the original CL, so there is no surprise :)