New issue
Advanced search Search tips
Starred by 20 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

detect and block tab-unders

Project Member Reported by jochen@chromium.org, Nov 2 2016

Issue description

since pop-unders are more and more difficult, some sites resort to tab-unders: you open a new tab (requires user gesture) and then navigate the old tab to a different URL (doesn't require a user gesture).

Note that you can trigger the navigation change before opening the new tab.

I wonder how common this pattern is when the user really wants this to happen. As a first step, I'd like to measure how long a tab that is in the background when it navigates to another site (cross origin) is visible before it is closed.

My assumption is that users will close those tabs immediately after switching to them, or without even switching to them
 

Comment 1 by ojan@chromium.org, Mar 28 2017

My thought on how to fix this is to make it so that opening a new tab puts the current tab in a mode where it isn't allowed to navigate until it gets another user gesture. How does that sound?

BTW, jochen, I assume you're not actually planning on working on this still. Should I try to find a different owner?

Comment 2 by jochen@chromium.org, Mar 28 2017

I have a wip CL that's supposed to add UMA. I still think that's the first step. If you have somebody willing to spend more time on this, I'd gladly hand over. Otherwise, I'll plan to do this eventually :)
Cc: benwells@chromium.org
Cc: csharrison@chromium.org
jochen and I chatted about this offline. He proposed we just close the original tab and show a blocked popup UI on the new window with the redirect URL.

I thought maybe we could reuse the UI for top-level redirects, but I'm starting to like the idea of just closing the opener due to memory saving wins, etc.

Comment 5 by jochen@chromium.org, Sep 14 2017

Cc: jochen@chromium.org timloh@chromium.org
Owner: csharrison@chromium.org
It looks like people are happy with Jochen's idea to close the opener tab, and show the blocked popup dialog on the opened popup. I think this can work but I have a few concerns:

1. How will the content setting exception work? If we show the content setting UI on the popup, then we will need to wait for the popup to finish navigating before we can close the opener (to know which site the setting should be using). It is unfortunate to do this waiting but I think it's OK.

If we don't use an exception the UI will need to be changed.

2. On Android, the popup blocker UI does not show the URL that was blocked. We are considering changing this but for the time being it just has an option to "always allow popups on this site".

I feel we probably need to update the UI to ship this on Android.

Comment 7 by jochen@chromium.org, Sep 20 2017

If I was developing such sites, I'd delay the cross origin redirect until the tab gets focused again. So if we anticipate this, we'd end up just closing the tab once it opens a new foreground tab.

I'd measure how long such a background tab is seen by users, and assuming that it's almost always either closed before even getting activated, or within seconds of being activated, we should just close them right away
I'm OK closing right away, in fact it makes everything more simple. However, it does make the popup blocking UI inconsistent, since we won't have a meaningful whitelisting policy.
Cc: ojan@chromium.org
I chatted with ojan about this at BlinkOn, who thought that using the 3p redirect intervention would be an easier change to make than closing the tab. Here is what we came up with:

1. Make main frame navigations consume a user gesture. This constrains tab-unders so that they must open a popup before navigating, making them easily tagged.

2. Cross origin main frame navigations without a user gesture, coming from pages which have opened a popup, are blocked with the 3p redirect intervention (e.g. a mini infobar with the URL that was blocked). Since popups consume a user gesture, this means the user needs to interact with the page again before it can navigate away.
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 29 2017

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

commit 1305571e932237aa72fd9be2321081e65d9ac92a
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Sep 29 01:39:18 2017

Popup metrics: Log engagement time related to tab-unders

This patch logs the time a particular WebContents spends in the
foreground, after a navigation which:
 - Is started in the background
 - Commits to a cross-origin site
 - Has no user gesture
 - Is not the first navigation in the WebContents

This metric will serve as a basis for determining how much user
benefit comes from the (imo) harmful practice of navigating a site
without user action or direct consent.

In the near future, this metric will be broken up to also include
breakouts for if the WebContents opened a popup before performing
this kind of navigation.

