[PageLoadMetrics] Remove legacy IPC from Page Load Metrics.

[PageLoadMetrics] Remove legacy IPC from Page Load Metrics.

Page Load Metrics Mojofication has proven that we can safely migrate to use mojo
IPC, this patch removes the legacy IPC and enables mojo IPC by default. Field
Trial results:
https://uma.googleplex.com/p/chrome/variations/?sid=a3c2a66c1ba11bac2961b7e38db75af3

BUG=709259, 712915
Review-Url: https://codereview.chromium.org/2948323002
Cr-Commit-Position: refs/heads/master@{#482480}
Committed: https://chromium.googlesource.com/chromium/src/+/1543140844c4f08b51022030ac7f5ce29a3b8756

[lpy, 2017-06-22 22:22:24]

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

[commit-bot: I haz the power, 2017-06-22 22:23:09]

Dry run: 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-06-22 22:28:09]

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

[commit-bot: I haz the power, 2017-06-22 22:28:11]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[lpy, 2017-06-22 23:06:01]

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

[lpy, 2017-06-22 23:06:16]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-06-22 23:06:27]

Dry run: 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-06-23 00:52:22]

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

[commit-bot: I haz the power, 2017-06-23 00:52:23]

Dry run: This issue passed the CQ dry run.

[lpy, 2017-06-23 05:26:11]

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

[lpy, 2017-06-23 05:26:11]

PTAL.

[Bryan McQuade, 2017-06-23 15:15:00]

On 2017/06/23 at 05:26:11, lpy wrote:
> PTAL.

Thanks! This is great to see. :) I'm going to double check that there isn't
anything else we can remove, but on first pass it looks like you got everything,
thanks!

[Bryan McQuade, 2017-06-23 15:15:07]

https://codereview.chromium.org/2948323002/diff/2/chrome/browser/page_load_me...
File chrome/browser/page_load_metrics/metrics_web_contents_observer.cc (right):

https://codereview.chromium.org/2948323002/diff/2/chrome/browser/page_load_me...
chrome/browser/page_load_metrics/metrics_web_contents_observer.cc:615:
observer.DidReceiveTimingUpdate();
let's remove this

https://codereview.chromium.org/2948323002/diff/2/chrome/browser/page_load_me...
File chrome/browser/page_load_metrics/metrics_web_contents_observer.h (right):

https://codereview.chromium.org/2948323002/diff/2/chrome/browser/page_load_me...
chrome/browser/page_load_metrics/metrics_web_contents_observer.h:58: virtual
void DidReceiveTimingUpdate() {}
this callback was actually only really for verifying we received updates for the
expected IPC type, so i think it's better to just remove this method altogether.

[lpy, 2017-06-23 17:57:09]

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

[commit-bot: I haz the power, 2017-06-23 17:57:27]

Dry run: 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-06-23 20:02:51]

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

[commit-bot: I haz the power, 2017-06-23 20:02:52]

Dry run: This issue passed the CQ dry run.

[lpy, 2017-06-23 21:23:47]

Thanks, I removed the method

https://codereview.chromium.org/2948323002/diff/2/chrome/browser/page_load_me...
File chrome/browser/page_load_metrics/metrics_web_contents_observer.cc (right):

https://codereview.chromium.org/2948323002/diff/2/chrome/browser/page_load_me...
chrome/browser/page_load_metrics/metrics_web_contents_observer.cc:615:
observer.DidReceiveTimingUpdate();
On 2017/06/23 15:15:07, Bryan McQuade wrote:
> let's remove this

Done.

https://codereview.chromium.org/2948323002/diff/2/chrome/browser/page_load_me...
File chrome/browser/page_load_metrics/metrics_web_contents_observer.h (right):

https://codereview.chromium.org/2948323002/diff/2/chrome/browser/page_load_me...
chrome/browser/page_load_metrics/metrics_web_contents_observer.h:58: virtual
void DidReceiveTimingUpdate() {}
On 2017/06/23 15:15:07, Bryan McQuade wrote:
> this callback was actually only really for verifying we received updates for
the
> expected IPC type, so i think it's better to just remove this method
altogether.

Done.

[Bryan McQuade, 2017-06-23 22:04:17]

LGTM, thanks for this!

[lpy, 2017-06-23 22:56:34]

The CQ bit was checked by lpy@chromium.org

[commit-bot: I haz the power, 2017-06-23 22:56:52]

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-06-23 23:03:26]

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

[commit-bot: I haz the power, 2017-06-23 23:03:27]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[lpy, 2017-06-23 23:03:52]

lpy@chromium.org changed reviewers:
+ dcheng@chromium.org

[lpy, 2017-06-23 23:03:52]

+decheng@ as security reviewer on removing some IPC files :)

decheng@, ptal.

[Bryan McQuade, 2017-06-23 23:41:30]

On 2017/06/23 at 23:03:52, lpy wrote:
> +decheng@ as security reviewer on removing some IPC files :)
> 
> decheng@, ptal.

actually, ah, you can update chrome/common/page_load_metrics/OWNERS to remove
the entries for the removed messages files, too. :)

