New issue
Advanced search Search tips

Issue 792524 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Nov 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on: View detail
issue 786035
issue 793053
issue 779914
issue 786173



Sign in to add a comment

Reduce CPU overhead before navigation start

Project Member Reported by csharrison@chromium.org, Dec 6 2017

Issue description

There are definitely some low hanging fruits here. Every win directly decreases top level metrics like TTFCP.

The overall metric for this is Navigation.TimeToURLJobStart.
 
Blockedon: 786035 779914 786173
Note well that this includes time spent on the IO thread and on the UI thread. We're starting to see good results from moving some simple SafeBrowsing tasks to a worker thread, to avoid blocking the IO thread.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18dc6fc987faf560b78ca0143c4e5565107e13f5

commit 18dc6fc987faf560b78ca0143c4e5565107e13f5
Author: Charles Harrison <csharrison@chromium.org>
Date: Wed Dec 06 23:31:45 2017

Add microsecond histogram for InterceptNavigationThrottle::WillStartRequest

There have been chrome traces which indicate this throttle is expensive
on Android. We should perform some measurements in the wild to see how
often this is true.

Bug:  792524 
Change-Id: I4b35949e0265c9e3b291d623789cbf4fd32d85f0
Reviewed-on: https://chromium-review.googlesource.com/811828
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522250}
[modify] https://crrev.com/18dc6fc987faf560b78ca0143c4e5565107e13f5/components/navigation_interception/intercept_navigation_throttle.cc
[modify] https://crrev.com/18dc6fc987faf560b78ca0143c4e5565107e13f5/tools/metrics/histograms/histograms.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2f161f9d2b78ca8bdfe7d82c301eefd16d1437d

commit c2f161f9d2b78ca8bdfe7d82c301eefd16d1437d
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Dec 07 03:38:13 2017

Add traces to PageLoadTracker INVOKE_AND_PRUNE_OBSERVERS calls.

Some observers can have code that computes policy in an expensive way.

Bug:  792524 
Change-Id: I4787f295e946efe889528a0a7df19421791856f4
Reviewed-on: https://chromium-review.googlesource.com/811367
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522327}
[modify] https://crrev.com/c2f161f9d2b78ca8bdfe7d82c301eefd16d1437d/chrome/browser/page_load_metrics/page_load_tracker.cc

Blockedon: 793053
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81f70917d50c88ac3893743fe2a9ccd6a61e3cfb

commit 81f70917d50c88ac3893743fe2a9ccd6a61e3cfb
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Dec 07 21:11:46 2017

Replace Nav.Intercept.WillStart histogram in favor of main/subframe breakouts

The initial data looks heavily multi-modal, which I suspect is from the
split.

Bug:  792524 
Change-Id: Icd40c722aa5505f4b3eb69c4283b9aca211c9dc4
Reviewed-on: https://chromium-review.googlesource.com/814216
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Tao Bai <michaelbai@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522544}
[modify] https://crrev.com/81f70917d50c88ac3893743fe2a9ccd6a61e3cfb/components/navigation_interception/intercept_navigation_throttle.cc
[modify] https://crrev.com/81f70917d50c88ac3893743fe2a9ccd6a61e3cfb/tools/metrics/histograms/histograms.xml

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f2bb5a7608bfb0e65b2a9ba01a8868419b9a49a

commit 6f2bb5a7608bfb0e65b2a9ba01a8868419b9a49a
Author: Charlie Harrison <csharrison@chromium.org>
Date: Mon Dec 11 15:01:02 2017

Revert "Replace Nav.Intercept.WillStart histogram in favor of main/subframe breakouts"

This reverts commit 81f70917d50c88ac3893743fe2a9ccd6a61e3cfb.

Reason for revert: NavigationThrottle can be destructed by JNI call.

Original change's description:
> Replace Nav.Intercept.WillStart histogram in favor of main/subframe breakouts
>
> The initial data looks heavily multi-modal, which I suspect is from the
> split.
>
> Bug:  792524 
> Change-Id: Icd40c722aa5505f4b3eb69c4283b9aca211c9dc4
> Reviewed-on: https://chromium-review.googlesource.com/814216
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: Tao Bai <michaelbai@chromium.org>
> Commit-Queue: Charlie Harrison <csharrison@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#522544}