Bug: 661629
Change-Id: Ib6b59792b193bd447fd59939e06ed94d25c1be6b
Reviewed-on: https://chromium-review.googlesource.com/685559
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505248}
[modify] https://crrev.com/1305571e932237aa72fd9be2321081e65d9ac92a/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/1305571e932237aa72fd9be2321081e65d9ac92a/chrome/browser/ui/blocked_content/popup_opener_tab_helper.cc
[add] https://crrev.com/1305571e932237aa72fd9be2321081e65d9ac92a/chrome/browser/ui/blocked_content/popup_opener_tab_helper.h
[add] https://crrev.com/1305571e932237aa72fd9be2321081e65d9ac92a/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/1305571e932237aa72fd9be2321081e65d9ac92a/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/1305571e932237aa72fd9be2321081e65d9ac92a/chrome/test/BUILD.gn
[modify] https://crrev.com/1305571e932237aa72fd9be2321081e65d9ac92a/tools/metrics/histograms/histograms.xml

I wrote my thoughts on blocking tab-unders in a document [1] (for now, only open to chromium.org accounts)

[1]: https://docs.google.com/a/chromium.org/document/d/1ewmlwRVPyHUa-o63mubIq5qhdbk_kfqSAhdxmJMKvGc/edit?usp=sharing
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 2 2017

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

commit 9fc031ebdeade7ee64e065f78ec823f4983bfc43
Author: Charles Harrison <csharrison@chromium.org>
Date: Mon Oct 02 12:05:30 2017

Popup metrics: add popup opener specific metrics

This patch adds two new metrics.
1. A breakout metric of our "VisibleTimeAfterCrossOriginRedirect" metric,
   which measures the visible time for tabs that also have opened a popup
   before doing the redirect.
2. The amount of time between the popup -> redirect.

These metrics will help us evaluate how valuable tab-unders are to users.

This patch also adds some infrastructure for noting opener <-> popup
relationships between the popup_opener_tab_helper and the popup_tracker.
This information is not used all that much in this patch, but will be
critical in future patches if we want to intervene on tab-unders.

This added infrastructure caught a bug where we were considering
WebContents that had no opener to be a popup. This is amended and tested
in this patch, and the PopupTracker metric is versioned.

Bug: 661629
Change-Id: I4109333d3f689a03de3b4aaa6f5310c8a861fb09
Reviewed-on: https://chromium-review.googlesource.com/690297
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505585}
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/android/tab_web_contents_delegate_android.cc
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/ui/blocked_content/popup_opener_tab_helper.cc
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/ui/blocked_content/popup_opener_tab_helper.h
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/ui/blocked_content/popup_tracker.cc
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/ui/blocked_content/popup_tracker.h
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/ui/blocked_content/popup_tracker_browsertest.cc
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/chrome/browser/ui/browser.cc
[modify] https://crrev.com/9fc031ebdeade7ee64e065f78ec823f4983bfc43/tools/metrics/histograms/histograms.xml

Components: UI>Browser>Navigation UI>Browser>PopupBlocker
Status: Started (was: Assigned)
Cc: a...@chromium.org
Thanks creis, forgot avi wasn't ccd here (he is actually the reviewer of this work).

Also I just posted an i2i on blink-dev:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/9xBeP0UEpko
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 3 2017

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

commit ba936162ced9bb269bbf61f3e13ac6a194d314dc
Author: Charles Harrison <csharrison@chromium.org>
Date: Tue Oct 03 00:36:37 2017

Popup metrics: track visible time for all tabs

This will help as a reference point for other visibility metrics at the
tab layer (namely popups and tab-unders).

This CL contains a minor refactor, so that the ScopedVisibilityTrackers
no longer own their tick clocks.

Bug: 661629
Change-Id: I879fe751661876442b609bad9008d16af52ba2f1
Reviewed-on: https://chromium-review.googlesource.com/694741
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505886}
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/blocked_content/popup_opener_tab_helper.cc
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/blocked_content/popup_opener_tab_helper.h
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/blocked_content/popup_tracker.cc
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/blocked_content/popup_tracker.h
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/blocked_content/scoped_visibility_tracker.cc
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/blocked_content/scoped_visibility_tracker.h
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/blocked_content/scoped_visibility_tracker_unittest.cc
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/ba936162ced9bb269bbf61f3e13ac6a194d314dc/tools/metrics/histograms/histograms.xml

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 3 2017

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

commit 6dd11d28feba8b8003ef16f82b59cc3af4c6136e
Author: Charles Harrison <csharrison@chromium.org>
Date: Tue Oct 03 06:14:51 2017

Block tab-unders using the framebusting UI.

