Improve resiliency of PageLoadMetricsWaiter |
|||||
Issue descriptionOur current test infrastructure exposes timing update callbacks at a layer just above the IPC processing layer, in MetricsWebContentsObserver via the TestingObserver interface. This works fine when timing updates are dispatched synchronously to PageLoadMetricsObservers as part of the IPC processing flow. However, this strategy no longer works if updates are buffered before being dispatched. We may buffer updates in the browser process to allow for correct processing of out of order inter frame timing updates. This would require observing callbacks at a higher level than the MWCO timing update processing flow. The simplest way to address this is to allow the test infrastructure to inject a PageLoadMetricsObserver that can be used to wait for timing updates. This guarantees that our waiting logic is waiting on the same logic as other observers, which makes the tests more resilient. In addition, we can restrict waiting to a single page load - there have been some subtle browsertest bugs in the past where waiters waiting on a timing from one page load incorrectly stopped waiting too early because a previous page load dispatched a matching timing update. By observing at the PLMO level we have a page load level view of updates and can more precisely guarantee we are waiting on the right page load's updates.
,
May 29 2017
,
May 29 2017
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc commit c2ee5a4fd342deb46068e5eb31fc9023e799c4bc Author: bmcquade <bmcquade@chromium.org> Date: Tue May 30 01:24:19 2017 Make PageLoadMetricsWaiter more resilient Our current test infrastructure exposes timing update callbacks at a layer just above the IPC processing layer, in MetricsWebContentsObserver via the TestingObserver interface. This works fine when timing updates are dispatched synchronously to PageLoadMetricsObservers as part of the IPC processing flow. However, this strategy no longer works if updates are buffered before being dispatched. We may buffer updates in the browser process to allow for correct processing of out of order inter frame timing updates. This would require observing callbacks at a higher level than the MWCO timing update processing flow. The simplest way to address this is to allow the test infrastructure to inject a PageLoadMetricsObserver that can be used to wait for timing updates. This guarantees that our waiting logic is waiting on the same logic as other observers, which makes the tests more resilient. In addition, we can restrict waiting to a single page load - there have been some subtle browsertest bugs in the past where waiters waiting on a timing from one page load incorrectly stopped waiting too early because a previous page load dispatched a matching timing update. By observing at the PLMO level, and only starting waiting once a load commits, we have a page load level view of updates and can more precisely guarantee we are waiting on the right page load's updates. BUG= 725998 NOTRY=true NOPRESUBMIT=true TBR=csharrison Review-Url: https://codereview.chromium.org/2903693004 Cr-Original-Commit-Position: refs/heads/master@{#474996} Review-Url: https://codereview.chromium.org/2911983002 Cr-Commit-Position: refs/branch-heads/3112@{#21} Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897} [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/metrics_web_contents_observer.h [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/page_load_metrics_observer.h [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.h [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/page_load_tracker.cc [modify] https://crrev.com/c2ee5a4fd342deb46068e5eb31fc9023e799c4bc/chrome/browser/page_load_metrics/page_load_tracker.h
,
May 30 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, May 26 2017