TBR=michaelbai@chromium.org,asvitkine@chromium.org,csharrison@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  792524 , 793719
Change-Id: Ib6e4deab8bc20ba96428ea21c0b84acb97084606
Reviewed-on: https://chromium-review.googlesource.com/819330
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523095}
[modify] https://crrev.com/6f2bb5a7608bfb0e65b2a9ba01a8868419b9a49a/components/navigation_interception/intercept_navigation_throttle.cc
[modify] https://crrev.com/6f2bb5a7608bfb0e65b2a9ba01a8868419b9a49a/tools/metrics/histograms/histograms.xml

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24e730d2d5a98e1ec4fe01cf35f62f9d171e6503

commit 24e730d2d5a98e1ec4fe01cf35f62f9d171e6503
Author: Charles Harrison <csharrison@chromium.org>
Date: Tue Dec 12 17:56:58 2017

Predictor: Dispatch all predictions async from the resource throttle

This is a small refactor that should have some performance wins. Now
that PlzNavigate has launched, we should be dispatching our preconnects
after the initial navigation request has gone out, not before. This is
because frequently (especially on Android), dispatching preconnects
tends to hog the IO thread, delaying the initial request.

Note that this predictor is probably going away, but this CPU work
is in the critical path of some other work I am planning on tackling.

Bug:  792524 
Change-Id: I87e3c235be0480d766e3e45c9a97c4a26b15f60d
Reviewed-on: https://chromium-review.googlesource.com/821090
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523473}
[modify] https://crrev.com/24e730d2d5a98e1ec4fe01cf35f62f9d171e6503/chrome/browser/loader/predictor_resource_throttle.cc
[modify] https://crrev.com/24e730d2d5a98e1ec4fe01cf35f62f9d171e6503/chrome/browser/loader/predictor_resource_throttle.h
[modify] https://crrev.com/24e730d2d5a98e1ec4fe01cf35f62f9d171e6503/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/24e730d2d5a98e1ec4fe01cf35f62f9d171e6503/chrome/browser/net/predictor_tab_helper.cc
[modify] https://crrev.com/24e730d2d5a98e1ec4fe01cf35f62f9d171e6503/chrome/browser/net/predictor_tab_helper.h

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/35ae870825a7d33d78d49653cb35ccfbca77a04e

commit 35ae870825a7d33d78d49653cb35ccfbca77a04e
Author: Charlie Harrison <csharrison@chromium.org>
Date: Tue Dec 12 18:40:48 2017

Revert "Predictor: Dispatch all predictions async from the resource throttle"

This reverts commit 24e730d2d5a98e1ec4fe01cf35f62f9d171e6503.

Reason for revert: The delay navigation throttle will ruin data here.
Let me re-land tomorrow.

Original change's description:
> Predictor: Dispatch all predictions async from the resource throttle
> 
> This is a small refactor that should have some performance wins. Now
> that PlzNavigate has launched, we should be dispatching our preconnects
> after the initial navigation request has gone out, not before. This is
> because frequently (especially on Android), dispatching preconnects
> tends to hog the IO thread, delaying the initial request.
> 
> Note that this predictor is probably going away, but this CPU work
> is in the critical path of some other work I am planning on tackling.
> 
> Bug:  792524 
> Change-Id: I87e3c235be0480d766e3e45c9a97c4a26b15f60d
> Reviewed-on: https://chromium-review.googlesource.com/821090
> Commit-Queue: Charlie Harrison <csharrison@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#523473}

TBR=mmenke@chromium.org,csharrison@chromium.org

Change-Id: I596b40d4d3f527de7e0df6c00f376281a2dbe381
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  792524 
Reviewed-on: https://chromium-review.googlesource.com/823110
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523486}
[modify] https://crrev.com/35ae870825a7d33d78d49653cb35ccfbca77a04e/chrome/browser/loader/predictor_resource_throttle.cc
[modify] https://crrev.com/35ae870825a7d33d78d49653cb35ccfbca77a04e/chrome/browser/loader/predictor_resource_throttle.h
[modify] https://crrev.com/35ae870825a7d33d78d49653cb35ccfbca77a04e/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/35ae870825a7d33d78d49653cb35ccfbca77a04e/chrome/browser/net/predictor_tab_helper.cc
[modify] https://crrev.com/35ae870825a7d33d78d49653cb35ccfbca77a04e/chrome/browser/net/predictor_tab_helper.h

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ded5a15654abc9dc42bb155663345d1efc0cae41

commit ded5a15654abc9dc42bb155663345d1efc0cae41
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Dec 15 03:36:35 2017

Reland: Predictor: Dispatch all predictions async from the resource throttle

This patch relands the following CL:
https://chromium-review.googlesource.com/c/chromium/src/+/821090

This is a small refactor that should have some performance wins. Now
that PlzNavigate has launched, we should be dispatching our preconnects
after the initial navigation request has gone out, not before. This is
because frequently (especially on Android), dispatching preconnects
tends to hog the IO thread, delaying the initial request.

Note that this predictor is probably going away, but this CPU work
is in the critical path of some other work I am planning on tackling.

Bug:  792524 
Change-Id: I03b058af249088f779dfb574c8552b32150bea2c
Reviewed-on: https://chromium-review.googlesource.com/826783
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524308}
[modify] https://crrev.com/ded5a15654abc9dc42bb155663345d1efc0cae41/chrome/browser/loader/predictor_resource_throttle.cc
[modify] https://crrev.com/ded5a15654abc9dc42bb155663345d1efc0cae41/chrome/browser/loader/predictor_resource_throttle.h
[modify] https://crrev.com/ded5a15654abc9dc42bb155663345d1efc0cae41/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/ded5a15654abc9dc42bb155663345d1efc0cae41/chrome/browser/net/predictor_tab_helper.cc
[modify] https://crrev.com/ded5a15654abc9dc42bb155663345d1efc0cae41/chrome/browser/net/predictor_tab_helper.h

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53096ba1c32c36f6ee86ebf95fbd6e7d16e1e587

commit 53096ba1c32c36f6ee86ebf95fbd6e7d16e1e587
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Dec 15 15:39:48 2017

Add a few more navigation trace events

Bug:  792524 
Change-Id: I3c05b7c3f4c979ff795aa41cd97393e8b562df3d
Reviewed-on: https://chromium-review.googlesource.com/820790
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524388}
[modify] https://crrev.com/53096ba1c32c36f6ee86ebf95fbd6e7d16e1e587/content/browser/web_contents/web_contents_impl.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73722a14e265b530d351ac9210e4b67d7b89ea37

commit 73722a14e265b530d351ac9210e4b67d7b89ea37
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Dec 15 19:12:46 2017

Remove navigation start observing from DataUseAscriber

There's no reason to PostTask a bunch of data to the IO thread to do a
no-op method.

This CL should change no behavior.

Bug:  792524 
Change-Id: Ia0f88da99c72fa3369b164a2fd25352f77d13544
Reviewed-on: https://chromium-review.googlesource.com/827980
Reviewed-by: rajendrant <rajendrant@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524424}
[modify] https://crrev.com/73722a14e265b530d351ac9210e4b67d7b89ea37/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
[modify] https://crrev.com/73722a14e265b530d351ac9210e4b67d7b89ea37/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
[modify] https://crrev.com/73722a14e265b530d351ac9210e4b67d7b89ea37/chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.cc
[modify] https://crrev.com/73722a14e265b530d351ac9210e4b67d7b89ea37/chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h
[modify] https://crrev.com/73722a14e265b530d351ac9210e4b67d7b89ea37/chrome/browser/data_use_measurement/chrome_data_use_ascriber_unittest.cc
[modify] https://crrev.com/73722a14e265b530d351ac9210e4b67d7b89ea37/chrome/browser/data_use_measurement/data_use_web_contents_observer.cc
[modify] https://crrev.com/73722a14e265b530d351ac9210e4b67d7b89ea37/chrome/browser/data_use_measurement/data_use_web_contents_observer.h

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/07631d1297280eef99749ac0c50c98fdb18cc2e5

commit 07631d1297280eef99749ac0c50c98fdb18cc2e5
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Fri Dec 15 19:33:33 2017

Revert "Remove navigation start observing from DataUseAscriber"

This reverts commit 73722a14e265b530d351ac9210e4b67d7b89ea37.

Reason for revert: Breaking the Linux x64 build
https://ci.chromium.org/buildbot/chromium/Linux%20x64/55031

Original change's description:
> Remove navigation start observing from DataUseAscriber
> 
> There's no reason to PostTask a bunch of data to the IO thread to do a
> no-op method.
> 
> This CL should change no behavior.
> 
> Bug:  792524 
> Change-Id: Ia0f88da99c72fa3369b164a2fd25352f77d13544
> Reviewed-on: https://chromium-review.googlesource.com/827980
> Reviewed-by: rajendrant <rajendrant@chromium.org>
> Commit-Queue: rajendrant <rajendrant@chromium.org>
> Commit-Queue: Charlie Harrison <csharrison@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524424}