The CL adds a NavigationThrottle which cancels navigations if it
detects that they are tab-unders. This CL exposes this
functionality behind the command line flag
--enable-features="BlockTabUnders". A followup CL will add an 
entry to chrome://flags for additional user testing.

A tab-under is classified as a navigation in a tab which:
1. Has opened a popup since the last user gesture in that tab. And
2. Is a "suspicious" client redirect, which:
 a. Is renderer initiated
 b. Has no user gesture
 c. Was started when the tab was in the background
 d. Is cross origin to the last committed URL in the tab.

If a navigation is blocked, the WebContents delegate is told about it
so that the "framebusting" (aka 3p redirect intervention) UI is shown.
This allows a user to allow the navigation by clicking the link on some
native UI.

NOTE: This UI is currently only implemented on Android, so the
experiment should not be enabled for real users on desktop platforms.

In the future we may want to be even stricter here. Instead of showing
some native UI we could just close the opener tab entirely (see
discussion on the linked bug). However, this approach is a good first
step in that direction and has a better chance of breaking less web
content.

Additionally, this CL refactors some of the popup opener metrics to
share logic with the throttle.

Bug: 661629
Change-Id: Ifa46979016a7dee725f836948fbed6e1641e0bbc
Reviewed-on: https://chromium-review.googlesource.com/693255
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505964}
[modify] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/browser/ui/blocked_content/popup_opener_tab_helper.cc
[modify] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/browser/ui/blocked_content/popup_opener_tab_helper.h
[modify] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[add] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/browser/ui/blocked_content/tab_under_blocker_browsertest.cc
[add] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[add] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h
[modify] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/chrome/test/BUILD.gn
[modify] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/6dd11d28feba8b8003ef16f82b59cc3af4c6136e/tools/metrics/histograms/histograms.xml

Project Member

Comment 18 by bugdroid1@chromium.org, Oct 5 2017

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

commit 537343fe61b13e41227fe6968bb150132afffd1f
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Oct 05 01:13:14 2017

Popup metrics: simplify tab-under metrics

This CL does a few things.
1. Instantiate the tab-under nav throttle unconditionally, only
   *block* the navigation if the feature is enabled. This allows us
   to combine logic more effectively for metrics, by e.g. removing
   the flat set of navigations occurring in the background. For this
   data we can just trust the nav throttle.

2. Remove the old tab visibility metrics gated on cross origin redirects
   in favor of an approach that just logs 3 metrics:
    a. Tab.VisibleTime
    b. Tab.TabUnder.VisibleTime
    c. Tab.TabUnder.PopupToTabUnderTime
   I believe these three metrics are the most important, and they are
   now logged directly from input from the throttle, so they are
   comparable what will actually happen when we turn the experiment on.

3. A bug fix where we assume a navigation that started in the background,
   ends in the background.

4. Some new tests.

Bug: 661629
Change-Id: Idc1b82528cbf2bf2a1a94d174b4e07823ec30d17
Reviewed-on: https://chromium-review.googlesource.com/700718
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506601}
[modify] https://crrev.com/537343fe61b13e41227fe6968bb150132afffd1f/chrome/browser/ui/blocked_content/popup_opener_tab_helper.cc
[modify] https://crrev.com/537343fe61b13e41227fe6968bb150132afffd1f/chrome/browser/ui/blocked_content/popup_opener_tab_helper.h
[modify] https://crrev.com/537343fe61b13e41227fe6968bb150132afffd1f/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/537343fe61b13e41227fe6968bb150132afffd1f/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/537343fe61b13e41227fe6968bb150132afffd1f/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h
[modify] https://crrev.com/537343fe61b13e41227fe6968bb150132afffd1f/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/537343fe61b13e41227fe6968bb150132afffd1f/tools/metrics/histograms/histograms.xml

Project Member

Comment 19 by bugdroid1@chromium.org, Oct 5 2017

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

commit d629cc6e316acf324564a3341471c1b87509d5b7
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Oct 05 01:39:54 2017

Add chrome://flags entry for BlockTabUnders on Android

The UI is not implemented yet on desktop.

Bug: 661629
Change-Id: Iee0b2134d02eb63325b20376c38500a38d49fb9a
Reviewed-on: https://chromium-review.googlesource.com/701474
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506616}
[modify] https://crrev.com/d629cc6e316acf324564a3341471c1b87509d5b7/chrome/browser/about_flags.cc
[modify] https://crrev.com/d629cc6e316acf324564a3341471c1b87509d5b7/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/d629cc6e316acf324564a3341471c1b87509d5b7/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/d629cc6e316acf324564a3341471c1b87509d5b7/tools/metrics/histograms/enums.xml

