Commit f5f3e458 authored by Pete Williamson's avatar Pete Williamson Committed by Commit Bot

[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/1281176Reviewed-by: default avatarDmitry Titov <dimich@chromium.org>
Commit-Queue: Peter Williamson <petewil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600037}
parent eeb3499a
......@@ -30,6 +30,7 @@ import java.util.concurrent.TimeUnit;
*/
@JNINamespace("explore_sites")
public class ExploreSitesBackgroundTask extends NativeBackgroundTask {
private static final String TAG = "ESBackgroundTask";
public static final int DEFAULT_DELAY_HOURS = 25;
public static final int DEFAULT_FLEX_HOURS = 2;
......
......@@ -38,6 +38,7 @@ import java.util.List;
* Provides functionality when the user interacts with the explore sites page.
*/
public class ExploreSitesPage extends BasicNativePage {
private static final String TAG = "ExploreSitesPage";
private static final String CONTEXT_MENU_USER_ACTION_PREFIX = "ExploreSites";
static final PropertyModel.WritableIntPropertyKey STATUS_KEY =
new PropertyModel.WritableIntPropertyKey();
......
......@@ -34,6 +34,7 @@ import java.util.Map;
* It abstracts general tasks related to initializing and fetching data for the UI.
*/
public class ExploreSitesSection {
private static final String TAG = "ExploreSitesSection";
private static final int MAX_CATEGORIES = 3;
// This is a number of UMA histogram buckets that should be an upper bound
// of MAX_CATEGORIES over time, plus 1 for "More" button. If MAX_CATEGORIES
......
......@@ -116,6 +116,19 @@ void ExploreSitesServiceImpl::BlacklistSite(const std::string& url) {
// TODO(https://crbug.com/893845): Remove cached category icon if affected.
}
// Validate the catalog. Note this does not take ownership of the pointer.
void ValidateCatalog(Catalog* catalog) {
// TODO(https://crbug.com/895541): Validate that the category type is in the
// valid range. Also make sure the site has a title, else remove it.
// Convert the URLs to a standard format.
for (auto& category : *catalog->mutable_categories()) {
for (auto& site : *category.mutable_sites()) {
site.set_site_url(GURL(site.site_url()).spec());
}
}
}
void ExploreSitesServiceImpl::OnCatalogFetched(
ExploreSitesRequestStatus status,
std::unique_ptr<std::string> serialized_protobuf) {
......@@ -142,6 +155,9 @@ void ExploreSitesServiceImpl::OnCatalogFetched(
std::string catalog_version = catalog_response->version_token();
std::unique_ptr<Catalog> catalog(catalog_response->release_catalog());
// Check the catalog, canonicalizing any URLs in it.
ValidateCatalog(catalog.get());
// Add the catalog to our internal database.
task_queue_.AddTask(std::make_unique<ImportCatalogTask>(
explore_sites_store_.get(), catalog_version, std::move(catalog),
......
......@@ -18,6 +18,7 @@
namespace {
const char kCategoryName[] = "Technology";
const char kSite1UrlNoTrailingSlash[] = "https://example.com";
const char kSite1Url[] = "https://example.com/";
const char kSite2Url[] = "https://sample.com/";
const char kSite1Name[] = "example";
......@@ -135,8 +136,9 @@ std::string ExploreSitesServiceImplTest::CreateTestDataProto() {
// Fill in fields we need to add to the EoS database.
// Create two sites.
site1->set_site_url(kSite1Url);
// Create two sites. We create one with no trailing slash. The trailing
// slash should be added when we convert it to a GURL for canonicalization.
site1->set_site_url(kSite1UrlNoTrailingSlash);
site1->set_title(kSite1Name);
site2->set_site_url(kSite2Url);
site2->set_title(kSite2Name);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment