New issue
Advanced search Search tips

Issue 893845 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[EoS] Implement site blocking

Project Member Reported by petewil@chromium.org, Oct 9

Issue description

The user can chose to remove a site (blacklist it) from the list of Explore on Sites (EoS) recommendations.  We have the menu item on the EoS page, but we need to hook it up to actually remove the site.

This includes adding the site to the blacklist table in the database, removing from the view, removing from the model, replacing the site with another (if there are more than 8 available), and ensuring that we don't display sites in the blacklist for future openings of the EoS page.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 11

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

commit 3c8719bab320dc41751baaefcd3f270a19aded59
Author: Pete Williamson <petewil@chromium.org>
Date: Thu Oct 11 00:22:57 2018

[EoS] Site blacklisting, part 1

Catch the user request to blacklist a site, and add the site to the
site_blacklist table in the EoS database.  Unit tests provided.

Bug: 893845
Change-Id: I1edeaab87e7a533850e0ad81a3c6874708cde44c
Reviewed-on: https://chromium-review.googlesource.com/c/1273986
Reviewed-by: Cathy Li <chili@chromium.org>
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598587}
[modify] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBridge.java
[modify] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategoryCardView.java
[modify] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/browser/BUILD.gn
[add] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/browser/android/explore_sites/blacklist_site_task.cc
[add] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/browser/android/explore_sites/blacklist_site_task.h
[add] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/browser/android/explore_sites/blacklist_site_task_unittest.cc
[modify] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/browser/android/explore_sites/explore_sites_bridge.cc
[modify] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/browser/android/explore_sites/explore_sites_service.h
[modify] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
[modify] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/browser/android/explore_sites/explore_sites_service_impl.h
[modify] https://crrev.com/3c8719bab320dc41751baaefcd3f270a19aded59/chrome/test/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 12

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

commit da6c8bfcbd9edb1a2dfa29642124c3b762c05894
Author: Pete Williamson <petewil@chromium.org>
Date: Fri Oct 12 19:43:11 2018

[EoS] Site blacklisting part 2

Make sure sites in the blacklist are not returned as part of a catalog get
operation. This builds upon Site blacklisting part 1.

Bug: 893845
Change-Id: I38686753167bbcb8e9f9d75d6c323bda813cab31
Reviewed-on: https://chromium-review.googlesource.com/c/1274058
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599325}
[modify] https://crrev.com/da6c8bfcbd9edb1a2dfa29642124c3b762c05894/chrome/browser/android/explore_sites/get_catalog_task.cc
[modify] https://crrev.com/da6c8bfcbd9edb1a2dfa29642124c3b762c05894/chrome/browser/android/explore_sites/get_catalog_task_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 13

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

commit a564f6e01d3486288673b7f63e679e004108f067
Author: Pete Williamson <petewil@chromium.org>
Date: Sat Oct 13 00:24:27 2018

[EoS] Site Blacklisting part 3 - Remove sites from the UI.

When the user chooses the "remove" item from the long press menu,
remove the site from the UI.  It is already being removed from the database.

Bug: 893845
Change-Id: Icbd8292475983f4c263113043a001d11f3f09521
Reviewed-on: https://chromium-review.googlesource.com/c/1278955
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599438}
[modify] https://crrev.com/a564f6e01d3486288673b7f63e679e004108f067/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategory.java
[modify] https://crrev.com/a564f6e01d3486288673b7f63e679e004108f067/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategoryCardView.java

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 16

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

commit f5f3e4589bce29e58ca0c2df2285374087881da9
Author: Pete Williamson <petewil@chromium.org>
Date: Tue Oct 16 17:52:23 2018

[EoS] Fix the blacklist application

Some sites were being added to the blacklist properly, but not filtered
out when doing the join against the blacklist table.  This happened
because the sites in our database did not consistently end with "/",
but sometimes a "/" got added before we put the site into the blacklist.

The fix here is to pass all sites through GURL and get the spec after
parsing, so they will be consistent.

Bug: 893845
Change-Id: Id705aab833da546c57950fad115c694b96c5fd24
Reviewed-on: https://chromium-review.googlesource.com/c/1281176
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600037}
[modify] https://crrev.com/f5f3e4589bce29e58ca0c2df2285374087881da9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTask.java
[modify] https://crrev.com/f5f3e4589bce29e58ca0c2df2285374087881da9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesPage.java
[modify] https://crrev.com/f5f3e4589bce29e58ca0c2df2285374087881da9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java
[modify] https://crrev.com/f5f3e4589bce29e58ca0c2df2285374087881da9/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
[modify] https://crrev.com/f5f3e4589bce29e58ca0c2df2285374087881da9/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc

Labels: Merge-Request-69
I request merging changelists 2, 3, and 4 in this bug into M71 branch.  They have been tested in Canary.
Labels: -Merge-Request-69 Merge-Request-71
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 20

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 22

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d29132bf8dc021f7fbb964c7917523971d7af6e8

commit d29132bf8dc021f7fbb964c7917523971d7af6e8
Author: Pete Williamson <petewil@chromium.org>
Date: Mon Oct 22 16:25:24 2018

[EoS] Site blacklisting part 2

Make sure sites in the blacklist are not returned as part of a catalog get
operation. This builds upon Site blacklisting part 1.