Blockedon: 772515
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 7 2017

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

commit 6102d23e5ddd1b3dd8b197053f6099fd05578122
Author: Charles Harrison <csharrison@chromium.org>
Date: Sat Oct 07 01:30:44 2017

BlockTabUnders: Log click through metrics

This CL does a few things:
1. Stops using WebContentsDelegate to show the UI. Instead, we show
   the infobar manually. This makes things easier if we want to ever
   rename the infobar since it is used for more  than just frame busts.

2. Allow the infobar to take a OnceClosure which is run when the user
   clicks the link. We use this for logging an additional enum for our
   action histogram.

3. It turned out the unit test suite was not being run on Android :(.
   This CL fixes it and also instruments the infobar code to expect
   infobars.

Bug: 661629
Change-Id: I16e7d46863ce44aa9bb14667205598b7e91c20f7
Reviewed-on: https://chromium-review.googlesource.com/701700
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507256}
[modify] https://crrev.com/6102d23e5ddd1b3dd8b197053f6099fd05578122/chrome/browser/android/tab_web_contents_delegate_android.cc
[modify] https://crrev.com/6102d23e5ddd1b3dd8b197053f6099fd05578122/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/6102d23e5ddd1b3dd8b197053f6099fd05578122/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/6102d23e5ddd1b3dd8b197053f6099fd05578122/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h
[modify] https://crrev.com/6102d23e5ddd1b3dd8b197053f6099fd05578122/chrome/browser/ui/interventions/framebust_block_message_delegate.cc
[modify] https://crrev.com/6102d23e5ddd1b3dd8b197053f6099fd05578122/chrome/browser/ui/interventions/framebust_block_message_delegate.h
[modify] https://crrev.com/6102d23e5ddd1b3dd8b197053f6099fd05578122/chrome/test/BUILD.gn
[modify] https://crrev.com/6102d23e5ddd1b3dd8b197053f6099fd05578122/tools/metrics/histograms/enums.xml

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 21 2017

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

commit 386b20f3058097507fe3600f6f0e0325848af936
Author: Charles Harrison <csharrison@chromium.org>
Date: Sat Oct 21 05:44:30 2017

[tab_under] Simplify visibility metrics and add a new one

We used to have two visibility trackers, one for the tab in general,
and one for tabs after they did a tab-under.

This doesn't actually make a whole lot of sense, as one of the trackers
is just a subset of the other one! This patch removes the tab-under
visibility tracker in favor of just logging a TimeDelta instead.

We also add a new metrics for visibility time *before* the first
tab-under, for tabs that do tab-unders.

Bug: 661629
Change-Id: Ia54cc552e5aba131d3a8e63fad7a4f7d99836a85
Reviewed-on: https://chromium-review.googlesource.com/730857
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510664}
[modify] https://crrev.com/386b20f3058097507fe3600f6f0e0325848af936/chrome/browser/ui/blocked_content/popup_opener_tab_helper.cc
[modify] https://crrev.com/386b20f3058097507fe3600f6f0e0325848af936/chrome/browser/ui/blocked_content/popup_opener_tab_helper.h
[modify] https://crrev.com/386b20f3058097507fe3600f6f0e0325848af936/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/386b20f3058097507fe3600f6f0e0325848af936/tools/metrics/histograms/histograms.xml

Cc: cbentzel@chromium.org
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 21 2017

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

commit b5838b6467841706e1c7f88cf9f20584f69e6d48
Author: Charles Harrison <csharrison@chromium.org>
Date: Sat Oct 21 17:52:07 2017

[tab_under] Report kDidTabUnder once per navigation

This metric was slightly broken, in that for clients without the
experiment enabled, we potentially reported kDidTabUnder on every
cross origin redirect that was "suspicious". This CL ensures it is
reported at most once. 

Bug: 661629
Change-Id: If919357b005762cea0e3d33edd589e90e1c9d1e2
Reviewed-on: https://chromium-review.googlesource.com/731775
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510689}
[modify] https://crrev.com/b5838b6467841706e1c7f88cf9f20584f69e6d48/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/b5838b6467841706e1c7f88cf9f20584f69e6d48/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/b5838b6467841706e1c7f88cf9f20584f69e6d48/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 22 2017

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

