Disable Safe Search whitelist for Google properties with feature flag.

We plan to remove this code in the next release, but for now we'll keep
it behind a "live switch" so that we may re-enable in the server if
necessary.

Bug: 859097
Change-Id: I6c86e92de4b79c1a622a559b30840b9442f76716
Reviewed-on: https://chromium-review.googlesource.com/1122636
Commit-Queue: Bruno Kim Medeiros Cesar <brunokim@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572554}

components/safe_search_api/BUILD.gn

@@ -26,6 +26,7 @@ source_set("unit_tests") {
   deps = [
     ":safe_search_api",
     "//base",
+    "//base/test:test_support",
     "//net",
     "//net/traffic_annotation:test_support",
     "//services/network:test_support",

components/safe_search_api/url_checker.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/callback.h"
+#include "base/feature_list.h"
 #include "base/json/json_reader.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
@@ -73,6 +74,10 @@ bool ParseResponse(const std::string& response, bool* is_porn) {
 
 }  // namespace
 
+// Consider all URLs within a google domain to be safe.
+const base::Feature kAllowAllGoogleUrls{"SafeSearchAllowAllGoogleURLs",
+                                        base::FEATURE_DISABLED_BY_DEFAULT};
+
 struct URLChecker::Check {
   Check(const GURL& url,
         std::unique_ptr<network::SimpleURLLoader> simple_url_loader,
@@ -127,8 +132,8 @@ URLChecker::URLChecker(
 URLChecker::~URLChecker() = default;
 
 bool URLChecker::CheckURL(const GURL& url, CheckCallback callback) {
-  // TODO(treib): Hack: For now, allow all Google URLs to save QPS. If we ever
-  // remove this, we should find a way to allow at least the NTP.
+  if (base::FeatureList::IsEnabled(kAllowAllGoogleUrls)) {
+    // TODO(treib): Hack: For now, allow all Google URLs to save QPS.
     if (google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
                                        google_util::ALLOW_NON_STANDARD_PORTS)) {
       std::move(callback).Run(url, Classification::SAFE, false);
@@ -136,11 +141,13 @@ bool URLChecker::CheckURL(const GURL& url, CheckCallback callback) {
     }
     // TODO(treib): Hack: For now, allow all YouTube URLs since YouTube has its
     // own Safety Mode anyway.
-  if (google_util::IsYoutubeDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
+    if (google_util::IsYoutubeDomainUrl(
+            url, google_util::ALLOW_SUBDOMAIN,
             google_util::ALLOW_NON_STANDARD_PORTS)) {
       std::move(callback).Run(url, Classification::SAFE, false);
       return true;
     }
+  }
 
   auto cache_it = cache_.Get(url);
   if (cache_it != cache_.end()) {

components/safe_search_api/url_checker.h

@@ -22,11 +22,18 @@ namespace network {
 class SharedURLLoaderFactory;
 }  // namespace network
 
+namespace base {
+struct Feature;
+}
+
 namespace safe_search_api {
 
 // The SafeSearch API classification of a URL.
 enum class Classification { SAFE, UNSAFE };
 
+// Visible for testing.
+extern const base::Feature kAllowAllGoogleUrls;
+
 // This class uses the SafeSearch API to check the SafeSearch classification
 // of the content on a given URL and returns the result asynchronously
 // via a callback.

components/safe_search_api/url_checker_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "net/base/net_errors.h"
@@ -59,6 +60,10 @@ std::string BuildResponse(bool is_porn) {
   return result;
 }
 
+std::string BuildPornResponse() {
+  return BuildResponse(true);
+}
+
 }  // namespace
 
 class SafeSearchURLCheckerTest : public testing::Test {
@@ -195,4 +200,44 @@ TEST_F(SafeSearchURLCheckerTest, CacheTimeout) {
   ASSERT_FALSE(SendValidResponse(url, true));
 }
 
+TEST_F(SafeSearchURLCheckerTest, AllowAllGoogleURLs) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(kAllowAllGoogleUrls);
+  {
+    GURL url("https://sites.google.com/porn");
+    EXPECT_CALL(*this, OnCheckDone(url, Classification::SAFE, _));
+    // No server interaction.
+    bool cache_hit = CheckURL(url);
+    ASSERT_TRUE(cache_hit);
+  }
+  {
+    GURL url("https://youtube.com/porn");
+    EXPECT_CALL(*this, OnCheckDone(url, Classification::SAFE, _));
+    // No server interaction
+    bool cache_hit = CheckURL(url);
+    ASSERT_TRUE(cache_hit);
+  }
+}
+
+TEST_F(SafeSearchURLCheckerTest, NoAllowAllGoogleURLs) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndDisableFeature(kAllowAllGoogleUrls);
+  {
+    GURL url("https://sites.google.com/porn");
+    EXPECT_CALL(*this, OnCheckDone(url, Classification::UNSAFE, _));
+    SetupResponse(url, net::OK, BuildPornResponse());
+    bool cache_hit = CheckURL(url);
+    ASSERT_FALSE(cache_hit);
+    WaitForResponse();
+  }
+  {
+    GURL url("https://youtube.com/porn");
+    EXPECT_CALL(*this, OnCheckDone(url, Classification::UNSAFE, _));
+    SetupResponse(url, net::OK, BuildPornResponse());
+    bool cache_hit = CheckURL(url);
+    ASSERT_FALSE(cache_hit);
+    WaitForResponse();
+  }
+}
+
 }  // namespace safe_search_api