from gws logging incorrectly logs for intent-based navigations |
||||||
Issue descriptionOur newly updated gws logging logic seems to handle most cases correctly. However, it seems that certain URLs, such as those from wikipedia, use intent-based navigations. These confuse our fromgws logging logic, and cause us to log metrics in cases where we shouldn't. For example, if you search for 'test' and click on the wikipedia link, chrome performs the following navigations: provisional url: intent://en.m.wikipedia.org/wiki/Test#Intent;scheme=http;package=org.wik... this navigation doesn't appear to commit, but it appears to be from search results, so it passes the ShouldLogMetrics check. we seem to then get navigated to: https://www.google.com/searchurl/r.html#app=org.wikipedia&pingbase=https://www.g... our gws logging logic currently decides that this url is being navigated to from search, and logs it as a navigation. We may want to include https://www.google.com/searchurl/r.html as another search redirector URL. more investigation needed to understand if this is actually a redirector.
,
Apr 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f91fa02d7d146fb0dc4d17b25aef3128f644a439 commit f91fa02d7d146fb0dc4d17b25aef3128f644a439 Author: bmcquade <bmcquade@chromium.org> Date: Fri Apr 29 15:10:51 2016 Handle intent-based navigations from gws. We now explicitly ignore provisional navigations for non HTTP/HTTPS schemes. We also consider navigations from the intent redirector to be from search. BUG= 607330 Review-Url: https://codereview.chromium.org/1929673004 Cr-Commit-Position: refs/heads/master@{#390652} [modify] https://crrev.com/f91fa02d7d146fb0dc4d17b25aef3128f644a439/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc [modify] https://crrev.com/f91fa02d7d146fb0dc4d17b25aef3128f644a439/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h [modify] https://crrev.com/f91fa02d7d146fb0dc4d17b25aef3128f644a439/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc
,
Apr 29 2016
,
Apr 29 2016
,
Apr 30 2016
Your change meets the bar and is auto-approved for M51 (branch: 2704)
,
May 1 2016
Please merge your change to M51 branch 2704 before 5:00 PM PST, Monday (05/02/16), so we can take it in for next week M51 beta release. Thank you.
,
May 2 2016
M51 branch compilation is broken: ../../content/renderer/media/media_stream_audio_processor.cc:527:16: error: no member named 'RefinedAdaptiveFilter' in namespace 'webrtc'; did you mean 'switches::kAecRefinedAdaptiveFilter'? Should I go ahead and merge the changes, anyways?
,
May 2 2016
+ sshruthi@ (M51 Desktop TPM), could you please reply to comment #7.
,
May 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38bfe2b58578bad98d9731f820b13fe2b66da66f commit 38bfe2b58578bad98d9731f820b13fe2b66da66f Author: Charles Harrison <csharrison@chromium.org> Date: Mon May 02 21:20:53 2016 This CL fixes a bug in from_gws_page_load_metrics_observer.cc in WasAbortedInForeground. Earlier it did not consider cases where info.startedInForeground was false. BUG=608038 Review-Url: https://codereview.chromium.org/1935533003 Cr-Commit-Position: refs/heads/master@{#390776} (cherry picked from commit a3bed3b3193c41af5150b18706af412623c26a1a) Handle intent-based navigations from gws. We now explicitly ignore provisional navigations for non HTTP/HTTPS schemes. We also consider navigations from the intent redirector to be from search. BUG= 607330 TBR=bmcquade@chromium.org Review-Url: https://codereview.chromium.org/1929673004 Cr-Commit-Position: refs/heads/master@{#390652} (cherry picked from commit f91fa02d7d146fb0dc4d17b25aef3128f644a439) Review URL: https://codereview.chromium.org/1939983002 . Cr-Commit-Position: refs/branch-heads/2704@{#342} Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251} [modify] https://crrev.com/38bfe2b58578bad98d9731f820b13fe2b66da66f/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc [modify] https://crrev.com/38bfe2b58578bad98d9731f820b13fe2b66da66f/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h [modify] https://crrev.com/38bfe2b58578bad98d9731f820b13fe2b66da66f/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bmcquade@chromium.org
, Apr 27 2016Status: Started (was: Untriaged)