commit a42975d0c4941492c0ac49e8653b093731136648
Author: Charles Harrison <csharrison@chromium.org>
Date: Sun Oct 22 22:32:10 2017

[tab_under] Log block error to console

We already show some UI, but devs should probably be able to see the
chromestatus entry and find their way to the crbug.

We could also probably add a warning at some point, but I will wait till
the intent gets approval before logging something that isn't behind a
feature flag.

Bug: 661629
Change-Id: I3cb86f393b1ddda1fa5f5596823d1833b10814e9
Reviewed-on: https://chromium-review.googlesource.com/731967
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510710}
[modify] https://crrev.com/a42975d0c4941492c0ac49e8653b093731136648/chrome/browser/ui/blocked_content/tab_under_blocker_browsertest.cc
[modify] https://crrev.com/a42975d0c4941492c0ac49e8653b093731136648/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/a42975d0c4941492c0ac49e8653b093731136648/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h

Blocking: 777452
Project Member

Comment 27 by bugdroid1@chromium.org, Oct 24 2017

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

commit ba2471c660e8d83bfdc7191dd73f6549ffe32696
Author: Charles Harrison <csharrison@chromium.org>
Date: Tue Oct 24 21:48:08 2017

[tab_under] Log RAPPOR for the opener URL

We want to log UKM for this data in the near future, but that might not
be ready for some time. In the meantime we might be able to get decent
insight from RAPPOR.

Bug: 661629
Change-Id: Ibabdb5d76cf59d31d866c9177e74dcd20cfd964d
Reviewed-on: https://chromium-review.googlesource.com/735781
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511272}
[modify] https://crrev.com/ba2471c660e8d83bfdc7191dd73f6549ffe32696/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/ba2471c660e8d83bfdc7191dd73f6549ffe32696/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/ba2471c660e8d83bfdc7191dd73f6549ffe32696/tools/metrics/rappor/rappor.xml

Blockedon: 780485
Project Member

Comment 29 by bugdroid1@chromium.org, Nov 1 2017

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

commit d46ccaf5f8fc0090a4fda83a793bf0c40b57aa20
Author: Charles Harrison <csharrison@chromium.org>
Date: Wed Nov 01 20:00:07 2017

[tab_under] Add console logging unit test

Now that RenderFrameHostTesters keep track of these messages, it makes
writing unit tests pretty easy.

Bug: 661629
Change-Id: I3ddd13d7d9a83a90dba70dc6807073bf3d4b2b46
Reviewed-on: https://chromium-review.googlesource.com/749319
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513231}
[modify] https://crrev.com/d46ccaf5f8fc0090a4fda83a793bf0c40b57aa20/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc

Blockedon: 781890
Project Member

Comment 31 by bugdroid1@chromium.org, Nov 15 2017

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

commit 8ae803fed0f731dfdb6dd4969e9458e7e3677f1d
Author: Charles Harrison <csharrison@chromium.org>
Date: Wed Nov 15 23:48:34 2017

[tab_under] Log UKM of the tab-under opener

The CL adds some infra to the popup_opener_tab_helper to keep track
of the last committed ukm::SourceId. This information is used to log
UKM.

The CL also includes a minor refactor, pulling some logging code into
an anon function.

Bug: 661629
Change-Id: I76945a27c5850ee4f79ae8125b44edaae27e2b46
Reviewed-on: https://chromium-review.googlesource.com/766952
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516898}
[modify] https://crrev.com/8ae803fed0f731dfdb6dd4969e9458e7e3677f1d/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/8ae803fed0f731dfdb6dd4969e9458e7e3677f1d/chrome/browser/ui/blocked_content/popup_opener_tab_helper.cc
[modify] https://crrev.com/8ae803fed0f731dfdb6dd4969e9458e7e3677f1d/chrome/browser/ui/blocked_content/popup_opener_tab_helper.h
[modify] https://crrev.com/8ae803fed0f731dfdb6dd4969e9458e7e3677f1d/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/8ae803fed0f731dfdb6dd4969e9458e7e3677f1d/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/8ae803fed0f731dfdb6dd4969e9458e7e3677f1d/tools/metrics/ukm/ukm.xml

Project Member