TBR=csharrison@chromium.org,rajendrant@chromium.org

Change-Id: I00a16f8e2ee77b767f4289ad01ca4a05c711fdc9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  792524 
Reviewed-on: https://chromium-review.googlesource.com/830473
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524431}
[modify] https://crrev.com/07631d1297280eef99749ac0c50c98fdb18cc2e5/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
[modify] https://crrev.com/07631d1297280eef99749ac0c50c98fdb18cc2e5/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
[modify] https://crrev.com/07631d1297280eef99749ac0c50c98fdb18cc2e5/chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.cc
[modify] https://crrev.com/07631d1297280eef99749ac0c50c98fdb18cc2e5/chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h
[modify] https://crrev.com/07631d1297280eef99749ac0c50c98fdb18cc2e5/chrome/browser/data_use_measurement/chrome_data_use_ascriber_unittest.cc
[modify] https://crrev.com/07631d1297280eef99749ac0c50c98fdb18cc2e5/chrome/browser/data_use_measurement/data_use_web_contents_observer.cc
[modify] https://crrev.com/07631d1297280eef99749ac0c50c98fdb18cc2e5/chrome/browser/data_use_measurement/data_use_web_contents_observer.h

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f18e14e81546a0676f5852f58f5d7b229741ea2

commit 1f18e14e81546a0676f5852f58f5d7b229741ea2
Author: Charles Harrison <csharrison@chromium.org>
Date: Mon Dec 18 20:21:22 2017

Reland: Remove navigation start observing from DataUseAscriber

This relands https://chromium-review.googlesource.com/827980,
which ended up getting through the CQ despite compilation errors, due
to another CL which used one of the removed methods landing after the
CQ tested the patch.

Bug:  792524 
Change-Id: Id8f006dd95626a4e0c72e3ee235060d58798293f
Reviewed-on: https://chromium-review.googlesource.com/830787
Reviewed-by: rajendrant <rajendrant@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524784}
[modify] https://crrev.com/1f18e14e81546a0676f5852f58f5d7b229741ea2/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
[modify] https://crrev.com/1f18e14e81546a0676f5852f58f5d7b229741ea2/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
[modify] https://crrev.com/1f18e14e81546a0676f5852f58f5d7b229741ea2/chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.cc
[modify] https://crrev.com/1f18e14e81546a0676f5852f58f5d7b229741ea2/chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h
[modify] https://crrev.com/1f18e14e81546a0676f5852f58f5d7b229741ea2/chrome/browser/data_use_measurement/chrome_data_use_ascriber_unittest.cc
[modify] https://crrev.com/1f18e14e81546a0676f5852f58f5d7b229741ea2/chrome/browser/data_use_measurement/data_use_web_contents_observer.cc
[modify] https://crrev.com/1f18e14e81546a0676f5852f58f5d7b229741ea2/chrome/browser/data_use_measurement/data_use_web_contents_observer.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6

commit 3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Jan 12 23:20:44 2018

Remove a bunch of prerender lookups in PLM and metrics code

This patch should change no behavior.

Looking up the prerender contents / manager is not free, This patch
removes four instances in PLM navigation code, and one on every resource
request completion.

Bug:  792524 
Change-Id: Ie1dbcd07eedcc82c9e210b0fc378cfd4072b208f
Reviewed-on: https://chromium-review.googlesource.com/822830
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529105}
[modify] https://crrev.com/3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.cc
[modify] https://crrev.com/3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h
[modify] https://crrev.com/3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc
[modify] https://crrev.com/3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h
[modify] https://crrev.com/3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
[modify] https://crrev.com/3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6/chrome/browser/page_load_metrics/page_load_tracker.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/efed514b1d911aa33b4f2b9b9abf3b11a92a74ad

commit efed514b1d911aa33b4f2b9b9abf3b11a92a74ad
Author: Charlie Harrison <csharrison@chromium.org>
Date: Mon Jan 15 19:36:57 2018

Revert "Remove a bunch of prerender lookups in PLM and metrics code"

This reverts commit 3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6.

Reason for revert: Causing crbug.com/801981

