PageLoadMetricsObserver assumptions don't always hold with child frame paint logic |
||||
Issue descriptionSome observers assume that by the time they observe a paint, we must have started parsing. This is a reasonable assumption, but due to timing races with cross frame paint updates, observers don't always see this assumption hold in their callbacks. For example, we can observe: * parse starts in main frame * child frame created * child frame paints * child frame reports paint event * parent frame reports parse start event Even though the events happened in the expected order, due to cross frame buffering, we are notified of the child paint before the main frame parse start. We should buffer timing callbacks until the assumptions in observers are met.
,
May 16 2017
Addressing this will fix the flake in https://bugs.chromium.org/p/chromium/issues/detail?id=722703
,
May 16 2017
,
May 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a886995ef55f7c165f50a27a309761c7432bc2c commit 8a886995ef55f7c165f50a27a309761c7432bc2c Author: bmcquade <bmcquade@chromium.org> Date: Tue May 16 20:09:42 2017 Buffer timing callbacks until timing state converges. With our recent change to merge paint timings across all frames in a page, some of the timing ordering assumptions we make, such as parse and layout always starting before paint, can be violated. More specifically, we can observe sequences like the following: * parse starts in main frame * child frame created * child frame paints * child frame reports paint event * parent frame reports parse start event Even though the events happened in the expected order, due to cross frame buffering, we are notified of the child paint before the main frame parse start. This change addresses this issue, by buffering observer callbacks until we've received events with the expected ordering. zhenw's recent change https://codereview.chromium.org/2872793002 encountered this issue and had to be rolled back as a result. I confirmed that, with zhen's patch applied, the flaky test failed on 8 of 10 runs. With zhen's patch and this patch applied, that same test passed 10 of 10 times. BUG= 722860 Review-Url: https://codereview.chromium.org/2885053002 Cr-Commit-Position: refs/heads/master@{#472199} [modify] https://crrev.com/8a886995ef55f7c165f50a27a309761c7432bc2c/chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc [modify] https://crrev.com/8a886995ef55f7c165f50a27a309761c7432bc2c/chrome/browser/page_load_metrics/page_load_tracker.cc [modify] https://crrev.com/8a886995ef55f7c165f50a27a309761c7432bc2c/chrome/browser/page_load_metrics/page_load_tracker.h [modify] https://crrev.com/8a886995ef55f7c165f50a27a309761c7432bc2c/tools/metrics/histograms/histograms.xml
,
May 25 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 Deleted