Break page load metrics test dependency on IPC.

Break page load metrics test dependency on IPC.

This change breaks direct dependencies on IPC for page load metrics
tests, in order to ease the transition to mojo.

Previously, unit tests exercised the IPC dispatch path. This
achieved slightly increased coverage but coupled unit tests to
IPC. We now dispatch simulated timing events directly to callbacks,
bypassing the IPC dispatch.

Additionally, we break the browsertest dependency on IPC and add
a MetricsWebContentsObserver::TestingObserver, which can observe
state changes at the observer level, instead of watching for IPC
messages. This both simplifies the logic and more directly verifies
expected behavior at the appropriate level.

BUG=715744
TBR=csharrison

Review-Url: https://codereview.chromium.org/2847513002
Cr-Commit-Position: refs/heads/master@{#467687}
Committed: https://chromium.googlesource.com/chromium/src/+/9567f9fa3fcfcaa50fc7b6a6adf66106a2eb6be5

[Bryan McQuade, 2017-04-26 20:23:18]

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

[commit-bot: I haz the power, 2017-04-26 20:24:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bryan McQuade, 2017-04-26 20:47:19]

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

[commit-bot: I haz the power, 2017-04-26 20:47:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bryan McQuade, 2017-04-26 20:51:11]

Description was changed from

==========
Break page load metrics test dependency on IPC.

This change breaks direct dependencies on IPC for page load metrics
tests, in order to ease the transition to mojo.

Previously, unit tests exercised the IPC dispatch path. This
achieved slightly increased coverage but coupled unit tests to
IPC. We now dispatch simulated timing events directly to callbacks,
bypassing the IPC dispatch.

Additionally, we break the browsertest dependency on IPC and add
a MetricsWebContentsObserver::TestingObserver, which can observe
state changes at the observer level, instead of watching for IPC
messages. This both simplifies the logic and more directly verifies
expected behavior at the appropriate level.

BUG=
==========

to

==========
Break page load metrics test dependency on IPC.

This change breaks direct dependencies on IPC for page load metrics
tests, in order to ease the transition to mojo.

Previously, unit tests exercised the IPC dispatch path. This
achieved slightly increased coverage but coupled unit tests to
IPC. We now dispatch simulated timing events directly to callbacks,
bypassing the IPC dispatch.

Additionally, we break the browsertest dependency on IPC and add
a MetricsWebContentsObserver::TestingObserver, which can observe
state changes at the observer level, instead of watching for IPC
messages. This both simplifies the logic and more directly verifies
expected behavior at the appropriate level.

BUG=715744
==========

[Bryan McQuade, 2017-04-26 20:54:09]

bmcquade@chromium.org changed reviewers:
+ csharrison@chromium.org, lpy@chromium.org

[Bryan McQuade, 2017-04-26 20:54:16]

PTAL, thanks!

[commit-bot: I haz the power, 2017-04-26 22:10:21]

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

[commit-bot: I haz the power, 2017-04-26 22:10:22]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2017-04-27 00:39:59]

FYI I may have some latency on this review because I'm going OOO for a few days.

[Bryan McQuade, 2017-04-27 00:49:55]

On 2017/04/27 at 00:39:59, csharrison wrote:
> FYI I may have some latency on this review because I'm going OOO for a few
days.

No problem, thanks!

[Bryan McQuade, 2017-04-27 00:50:28]

bmcquade@chromium.org changed reviewers:
+ jkarlin@chromium.org
- csharrison@chromium.org

[Bryan McQuade, 2017-04-27 00:50:41]

jkarlin, PTAL, thanks!

[lpy, 2017-04-27 06:31:52]

lgtm

[Bryan McQuade, 2017-04-27 15:28:11]

bmcquade@chromium.org changed reviewers:
- jkarlin@chromium.org

[Bryan McQuade, 2017-04-27 15:28:14]

The CQ bit was checked by bmcquade@chromium.org

[commit-bot: I haz the power, 2017-04-27 15:28:48]

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-04-27 15:28:48]

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

[commit-bot: I haz the power, 2017-04-27 15:28:49]

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[Bryan McQuade, 2017-04-27 15:29:42]

Description was changed from

==========
Break page load metrics test dependency on IPC.

This change breaks direct dependencies on IPC for page load metrics
tests, in order to ease the transition to mojo.

Previously, unit tests exercised the IPC dispatch path. This
achieved slightly increased coverage but coupled unit tests to
IPC. We now dispatch simulated timing events directly to callbacks,
bypassing the IPC dispatch.

