New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 607332 link

Starred by 1 user
Status: Fixed
Owner:
█ bmcquade@chromium.org
Last visit > 30 days ago
Closed: Apr 2016
Cc:
csharrison@chromium.org
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

from gws logging fails for pages that open in new tabs

Project Member Reported by bmcquade@chromium.org, Apr 27 2016 
The opener logic we added works correctly, however, on Android at least, NavigationHandle::HasUserGesture() returns false for navigations that open in new tabs. Removing this check (while retaining the check that the transition type is link) works correctly for the new tab case.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 27 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc86df8278e4f3814e391c42fe9d34b3ef34f266

commit fc86df8278e4f3814e391c42fe9d34b3ef34f266
Author: bmcquade <bmcquade@chromium.org>
Date: Wed Apr 27 23:19:47 2016

Remove HasUserGesture check from from gws metric logger.

Navigations opened in new tabs return false for HasUserGesture,
so we need to remove that check in order to support that case.

BUG= 607332 

Review-Url: https://codereview.chromium.org/1915123007
Cr-Commit-Position: refs/heads/master@{#390228}

[modify] https://crrev.com/fc86df8278e4f3814e391c42fe9d34b3ef34f266/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc

Comment 2 by bmcquade@chromium.org, Apr 28 2016

Labels: Merge-Request-51 OS-All
Status: Fixed (was: Untriaged)

Comment 3 by tin...@google.com, Apr 28 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)

Comment 4 by gov...@chromium.org, Apr 29 2016 

Please merge your change to M51 branch 2704 before 5:00 PM PST, tomorrow (Friday), so we can take it in for next week M51 beta release. Thank you.

Comment 5 by csharrison@chromium.org, Apr 29 2016 

I'll merge this for you.

Comment 6 by csharrison@chromium.org, Apr 29 2016 

Tree is closed right now. Will try again when it's open.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 29 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca8e6aeed2b78d0a7699ad9e4b1589a53795aac1

commit ca8e6aeed2b78d0a7699ad9e4b1589a53795aac1
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Apr 29 14:08:57 2016

Remove HasUserGesture check from from gws metric logger.

Navigations opened in new tabs return false for HasUserGesture,
so we need to remove that check in order to support that case.

BUG= 607332 

Review-Url: https://codereview.chromium.org/1915123007
Cr-Commit-Position: refs/heads/master@{#390228}
(cherry picked from commit fc86df8278e4f3814e391c42fe9d34b3ef34f266)

TBR=bmcquade@chromium.org

Review URL: https://codereview.chromium.org/1930403002 .

Cr-Commit-Position: refs/branch-heads/2704@{#308}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/ca8e6aeed2b78d0a7699ad9e4b1589a53795aac1/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.cc
►

Sign in to add a comment