Original change's description:
> Remove a bunch of prerender lookups in PLM and metrics code
>
> This patch should change no behavior.
>
> Looking up the prerender contents / manager is not free, This patch
> removes four instances in PLM navigation code, and one on every resource
> request completion.
>
> Bug:  792524 
> Change-Id: Ie1dbcd07eedcc82c9e210b0fc378cfd4072b208f
> Reviewed-on: https://chromium-review.googlesource.com/822830
> Reviewed-by: David Roger <droger@chromium.org>
> Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
> Commit-Queue: Charlie Harrison <csharrison@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#529105}

TBR=droger@chromium.org,bmcquade@chromium.org,csharrison@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  792524 ,801981
Change-Id: Ia3e9f2ddcf574c25fbec911ed561790d98c73d08
Reviewed-on: https://chromium-review.googlesource.com/867191
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529321}
[modify] https://crrev.com/efed514b1d911aa33b4f2b9b9abf3b11a92a74ad/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/efed514b1d911aa33b4f2b9b9abf3b11a92a74ad/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.cc
[modify] https://crrev.com/efed514b1d911aa33b4f2b9b9abf3b11a92a74ad/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h
[modify] https://crrev.com/efed514b1d911aa33b4f2b9b9abf3b11a92a74ad/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc
[modify] https://crrev.com/efed514b1d911aa33b4f2b9b9abf3b11a92a74ad/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h
[modify] https://crrev.com/efed514b1d911aa33b4f2b9b9abf3b11a92a74ad/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
[modify] https://crrev.com/efed514b1d911aa33b4f2b9b9abf3b11a92a74ad/chrome/browser/page_load_metrics/page_load_tracker.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 15 2018

Labels: merge-merged-3322
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1dea820cbff7dd90ac0f49ae679a84280e45d764

commit 1dea820cbff7dd90ac0f49ae679a84280e45d764
Author: Charlie Harrison <csharrison@chromium.org>
Date: Mon Jan 15 19:38:31 2018

Revert "Remove a bunch of prerender lookups in PLM and metrics code"

This reverts commit 3f265ba8de11a81a4bc151b1908c4eddf6b7c9c6.

Reason for revert: Causing crbug.com/801981

Original change's description:
> Remove a bunch of prerender lookups in PLM and metrics code
>
> This patch should change no behavior.
>
> Looking up the prerender contents / manager is not free, This patch
> removes four instances in PLM navigation code, and one on every resource
> request completion.
>
> Bug:  792524 
> Change-Id: Ie1dbcd07eedcc82c9e210b0fc378cfd4072b208f
> Reviewed-on: https://chromium-review.googlesource.com/822830
> Reviewed-by: David Roger <droger@chromium.org>
> Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
> Commit-Queue: Charlie Harrison <csharrison@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#529105}

TBR=droger@chromium.org,bmcquade@chromium.org,csharrison@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  792524 ,801981
Change-Id: Ia3e9f2ddcf574c25fbec911ed561790d98c73d08
Reviewed-on: https://chromium-review.googlesource.com/867191
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#529321}(cherry picked from commit efed514b1d911aa33b4f2b9b9abf3b11a92a74ad)
Reviewed-on: https://chromium-review.googlesource.com/867530
Cr-Commit-Position: refs/branch-heads/3322@{#4}
Cr-Branched-From: 4522efba74e35026b0f62cb1c97d793a92a0d5da-refs/heads/master@{#529187}
[modify] https://crrev.com/1dea820cbff7dd90ac0f49ae679a84280e45d764/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/1dea820cbff7dd90ac0f49ae679a84280e45d764/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.cc
[modify] https://crrev.com/1dea820cbff7dd90ac0f49ae679a84280e45d764/chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h
[modify] https://crrev.com/1dea820cbff7dd90ac0f49ae679a84280e45d764/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc
[modify] https://crrev.com/1dea820cbff7dd90ac0f49ae679a84280e45d764/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h
[modify] https://crrev.com/1dea820cbff7dd90ac0f49ae679a84280e45d764/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
[modify] https://crrev.com/1dea820cbff7dd90ac0f49ae679a84280e45d764/chrome/browser/page_load_metrics/page_load_tracker.cc

Status: WontFix (was: Untriaged)
This regression has been open for half a year. It's not very actionable and the regression has been in all Chrome user's hands for months.

Sign in to add a comment