Bug: 893845
Change-Id: I38686753167bbcb8e9f9d75d6c323bda813cab31
Reviewed-on: https://chromium-review.googlesource.com/c/1274058
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599325}(cherry picked from commit da6c8bfcbd9edb1a2dfa29642124c3b762c05894)
Reviewed-on: https://chromium-review.googlesource.com/c/1293993
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#213}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/d29132bf8dc021f7fbb964c7917523971d7af6e8/chrome/browser/android/explore_sites/get_catalog_task.cc
[modify] https://crrev.com/d29132bf8dc021f7fbb964c7917523971d7af6e8/chrome/browser/android/explore_sites/get_catalog_task_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 22

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

commit 3bbd956ab7375c946dbdb3a8e5d3c1a9ed5fa814
Author: Pete Williamson <petewil@chromium.org>
Date: Mon Oct 22 16:26:25 2018

[EoS] Site Blacklisting part 3 - Remove sites from the UI.

When the user chooses the "remove" item from the long press menu,
remove the site from the UI.  It is already being removed from the database.

Bug: 893845
Change-Id: Icbd8292475983f4c263113043a001d11f3f09521
Reviewed-on: https://chromium-review.googlesource.com/c/1278955
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599438}(cherry picked from commit a564f6e01d3486288673b7f63e679e004108f067)
Reviewed-on: https://chromium-review.googlesource.com/c/1294013
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#214}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/3bbd956ab7375c946dbdb3a8e5d3c1a9ed5fa814/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategory.java
[modify] https://crrev.com/3bbd956ab7375c946dbdb3a8e5d3c1a9ed5fa814/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesCategoryCardView.java

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 22

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

commit 9b102f4d05e81d8673b5ad284bc734d9d99549d9
Author: Pete Williamson <petewil@chromium.org>
Date: Mon Oct 22 16:27:14 2018

[EoS] Fix the blacklist application

Some sites were being added to the blacklist properly, but not filtered
out when doing the join against the blacklist table.  This happened
because the sites in our database did not consistently end with "/",
but sometimes a "/" got added before we put the site into the blacklist.

The fix here is to pass all sites through GURL and get the spec after
parsing, so they will be consistent.

Bug: 893845
Change-Id: Id705aab833da546c57950fad115c694b96c5fd24
Reviewed-on: https://chromium-review.googlesource.com/c/1281176
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600037}(cherry picked from commit f5f3e4589bce29e58ca0c2df2285374087881da9)
Reviewed-on: https://chromium-review.googlesource.com/c/1293994
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#215}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/9b102f4d05e81d8673b5ad284bc734d9d99549d9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBackgroundTask.java
[modify] https://crrev.com/9b102f4d05e81d8673b5ad284bc734d9d99549d9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesPage.java
[modify] https://crrev.com/9b102f4d05e81d8673b5ad284bc734d9d99549d9/chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesSection.java
[modify] https://crrev.com/9b102f4d05e81d8673b5ad284bc734d9d99549d9/chrome/browser/android/explore_sites/explore_sites_service_impl.cc
[modify] https://crrev.com/9b102f4d05e81d8673b5ad284bc734d9d99549d9/chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/3bbd956ab7375c946dbdb3a8e5d3c1a9ed5fa814

Commit: 3bbd956ab7375c946dbdb3a8e5d3c1a9ed5fa814
Author: petewil@chromium.org
Commiter: petewil@chromium.org
Date: 2018-10-22 16:26:25 +0000 UTC

[EoS] Site Blacklisting part 3 - Remove sites from the UI.

When the user chooses the "remove" item from the long press menu,
remove the site from the UI.  It is already being removed from the database.

Bug: 893845
Change-Id: Icbd8292475983f4c263113043a001d11f3f09521
Reviewed-on: https://chromium-review.googlesource.com/c/1278955
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599438}(cherry picked from commit a564f6e01d3486288673b7f63e679e004108f067)
Reviewed-on: https://chromium-review.googlesource.com/c/1294013
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#214}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/9b102f4d05e81d8673b5ad284bc734d9d99549d9

Commit: 9b102f4d05e81d8673b5ad284bc734d9d99549d9
Author: petewil@chromium.org
Commiter: petewil@chromium.org
Date: 2018-10-22 16:27:14 +0000 UTC

[EoS] Fix the blacklist application

Some sites were being added to the blacklist properly, but not filtered
out when doing the join against the blacklist table.  This happened
because the sites in our database did not consistently end with "/",
but sometimes a "/" got added before we put the site into the blacklist.

The fix here is to pass all sites through GURL and get the spec after
parsing, so they will be consistent.

Bug: 893845
Change-Id: Id705aab833da546c57950fad115c694b96c5fd24
Reviewed-on: https://chromium-review.googlesource.com/c/1281176
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#600037}(cherry picked from commit f5f3e4589bce29e58ca0c2df2285374087881da9)
Reviewed-on: https://chromium-review.googlesource.com/c/1293994
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#215}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/d29132bf8dc021f7fbb964c7917523971d7af6e8

Commit: d29132bf8dc021f7fbb964c7917523971d7af6e8
Author: petewil@chromium.org
Commiter: petewil@chromium.org
Date: 2018-10-22 16:25:24 +0000 UTC

[EoS] Site blacklisting part 2

Make sure sites in the blacklist are not returned as part of a catalog get
operation. This builds upon Site blacklisting part 1.

Bug: 893845
Change-Id: I38686753167bbcb8e9f9d75d6c323bda813cab31
Reviewed-on: https://chromium-review.googlesource.com/c/1274058
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599325}(cherry picked from commit da6c8bfcbd9edb1a2dfa29642124c3b762c05894)
Reviewed-on: https://chromium-review.googlesource.com/c/1293993
Reviewed-by: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#213}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Labels: -merge-merged-3578 -Merge-Merged-71-3578

Sign in to add a comment