Comment 32 by bugdroid1@chromium.org, Dec 4

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

commit 5e196a515894c6179a923800bcd094930939b1e1
Author: Charles Harrison <csharrison@chromium.org>
Date: Mon Dec 04 20:51:43 2017

[tab_under] Hook into the new desktop UI

This doesn't properly plumb through the click closure, which might need
some rethinking since we share the UI with framebusting.

Bug: 661629
Change-Id: I8a38140eee2f314e98183fc0874af94dda15a626
Reviewed-on: https://chromium-review.googlesource.com/763909
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521442}
[modify] https://crrev.com/5e196a515894c6179a923800bcd094930939b1e1/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/5e196a515894c6179a923800bcd094930939b1e1/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Dec 5

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

commit 93e553b0a57a25f40dca7cec07eff5f12b6282b6
Author: Charles Harrison <csharrison@chromium.org>
Date: Tue Dec 05 15:50:34 2017

[tab_under] Add UI expectations to browser tests

This change also moves more tests from interactive_ui_tests to
browser_tests since they don't really need to be interactive_ui_tests.

Bug: 661629
Change-Id: I24b1c4badf848826adf6db4511ddfb1368eb60e0
Reviewed-on: https://chromium-review.googlesource.com/806686
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521705}
[modify] https://crrev.com/93e553b0a57a25f40dca7cec07eff5f12b6282b6/chrome/browser/ui/blocked_content/tab_under_blocker_browsertest.cc
[modify] https://crrev.com/93e553b0a57a25f40dca7cec07eff5f12b6282b6/chrome/test/BUILD.gn

Project Member

Comment 34 by bugdroid1@chromium.org, Dec 5

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

commit 985bfe089ba3cc922451ce0be2a5d1d11ede01c6
Author: Charles Harrison <csharrison@chromium.org>
Date: Tue Dec 05 20:57:32 2017

[tab_under] Enable chrome://flags for desktop

Bug: 661629
Change-Id: I39858457456e28ecf4fdf5e7d3c1c875746dae78
Reviewed-on: https://chromium-review.googlesource.com/809201
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521825}
[modify] https://crrev.com/985bfe089ba3cc922451ce0be2a5d1d11ede01c6/chrome/browser/about_flags.cc

I found an interesting tab-under today, from retailmenot.com. If you click on one of their deals with a code, they:
1. Open a same-site new tab to show the code in the foreground
2. Navigate the original tab to the website the deal is for

This intervention blocks (2), despite it being more benign than traditional tab-unders. However, it is a jarring experience, and takes control from the user.
Project Member

Comment 36 by bugdroid1@chromium.org, Dec 6

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

commit 5179de1579a818b685d09934d32d137b6bb376fb
Author: Charles Harrison <csharrison@chromium.org>
Date: Wed Dec 06 19:44:17 2017

[tab_under] Add click through metrics for desktop

This CL abstracts some logic for "click position" to be shared by
both the popup tab helper, and the framebust tab helper.

Additionally, it stores a closure on the framebust tab helper to be
associated with each individual entry. This is used to collect metrics
for tab-under blocking on desktop.

Bug: 661629
Change-Id: Ibdc043c1d3068c38dad304be42b2d48133b3f21e
Reviewed-on: https://chromium-review.googlesource.com/809098
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522171}
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/content_settings/tab_specific_content_settings.cc
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/content_settings/tab_specific_content_settings.h
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/framebust_block_tab_helper.cc
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/framebust_block_tab_helper.h
[add] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/list_item_position.cc
[add] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/list_item_position.h
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/browser.cc
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/chrome/browser/ui/content_settings/framebust_block_browsertest.cc
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/5179de1579a818b685d09934d32d137b6bb376fb/tools/metrics/histograms/histograms.xml

Blockedon: 796922
Blockedon: 733736
Project Member

Comment 39 by bugdroid1@chromium.org, Feb 16

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

commit 3fa1adab1eb39947a7b4c1654fe8c2134bca1581
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Feb 16 02:32:18 2018

[tab-under] Enable on fieldtrial_testing_configs

In preparation for a beta experiment.

Bug: 661629
Change-Id: Ie63bdcbfb9e610bdfa0aa925b3f9e21644baacb8
Reviewed-on: https://chromium-review.googlesource.com/922002
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537115}
[modify] https://crrev.com/3fa1adab1eb39947a7b4c1654fe8c2134bca1581/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/3fa1adab1eb39947a7b4c1654fe8c2134bca1581/testing/variations/fieldtrial_testing_config.json