Additionally, we break the browsertest dependency on IPC and add
a MetricsWebContentsObserver::TestingObserver, which can observe
state changes at the observer level, instead of watching for IPC
messages. This both simplifies the logic and more directly verifies
expected behavior at the appropriate level.

BUG=715744
==========

to

==========
Break page load metrics test dependency on IPC.

This change breaks direct dependencies on IPC for page load metrics
tests, in order to ease the transition to mojo.

Previously, unit tests exercised the IPC dispatch path. This
achieved slightly increased coverage but coupled unit tests to
IPC. We now dispatch simulated timing events directly to callbacks,
bypassing the IPC dispatch.

Additionally, we break the browsertest dependency on IPC and add
a MetricsWebContentsObserver::TestingObserver, which can observe
state changes at the observer level, instead of watching for IPC
messages. This both simplifies the logic and more directly verifies
expected behavior at the appropriate level.

BUG=715744
TBR=csharrison
==========

[Bryan McQuade, 2017-04-27 15:29:46]

The CQ bit was checked by bmcquade@chromium.org

[commit-bot: I haz the power, 2017-04-27 15:30:36]

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-04-27 15:35:53]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493306986596960,
"parent_rev": "b69ccf6f92a78f8dc097466d8132b7ccfd000950", "commit_rev":
"9567f9fa3fcfcaa50fc7b6a6adf66106a2eb6be5"}

[commit-bot: I haz the power, 2017-04-27 15:36:04]

Description was changed from

==========
Break page load metrics test dependency on IPC.

This change breaks direct dependencies on IPC for page load metrics
tests, in order to ease the transition to mojo.

Previously, unit tests exercised the IPC dispatch path. This
achieved slightly increased coverage but coupled unit tests to
IPC. We now dispatch simulated timing events directly to callbacks,
bypassing the IPC dispatch.

Additionally, we break the browsertest dependency on IPC and add
a MetricsWebContentsObserver::TestingObserver, which can observe
state changes at the observer level, instead of watching for IPC
messages. This both simplifies the logic and more directly verifies
expected behavior at the appropriate level.

BUG=715744
TBR=csharrison
==========

to

==========
Break page load metrics test dependency on IPC.

This change breaks direct dependencies on IPC for page load metrics
tests, in order to ease the transition to mojo.

Previously, unit tests exercised the IPC dispatch path. This
achieved slightly increased coverage but coupled unit tests to
IPC. We now dispatch simulated timing events directly to callbacks,
bypassing the IPC dispatch.

Additionally, we break the browsertest dependency on IPC and add
a MetricsWebContentsObserver::TestingObserver, which can observe
state changes at the observer level, instead of watching for IPC
messages. This both simplifies the logic and more directly verifies
expected behavior at the appropriate level.

BUG=715744
TBR=csharrison

Review-Url: https://codereview.chromium.org/2847513002
Cr-Commit-Position: refs/heads/master@{#467687}
Committed:
https://chromium.googlesource.com/chromium/src/+/9567f9fa3fcfcaa50fc7b6a6adf6...
==========

[commit-bot: I haz the power, 2017-04-27 15:36:05]

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

[zmin, 2017-04-27 15:43:20]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2847803002/ by zmin@chromium.org.

The reason for reverting is: PageLoadMetricsBrowserTest.DocumentWriteReloada is
flaky

https://luci-milo.appspot.com/buildbot/chromium.memory/Linux%20MSan%20Tests/387.

[Bryan McQuade, 2017-04-27 15:53:40]

On 2017/04/27 at 15:43:20, zmin wrote:
> A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2847803002/ by zmin@chromium.org.
> 
> The reason for reverting is: PageLoadMetricsBrowserTest.DocumentWriteReloada
is flaky
> 
>
https://luci-milo.appspot.com/buildbot/chromium.memory/Linux%20MSan%20Tests/387.

FYI, the flake cited here happened before this patch landed, so this patch is
not the cause of flakiness, and should be a step towards unflaking the test. So
I'm reverting the revert.

[Charlie Harrison, 2017-05-01 16:23:33]

csharrison@chromium.org changed reviewers:
+ csharrison@chromium.org

[Charlie Harrison, 2017-05-01 16:23:34]

belated lgtm/2

https://codereview.chromium.org/2847513002/diff/20001/chrome/browser/page_loa...
File chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc (right):

https://codereview.chromium.org/2847513002/diff/20001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc:21: #include
"chrome/common/page_load_metrics/page_load_metrics_messages.h"
still needed?