[subresource_filter] Warning sites should not show up in settings UI |
||||||||
Issue descriptionThis is a problem with our implementation of warning sites. A bug in the code considers them activated (even though nothing is blocked), so all our settings UIs will show up for that site. This includes: - Android Page Info / Site Details - Desktop OIB The site should not show up in these UI surfaces, as it implies we will be blocking ads on the site (we aren't). This is a regression issue in M63, when warning sites were implemented. I have a fix in progress: https://chromium-review.googlesource.com/c/chromium/src/+/744442
,
Oct 30 2017
,
Oct 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16139bbf766b741be3037d19dba381dfcc311d9c commit 16139bbf766b741be3037d19dba381dfcc311d9c Author: Charles Harrison <csharrison@chromium.org> Date: Mon Oct 30 21:54:52 2017 [subresource_filter] Do not populate metadata for warning sites. Currently, we populate website data metadata for sites which activate as ENABLED. This metadata is used to trigger showing the "Ads" setting in the settings UI, OIB, and page info. However, we also activate as ENABLED for warning sites, and decrease activation level downstream in the driver factory. This means that warning sites will show up in the various settings UIs as having intrusive ads. This CL makes it so that warning sites do not populate the metadata. Bug: 779673 Change-Id: I972faf6aeda6884b584c8383918fa7a9ba966b6c Reviewed-on: https://chromium-review.googlesource.com/744442 Reviewed-by: Shivani Sharma <shivanisha@chromium.org> Commit-Queue: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/heads/master@{#512636} [modify] https://crrev.com/16139bbf766b741be3037d19dba381dfcc311d9c/chrome/browser/subresource_filter/subresource_filter_unittest.cc [modify] https://crrev.com/16139bbf766b741be3037d19dba381dfcc311d9c/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc
,
Oct 31 2017
This needs merge to M63. The CL logic is very minimal (one line diff), and it fixes a regression where sites where Chrome is *not* blocking ads (they just get devtools warnings), show up in a few different UIs that Chrome *is* blocking ads on the site.
,
Oct 31 2017
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 31 2017
Approving merge to M63 branch 3239 based on comment #4. Please merge ASAP so we can take it in for tomorrow's Beta release. Thank you.
,
Oct 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e97cf9d3f6843649c48b1f7ba446dafa9cc68e89 commit e97cf9d3f6843649c48b1f7ba446dafa9cc68e89 Author: Charles Harrison <csharrison@chromium.org> Date: Tue Oct 31 15:09:15 2017 [subresource_filter] Do not populate metadata for warning sites. Currently, we populate website data metadata for sites which activate as ENABLED. This metadata is used to trigger showing the "Ads" setting in the settings UI, OIB, and page info. However, we also activate as ENABLED for warning sites, and decrease activation level downstream in the driver factory. This means that warning sites will show up in the various settings UIs as having intrusive ads. This CL makes it so that warning sites do not populate the metadata. TBR=shivanisha@chromium.org Bug: 779673 Change-Id: I972faf6aeda6884b584c8383918fa7a9ba966b6c Reviewed-on: https://chromium-review.googlesource.com/744442 Reviewed-by: Shivani Sharma <shivanisha@chromium.org> Commit-Queue: Charlie Harrison <csharrison@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#512636}(cherry picked from commit 16139bbf766b741be3037d19dba381dfcc311d9c) Reviewed-on: https://chromium-review.googlesource.com/746822 Reviewed-by: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#313} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/e97cf9d3f6843649c48b1f7ba446dafa9cc68e89/chrome/browser/subresource_filter/subresource_filter_unittest.cc [modify] https://crrev.com/e97cf9d3f6843649c48b1f7ba446dafa9cc68e89/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle.cc
,
Oct 31 2017
,
Nov 3 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by csharrison@chromium.org
, Oct 30 2017