Blockedon: 814357
Blockedon: 817514
Blockedon: 818753
Labels: Hotlist-Abusive
Project Member

Comment 44 by bugdroid1@chromium.org, Mar 13

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

commit 3c5cebe8adf2269b568c4851e3ef748cd8ef848b
Author: Charles Harrison <csharrison@chromium.org>
Date: Tue Mar 13 17:18:00 2018

[tab-under] Relax suspicion to cross-site rather than cross-origin

This partially fixes breakage to gmail, which wants to redirect
mail.google.com -> accounts.google.com for auto-logout.

See the design doc for relaxing tab-under protection:
https://docs.google.com/document/d/1N-0etmATN6PhQt-m9S8sCgfz88zv8lZ2dBFdqKlzsPg/edit?ts=5aa2f94e#

It feels like this change is the least controversial and it won't reduce the
efficacy of the intervention.

Bug: 661629
Change-Id: I01c7435a12515aacf011a722ee155d60cb1d6bae
Reviewed-on: https://chromium-review.googlesource.com/959689
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542835}
[modify] https://crrev.com/3c5cebe8adf2269b568c4851e3ef748cd8ef848b/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/3c5cebe8adf2269b568c4851e3ef748cd8ef848b/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/3c5cebe8adf2269b568c4851e3ef748cd8ef848b/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h

Project Member

Comment 45 by bugdroid1@chromium.org, Mar 14

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

commit a4caf65b35b10e22ad76213f199f25dfe1e234dc
Author: Charles Harrison <csharrison@chromium.org>
Date: Wed Mar 14 19:44:14 2018

[tab-under] Add UMA for engagement score

This will help us see how much we stand to lose by relaxing tab-under
protection to sites with low (or 0) site engagement scores.

A followup will probably land a relaxed blocking scheme for origins
with nonzero engagement, possibly behind another variation flag.
This data will give us better insight into whether our hypothesis
is right: namely, that tab-under destinations will often have low
engagement.

See doc for tab-under relaxation:
https://docs.google.com/document/d/1N-0etmATN6PhQt-m9S8sCgfz88zv8lZ2dBFdqKlzsPg/edit?ts=5aa73bff#

Bug: 661629
Change-Id: I535710fdde0a162c53994b0df99f6333a8af99ce
Reviewed-on: https://chromium-review.googlesource.com/959538
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543161}
[modify] https://crrev.com/a4caf65b35b10e22ad76213f199f25dfe1e234dc/chrome/browser/engagement/site_engagement_service.cc
[modify] https://crrev.com/a4caf65b35b10e22ad76213f199f25dfe1e234dc/chrome/browser/engagement/site_engagement_service.h
[modify] https://crrev.com/a4caf65b35b10e22ad76213f199f25dfe1e234dc/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/a4caf65b35b10e22ad76213f199f25dfe1e234dc/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/a4caf65b35b10e22ad76213f199f25dfe1e234dc/tools/metrics/histograms/histograms.xml

Project Member

Comment 46 by bugdroid1@chromium.org, Mar 16

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

commit 80f495fdbb4265b952040b991c4805fd80d42d56
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Mar 16 19:22:07 2018

[tab-under] Gate via Site Engagement

This CL adds a variation parameter which encodes a threshold for
the Site Engagement score [1] required to do tab-unders. By default,
we allow tab-unders to origins that have any non-zero engagement.

This moves the Tab.TabUnder.EngagementScore metric as the last check
of IsSuspiciousClientRedirect, to simulate the scores blocked tab-unders
would have if we removed that check.

[1]: https://www.chromium.org/developers/design-documents/site-engagement

Bug: 661629
Change-Id: I48ba83ab8cc58641cc21dcad062c313f1d1290ef
Reviewed-on: https://chromium-review.googlesource.com/965403
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543779}
[modify] https://crrev.com/80f495fdbb4265b952040b991c4805fd80d42d56/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/80f495fdbb4265b952040b991c4805fd80d42d56/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc
[modify] https://crrev.com/80f495fdbb4265b952040b991c4805fd80d42d56/chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h

Blockedon: 823700
Cc: -ojan@chromium.org

Sign in to add a comment