[PageLoadMetrics] Handle case where navigation aborts

[PageLoadMetrics] Reland handling of nav aborts in ads metrics

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled and the browsertest fixed to enable the feature (in case 
it's disabled by default).

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570
Review-Url: https://codereview.chromium.org/2883433002
Cr-Commit-Position: refs/heads/master@{#471446}
Committed: https://chromium.googlesource.com/chromium/src/+/98e70d364070da7465beea8205d99adc89279e66

[jkarlin, 2017-05-12 12:02:44]

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

[commit-bot: I haz the power, 2017-05-12 12:03:04]

Dry run: CQ is trying da patch.

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

[jkarlin, 2017-05-12 12:04:04]

Description was changed from

==========
[PargeLoadMetrics] Handle case where navigation aborts

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570
==========

to

==========
[PargeLoadMetrics] Reland handling of nav aborts in ads metrics

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled and the browsertest fixed to enable the feature.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570
==========

[jkarlin, 2017-05-12 12:05:49]

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

[jkarlin, 2017-05-12 12:05:49]

bmcquade@ PTAL, thanks!

[jkarlin, 2017-05-12 12:06:23]

Description was changed from

==========
[PargeLoadMetrics] Reland handling of nav aborts in ads metrics

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled and the browsertest fixed to enable the feature.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570
==========

to

==========
[PargeLoadMetrics] Reland handling of nav aborts in ads metrics

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled and the browsertest fixed to enable the feature (in case 
it's disabled by default).

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570
==========

[Bryan McQuade, 2017-05-12 12:43:59]

LGTM, thanks!

https://codereview.chromium.org/2883433002/diff/40001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2883433002/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc:39:
navigation_handle->GetWebContents()->UnsafeFindFrameByFrameTreeNodeId(
does this only return frames for committed loads? or will it return frames if a
provisional load is in flight as well? maybe add a comment to explain.

[commit-bot: I haz the power, 2017-05-12 14:21:06]

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

[commit-bot: I haz the power, 2017-05-12 14:21:07]

Dry run: This issue passed the CQ dry run.

[jkarlin, 2017-05-12 14:31:23]

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

[jkarlin, 2017-05-12 14:31:43]

Thanks, and PTAL for one more change.

https://codereview.chromium.org/2883433002/diff/40001/chrome/browser/page_loa...
File
chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc
(right):

https://codereview.chromium.org/2883433002/diff/40001/chrome/browser/page_loa...
chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc:39:
navigation_handle->GetWebContents()->UnsafeFindFrameByFrameTreeNodeId(
On 2017/05/12 12:43:59, Bryan McQuade wrote:
> does this only return frames for committed loads? or will it return frames if
a
> provisional load is in flight as well? maybe add a comment to explain.

Good question. It returns the committed RFH for the given frame id. But really
it's silly of me to check the previously committed frame for its frame name. So
instead, only checking frame name on committed navigations. PTAL.

[commit-bot: I haz the power, 2017-05-12 14:32:19]

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-05-12 14:55:54]

lgtm

[Charlie Harrison, 2017-05-12 14:57:02]

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

[Charlie Harrison, 2017-05-12 14:57:03]

nit: PargeLoadMetrics in title

[jkarlin, 2017-05-12 14:58:46]

Description was changed from

==========
[PargeLoadMetrics] Reland handling of nav aborts in ads metrics

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled and the browsertest fixed to enable the feature (in case 
it's disabled by default).

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570
==========

to

==========
[PageLoadMetrics] Reland handling of nav aborts in ads metrics

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled and the browsertest fixed to enable the feature (in case 
it's disabled by default).

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570
==========

[jkarlin, 2017-05-12 14:58:54]

On 2017/05/12 14:57:03, Charlie Harrison wrote:
> nit: PargeLoadMetrics in title

Thanks :)

[commit-bot: I haz the power, 2017-05-12 15:10:33]

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

[commit-bot: I haz the power, 2017-05-12 15:10:34]

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

[jkarlin, 2017-05-12 15:58:01]

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

[commit-bot: I haz the power, 2017-05-12 15:58:24]

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-05-12 17:01:19]

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

[commit-bot: I haz the power, 2017-05-12 17:01:20]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jkarlin, 2017-05-12 17:27:59]

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

[jkarlin, 2017-05-12 17:28:18]

I've gone back to PS3 (which was previously lgtm'd) with better comments.

[commit-bot: I haz the power, 2017-05-12 17:29:19]

Dry run: CQ is trying da patch.

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

[jkarlin, 2017-05-12 18:52:10]

The CQ bit was unchecked by jkarlin@chromium.org

[jkarlin, 2017-05-12 18:52:33]

On 2017/05/12 17:28:18, jkarlin wrote:
> I've gone back to PS3 (which was previously lgtm'd) with better comments.

After F2F approval I'm committing.

[jkarlin, 2017-05-12 18:52:37]

The CQ bit was checked by jkarlin@chromium.org

[jkarlin, 2017-05-12 18:52:37]

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/2883433002/#ps100001
(title: "Back to PS3")

[commit-bot: I haz the power, 2017-05-12 18:53: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-05-12 21:27:25]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1494615157659280,
"parent_rev": "3f98e54b610b6f3ddbf07cc610380a77e554c7c5", "commit_rev":
"98e70d364070da7465beea8205d99adc89279e66"}

[commit-bot: I haz the power, 2017-05-12 21:27:34]

Description was changed from

==========
[PageLoadMetrics] Reland handling of nav aborts in ads metrics

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled and the browsertest fixed to enable the feature (in case 
it's disabled by default).

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570
==========

to

==========
[PageLoadMetrics] Reland handling of nav aborts in ads metrics

This is a reland of https://codereview.chromium.org/2861433005 but with ads
metrics disabled and the browsertest fixed to enable the feature (in case 
it's disabled by default).

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

BUG=708570

Review-Url: https://codereview.chromium.org/2883433002
Cr-Commit-Position: refs/heads/master@{#471446}
Committed:
https://chromium.googlesource.com/chromium/src/+/98e70d364070da7465beea8205d9...
==========

[commit-bot: I haz the power, 2017-05-12 21:27:36]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/98e70d364070da7465beea8205d9...