[lpy, 2017-06-24 00:32:14]

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

[commit-bot: I haz the power, 2017-06-24 00:32:26]

Dry run: 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-06-24 01:58:46]

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

[commit-bot: I haz the power, 2017-06-24 01:58:47]

Dry run: This issue passed the CQ dry run.

[lpy, 2017-06-24 02:01:31]

The CQ bit was checked by lpy@chromium.org

[lpy, 2017-06-24 02:01:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmcquade@chromium.org
Link to the patchset: https://codereview.chromium.org/2948323002/#ps50001
(title: "Clean up OWNERS")

[commit-bot: I haz the power, 2017-06-24 02:01: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-06-24 02:13:33]

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

[commit-bot: I haz the power, 2017-06-24 02:13:34]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dcheng, 2017-06-25 07:44:46]

ipc lgtm with some comments that don't need to block the cl

(i do have a larger question about the status of oopif support of this stuff
though, but again that shouldn't block this cl)

https://codereview.chromium.org/2948323002/diff/50001/chrome/browser/page_loa...
File chrome/browser/page_load_metrics/metrics_web_contents_observer.cc (right):

https://codereview.chromium.org/2948323002/diff/50001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/metrics_web_contents_observer.cc:41:
content::RenderFrameHost* GetMainFrame(content::RenderFrameHost* rfh) {
Not introduced by this CL, but this doesn't really match the terminology we use
for OOPIFs--this would be considered a 'local root'.

Also, it seems like OnTimingUpdated drops the IPC if it doesn't match the main
frame of the WebContents: are there additional fixes required for OOPIF here?

https://codereview.chromium.org/2948323002/diff/50001/chrome/browser/page_loa...
File chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc (left):

https://codereview.chromium.org/2948323002/diff/50001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc:278: default:
For future reference, it's best to omit the default case if the switch handles
all known values--that way the compiler can warn if a new enum value is added.

https://codereview.chromium.org/2948323002/diff/50001/chrome/renderer/page_lo...
File chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc (right):

https://codereview.chromium.org/2948323002/diff/50001/chrome/renderer/page_lo...
chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc:194: return
base::WrapUnique<PageTimingSender>(
Nit: not introduced by this CL, but WrapUnique's template parameter should not
be specified (the whole point of WrapUnique is to auto-deduce the appropriate
type)

https://codereview.chromium.org/2948323002/diff/50001/chrome/renderer/page_lo...
chrome/renderer/page_load_metrics/metrics_render_frame_observer.cc:195: new
MojoPageTimingSender(render_frame()));
Nit: though we should just prefer base::MakeUnique, as the constructor is public
anyway.

[lpy, 2017-06-26 16:28:01]

The CQ bit was checked by lpy@chromium.org

[commit-bot: I haz the power, 2017-06-26 16:28:12]

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-06-26 16:35:12]

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

[commit-bot: I haz the power, 2017-06-26 16:35:15]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[lpy, 2017-06-26 16:36:47]

lpy@chromium.org changed reviewers:
+ sky@chromium.org

[lpy, 2017-06-26 16:36:48]

+sky@ as chrome/ owner, ptal.

[sky, 2017-06-26 20:48:46]

LGTM

[lpy, 2017-06-26 23:38:13]

The CQ bit was checked by lpy@chromium.org

[commit-bot: I haz the power, 2017-06-26 23:38:32]

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-06-27 00:10:44]

CQ is committing da patch.

Bot data: {"patchset_id": 50001, "attempt_start_ts": 1498520293769750,
"parent_rev": "b9ab2095d1f334506aa74681fa758adf03ab4a13", "commit_rev":
"1543140844c4f08b51022030ac7f5ce29a3b8756"}

[commit-bot: I haz the power, 2017-06-27 00:10:56]

Description was changed from

==========
[PageLoadMetrics] Remove legacy IPC from Page Load Metrics.

Page Load Metrics Mojofication has proven that we can safely migrate to use mojo
IPC, this patch removes the legacy IPC and enables mojo IPC by default. Field
Trial results:
https://uma.googleplex.com/p/chrome/variations/?sid=a3c2a66c1ba11bac2961b7e38...

BUG=709259,712915
==========

to

==========
[PageLoadMetrics] Remove legacy IPC from Page Load Metrics.

Page Load Metrics Mojofication has proven that we can safely migrate to use mojo
IPC, this patch removes the legacy IPC and enables mojo IPC by default. Field
Trial results:
https://uma.googleplex.com/p/chrome/variations/?sid=a3c2a66c1ba11bac2961b7e38...

BUG=709259,712915

Review-Url: https://codereview.chromium.org/2948323002
Cr-Commit-Position: refs/heads/master@{#482480}
Committed:
https://chromium.googlesource.com/chromium/src/+/1543140844c4f08b51022030ac7f...
==========

[commit-bot: I haz the power, 2017-06-27 00:10:58]

Committed patchset #3 (id:50001) as
https://chromium.googlesource.com/chromium/src/+/1543140844c4f08b51022030ac7f...