Excessive amount of Icing calls per navigation |
|||
Issue descriptionChrome Version: M61 OS: Android What steps will reproduce the problem? (1) Run adb logcat to stream logs and use local build of Clank (for Debug level adb log) (2) Open http://www.bbc.com/news/world-asia-40513334 What is the expected result? reportContext() should be called for a reasonable amount of times. Once per navigation would be ideal. Once for title change, and once for URL change is still reasonable. What happens instead? The following message showed up for >10 times: [GoogleApiIcingClientImpl.java:210] reportContext successful. Excessive reporting led to unnecessary calls to Icing. Reference: b/63133662
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b1ff788c214ab8c446f9a6df82f333872c6c0da7 commit b1ff788c214ab8c446f9a6df82f333872c6c0da7 Author: wychen <wychen@chromium.org> Date: Tue Jul 11 00:49:41 2017 Skip TabObserver#onUrlUpdated when not in main frame onUrlUpdated() should be called when the URL is updated. It doesn't make sense to call onUrlUpdated() at didFinishNavigation() of the iframes, since the main URL is not updated. onUrlUpdated() triggers GoogleApiIcingClientImpl#reportContext(), and excessive calls to onUrlUpdated() translates to excessive Icing calls. This CL lowers reporting per navigation to <5 roughly. In UMA, total count and Success bucket count of Search.IcingContextReportingStatus per navigation should decrease. BUG= 739570 Review-Url: https://codereview.chromium.org/2971953002 Cr-Commit-Position: refs/heads/master@{#485468} [modify] https://crrev.com/b1ff788c214ab8c446f9a6df82f333872c6c0da7/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/76293b107a339e6a93802c1c255ecbfdfceafcca commit 76293b107a339e6a93802c1c255ecbfdfceafcca Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org> Date: Tue Jul 11 07:01:24 2017 Dedup context reporting to Icing Context reporting records the URL, title, and selection to Icing. If the context is the same as last time, skip it to avoid useless API calls to Icing. This CL lowers the reporting per navigation to ~1. Bug: 739570 Change-Id: Id8d538c02f08e51ebdc729190a683856832b3a2e Reviewed-on: https://chromium-review.googlesource.com/562546 Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#485543} [modify] https://crrev.com/76293b107a339e6a93802c1c255ecbfdfceafcca/chrome/android/java/src/org/chromium/chrome/browser/gsa/ContextReporter.java [modify] https://crrev.com/76293b107a339e6a93802c1c255ecbfdfceafcca/tools/metrics/histograms/enums.xml
,
Jul 11 2017
,
Jul 14 2017
Context reporting ratio := Success count of Search.IcingContextReportingStatus / Total count of Navigation.IsMobileOptimized For Stable channel, the ratio is about 5.2. https://uma.googleplex.com/p/chrome/histograms/?endDate=20170710&dayCount=1&histograms=Navigation.IsMobileOptimized%2CSearch.IcingContextReportingStatus&fixupData=true&showMax=true&filters=platform%2Ceq%2CA%2Cchannel%2Ceq%2C4%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial For Canary channel with the fix, the ratio is about 1. https://uma.googleplex.com/p/chrome/histograms/?endDate=latest&dayCount=7&histograms=Navigation.IsMobileOptimized%2CSearch.IcingContextReportingStatus&fixupData=true&showMax=true&filters=platform%2Ceq%2CA%2Cchannel%2Ceq%2C1%2Call_version%2Ccnt%2C61.0.3155%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial We can do an apple-to-apple comparison once the fix is propagated to Stable.
,
Jul 18 2017
Verified on Canary 61.0.3155.0 |
|||
►
Sign in to add a comment |
|||
Comment 1 by wychen@chromium.org
, Jul 6 2017