Blink.UseCounter.SVGImage.Features is not tracking kPageVisits properly. |
||||
Issue descriptionWhat steps will reproduce the problem? (1) Perform a local build (2) Locally open chrome://histograms/Blink.UseCounter.SVGImage.Features (3) Locally open a page with an svg image (4) Refresh the page and the histogram What is the expected result? kPageVisits (52) will get updated along with other features (e.g. 138 SVGSVGElement). What happens instead? Other features get tracked, but kPageVisits remain unlogged. https://uma.googleplex.com/p/chrome/timeline_v2/?sid=adad825e111ae63132201c0628a12f06 https://chromium.googlesource.com/chromium/src/+log/9c5adea328f9744570e99520da2fa814af71df25/third_party/WebKit/Source/core/frame/UseCounter.cpp
,
Nov 21 2017
Changing in behaviour happened in between these two versions: 63.0.3223.8 <-> 63.0.3225.0 https://chromium.googlesource.com/chromium/src/+log/63.0.3223.0..63.0.3225.0?pretty=fuller&n=10000 Trying to bisect to find the causing CL
,
Nov 21 2017
Caused by this change: https://chromium.googlesource.com/chromium/src/+/6bb29391a86f2be58c626170156cbfaa2cbc5c91
,
Nov 21 2017
Seems like now the SVGImage's Page does not ever call Page::DidCommitLoad which then calls UseCounter::DidCommitLoad to log PageVisits usage. japhet@, do you have any insight how I might be able to correctly fix this (i.e. log PageVisits for every loading of an SVG image in a subframe)? Thanks
,
Nov 22 2017
,
Nov 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0bf269cd8533eb5ab4c595df46e09bdc5e36f3a1 commit 0bf269cd8533eb5ab4c595df46e09bdc5e36f3a1 Author: Luna Lu <loonybear@chromium.org> Date: Thu Nov 23 16:20:23 2017 Log Features Histogram PageVisists correctly for SVGImage After the change in https://chromium.googlesource.com/chromium/src/+/6bb29391a86f2be58c626170156cbfaa2cbc5c91 SVGImage pages no longer call Page::DidCommitLoad in path of loading, which dropped the PageVisists counts for Blink.UseCounter.SVGImage.Features histogram. Calling UseCounter::DidCommitLoad in SVGImage upon load complete fixed this issue. The fix has been locally verified. Bug: 787503 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I082ba0cffb021233dad40eadecd88694b9a21c95 Reviewed-on: https://chromium-review.googlesource.com/786330 Commit-Queue: Luna Lu <loonybear@chromium.org> Reviewed-by: Fredrik Söderquist <fs@opera.com> Cr-Commit-Position: refs/heads/master@{#518953} [modify] https://crrev.com/0bf269cd8533eb5ab4c595df46e09bdc5e36f3a1/third_party/WebKit/Source/core/frame/LocalFrame.cpp
,
Nov 23 2017
,
Jan 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/586fcf2127307c8f24c6a263548ecb4cb54055c4 commit 586fcf2127307c8f24c6a263548ecb4cb54055c4 Author: Luna Lu <loonybear@chromium.org> Date: Wed Jan 24 15:16:11 2018 Only track PageVisits for SVGIamge in LocalFrame::ForceSynchronousDocumentInstall After the change made in 6bb29391a86f2be58c626170156cbfaa2cbc5c91. SVGImage pages no longer tracks PageVisits count correctly. 0bf269cd8533eb5ab4c595df46e09bdc5e36f3a1 fixed that by adding a call to UseCounter::DidCommitLoad in LocalFrame::ForceSynchronousDocumentInstall. However this change also caused PageVisits to be tracked for unwanted pages like WebPagePopupImpl, InspectorOverlayAgent, and ValidationMessageOverlayDelegate. This change makes sure that PageVisits is only tracked for SVGImage. Bug: 787503 Change-Id: I98f4ba47d99ed821e6a0540887f65776c582bc8a Reviewed-on: https://chromium-review.googlesource.com/882202 Reviewed-by: Rick Byers <rbyers@chromium.org> Commit-Queue: Luna Lu <loonybear@chromium.org> Cr-Commit-Position: refs/heads/master@{#531531} [modify] https://crrev.com/586fcf2127307c8f24c6a263548ecb4cb54055c4/third_party/WebKit/Source/core/frame/LocalFrame.cpp |
||||
►
Sign in to add a comment |
||||
Comment 1 by loonyb...@chromium.org
, Nov 21 2017