Move safe_search_api to //components

The SafeSearchURLChecker didn't depend on //chrome. Move it to a new
safe_search_api component.

We will reuse safe_search_api::URLChecker in a later CL for filtering
URLs based on the SafeSearch API (a generalization of
SupervisedUserURLFilter).

Because we're creating a namespace, we can drop the SafeSearch prefix
and move the Classification enum out of the URLChecker class. Also clean
up presubmit issues in changed lines.

Bug: 819405
Change-Id: Iaf411b1232eb937f93056d8e3553435fb7779bee
Reviewed-on: https://chromium-review.googlesource.com/1081892Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Cait Phillips <caitkp@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565486}

chrome/browser/BUILD.gn

@@ -1277,8 +1277,6 @@ jumbo_split_static_library("browser") {
     "resource_coordinator/time.h",
     "resources_util.cc",
     "resources_util.h",
-    "safe_search_api/safe_search_url_checker.cc",
-    "safe_search_api/safe_search_url_checker.h",
     "search/instant_io_context.cc",
     "search/instant_io_context.h",
     "search/search.cc",
@@ -1725,6 +1723,7 @@ jumbo_split_static_library("browser") {
     "//components/rappor:rappor_recorder",
     "//components/renderer_context_menu",
     "//components/resources",
+    "//components/safe_search_api",
     "//components/search",
     "//components/search_engines",
     "//components/search_provider_logos",

chrome/browser/supervised_user/supervised_user_url_filter.cc

@@ -60,11 +60,11 @@ struct HashHostnameHash {
 
 SupervisedUserURLFilter::FilteringBehavior
 GetBehaviorFromSafeSearchClassification(
-    SafeSearchURLChecker::Classification classification) {
+    safe_search_api::Classification classification) {
   switch (classification) {
-    case SafeSearchURLChecker::Classification::SAFE:
+    case safe_search_api::Classification::SAFE:
       return SupervisedUserURLFilter::ALLOW;
-    case SafeSearchURLChecker::Classification::UNSAFE:
+    case safe_search_api::Classification::UNSAFE:
       return SupervisedUserURLFilter::BLOCK;
   }
   NOTREACHED();
@@ -553,8 +553,8 @@ void SupervisedUserURLFilter::InitAsyncURLChecker(
             "family dashboard."
           policy_exception_justification: "Not implemented."
         })");
-  async_url_checker_.reset(new SafeSearchURLChecker(
-      std::move(url_loader_factory), traffic_annotation));
+  async_url_checker_ = std::make_unique<safe_search_api::URLChecker>(
+      std::move(url_loader_factory), traffic_annotation);
 }
 
 void SupervisedUserURLFilter::ClearAsyncURLChecker() {
@@ -675,7 +675,7 @@ void SupervisedUserURLFilter::SetContents(std::unique_ptr<Contents> contents) {
 void SupervisedUserURLFilter::CheckCallback(
     FilteringBehaviorCallback callback,
     const GURL& url,
-    SafeSearchURLChecker::Classification classification,
+    safe_search_api::Classification classification,
     bool uncertain) const {
   DCHECK(default_behavior_ != BLOCK);
 

chrome/browser/supervised_user/supervised_user_url_filter.h

@@ -15,9 +15,9 @@
 #include "base/observer_list.h"
 #include "base/sequence_checker.h"
 #include "base/values.h"
-#include "chrome/browser/safe_search_api/safe_search_url_checker.h"
 #include "chrome/browser/supervised_user/supervised_user_site_list.h"
 #include "chrome/browser/supervised_user/supervised_users.h"
+#include "components/safe_search_api/url_checker.h"
 #include "components/supervised_user_error_page/supervised_user_error_page.h"
 #include "third_party/re2/src/re2/re2.h"
 
@@ -192,7 +192,7 @@ class SupervisedUserURLFilter {
 
   void CheckCallback(FilteringBehaviorCallback callback,
                      const GURL& url,
-                     SafeSearchURLChecker::Classification classification,
+                     safe_search_api::Classification classification,
                      bool uncertain) const;
 
   // This is mutable to allow notification in const member functions.
@@ -212,7 +212,7 @@ class SupervisedUserURLFilter {
   // Not owned.
   const SupervisedUserBlacklist* blacklist_;
 
-  std::unique_ptr<SafeSearchURLChecker> async_url_checker_;
+  std::unique_ptr<safe_search_api::URLChecker> async_url_checker_;
 
   re2::RE2 amp_cache_path_regex_;
   re2::RE2 google_amp_viewer_path_regex_;

chrome/test/BUILD.gn

@@ -3529,7 +3529,6 @@ test("unit_tests") {
       "../browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_test_utils.h",
       "../browser/safe_browsing/test_extension_event_observer.cc",
       "../browser/safe_browsing/test_extension_event_observer.h",
-      "../browser/safe_search_api/safe_search_url_checker_unittest.cc",
       "../browser/ssl/ssl_blocking_page_unittest.cc",
       "../browser/sync/glue/extensions_activity_monitor_unittest.cc",
       "../browser/sync_file_system/drive_backend/callback_helper_unittest.cc",

components/BUILD.gn

@@ -127,6 +127,7 @@ test("components_unittests") {
     "//components/query_parser:unit_tests",
     "//components/rappor:unit_tests",
     "//components/reading_list/core:unit_tests",
+    "//components/safe_search_api:unit_tests",
     "//components/search:unit_tests",
     "//components/search_engines:unit_tests",
     "//components/search_provider_logos:unit_tests",

components/safe_search_api/BUILD.gn

+# Copyright 2018 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+source_set("safe_search_api") {
+  sources = [
+    "url_checker.cc",
+    "url_checker.h",
+  ]
+
+  deps = [
+    "//base",
+    "//components/google/core/browser",
+    "//google_apis",
+    "//net",
+    "//services/network/public/cpp",
+    "//url",
+  ]
+}
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [
+    "url_checker_unittest.cc",
+  ]
+  deps = [
+    ":safe_search_api",
+    "//base",
+    "//net",
+    "//net/traffic_annotation:test_support",
+    "//services/network:test_support",
+    "//services/network/public/cpp",
+    "//testing/gmock",
+    "//testing/gtest",
+    "//url",
+  ]
+}

components/safe_search_api/DEPS

+include_rules = [
+  "+components/google/core/browser",
+  "+google_apis",
+  "+net",
+  "+services/network",
+]

chrome/browser/safe_search_api/safe_search_url_checker.cc → components/safe_search_api/url_checker.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/safe_search_api/safe_search_url_checker.h"
+#include "components/safe_search_api/url_checker.h"
 
 #include <string>
 #include <utility>
@@ -26,6 +26,8 @@
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "url/url_constants.h"
 
+namespace safe_search_api {
+
 namespace {
 
 const char kSafeSearchApiUrl[] =
@@ -71,7 +73,7 @@ bool ParseResponse(const std::string& response, bool* is_porn) {
 
 }  // namespace
 
-struct SafeSearchURLChecker::Check {
+struct URLChecker::Check {
   Check(const GURL& url,
         std::unique_ptr<network::SimpleURLLoader> simple_url_loader,
         CheckCallback callback);
@@ -83,7 +85,7 @@ struct SafeSearchURLChecker::Check {
   base::TimeTicks start_time;
 };
 
-SafeSearchURLChecker::Check::Check(
+URLChecker::Check::Check(
     const GURL& url,
     std::unique_ptr<network::SimpleURLLoader> simple_url_loader,
     CheckCallback callback)
@@ -93,26 +95,26 @@ SafeSearchURLChecker::Check::Check(
   callbacks.push_back(std::move(callback));
 }
 
-SafeSearchURLChecker::Check::~Check() {
+URLChecker::Check::~Check() {
   for (const CheckCallback& callback : callbacks) {
     DCHECK(!callback);
   }
 }
 
-SafeSearchURLChecker::CheckResult::CheckResult(Classification classification,
-                                               bool uncertain)
+URLChecker::CheckResult::CheckResult(Classification classification,
+                                     bool uncertain)
     : classification(classification),
       uncertain(uncertain),
       timestamp(base::TimeTicks::Now()) {}
 
-SafeSearchURLChecker::SafeSearchURLChecker(
+URLChecker::URLChecker(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     const net::NetworkTrafficAnnotationTag& traffic_annotation)
-    : SafeSearchURLChecker(std::move(url_loader_factory),
-                           traffic_annotation,
-                           kDefaultCacheSize) {}
+    : URLChecker(std::move(url_loader_factory),
+                 traffic_annotation,
+                 kDefaultCacheSize) {}
 
-SafeSearchURLChecker::SafeSearchURLChecker(
+URLChecker::URLChecker(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     const net::NetworkTrafficAnnotationTag& traffic_annotation,
     size_t cache_size)
@@ -122,9 +124,9 @@ SafeSearchURLChecker::SafeSearchURLChecker(
       cache_timeout_(
           base::TimeDelta::FromSeconds(kDefaultCacheTimeoutSeconds)) {}
 
-SafeSearchURLChecker::~SafeSearchURLChecker() {}
+URLChecker::~URLChecker() = default;
 
-bool SafeSearchURLChecker::CheckURL(const GURL& url, CheckCallback callback) {
+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 (google_util::IsGoogleDomainUrl(url, google_util::ALLOW_SUBDOMAIN,
@@ -183,12 +185,12 @@ bool SafeSearchURLChecker::CheckURL(const GURL& url, CheckCallback callback) {
   network::SimpleURLLoader* loader = it->get()->simple_url_loader.get();
   loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
       url_loader_factory_.get(),
-      base::BindOnce(&SafeSearchURLChecker::OnSimpleLoaderComplete,
+      base::BindOnce(&URLChecker::OnSimpleLoaderComplete,
                      base::Unretained(this), std::move(it)));
   return false;
 }
 
-void SafeSearchURLChecker::OnSimpleLoaderComplete(
+void URLChecker::OnSimpleLoaderComplete(
     CheckList::iterator it,
     std::unique_ptr<std::string> response_body) {
   Check* check = it->get();
@@ -219,3 +221,5 @@ void SafeSearchURLChecker::OnSimpleLoaderComplete(
   for (size_t i = 0; i < callbacks.size(); i++)
     std::move(callbacks[i]).Run(url, classification, uncertain);
 }
+
+}  // namespace safe_search_api

chrome/browser/safe_search_api/safe_search_url_checker.h → components/safe_search_api/url_checker.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_SAFE_SEARCH_API_SAFE_SEARCH_URL_CHECKER_H_
-#define CHROME_BROWSER_SAFE_SEARCH_API_SAFE_SEARCH_URL_CHECKER_H_
+#ifndef COMPONENTS_SAFE_SEARCH_API_URL_CHECKER_H_
+#define COMPONENTS_SAFE_SEARCH_API_URL_CHECKER_H_
 
 #include <stddef.h>
 
@@ -22,25 +22,26 @@ namespace network {
 class SharedURLLoaderFactory;
 }  // namespace network
 
+namespace safe_search_api {
+
+// The SafeSearch API classification of a URL.
+enum class Classification { SAFE, UNSAFE };
+
 // 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.
-class SafeSearchURLChecker {
+class URLChecker {
  public:
-  enum class Classification { SAFE, UNSAFE };
-
   // Returns whether |url| should be blocked. Called from CheckURL.
   using CheckCallback = base::OnceCallback<
       void(const GURL&, Classification classification, bool /* uncertain */)>;
 
-  explicit SafeSearchURLChecker(
-      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-      const net::NetworkTrafficAnnotationTag& traffic_annotation);
-  SafeSearchURLChecker(
-      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-      const net::NetworkTrafficAnnotationTag& traffic_annotation,
-      size_t cache_size);
-  ~SafeSearchURLChecker();
+  URLChecker(scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+             const net::NetworkTrafficAnnotationTag& traffic_annotation);
+  URLChecker(scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+             const net::NetworkTrafficAnnotationTag& traffic_annotation,
+             size_t cache_size);
+  ~URLChecker();
 
   // Returns whether |callback| was run synchronously.
   bool CheckURL(const GURL& url, CheckCallback callback);
@@ -70,7 +71,9 @@ class SafeSearchURLChecker {
   base::MRUCache<GURL, CheckResult> cache_;
   base::TimeDelta cache_timeout_;
 
-  DISALLOW_COPY_AND_ASSIGN(SafeSearchURLChecker);
+  DISALLOW_COPY_AND_ASSIGN(URLChecker);
 };
 
-#endif  // CHROME_BROWSER_SAFE_SEARCH_API_SAFE_SEARCH_URL_CHECKER_H_
+}  // namespace safe_search_api
+
+#endif  // COMPONENTS_SAFE_SEARCH_API_URL_CHECKER_H_

chrome/browser/safe_search_api/safe_search_url_checker_unittest.cc → components/safe_search_api/url_checker_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/safe_search_api/safe_search_url_checker.h"
+#include "components/safe_search_api/url_checker.h"
 
 #include <stddef.h>
 
@@ -14,6 +14,7 @@
 #include "base/json/json_writer.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
+#include "base/stl_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "net/base/net_errors.h"
@@ -25,22 +26,19 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
-using Classification = SafeSearchURLChecker::Classification;
 using testing::_;
 
+namespace safe_search_api {
+
 namespace {
 
 const size_t kCacheSize = 2;
 
 const char* kURLs[] = {
-    "http://www.randomsite1.com",
-    "http://www.randomsite2.com",
-    "http://www.randomsite3.com",
-    "http://www.randomsite4.com",
-    "http://www.randomsite5.com",
-    "http://www.randomsite6.com",
-    "http://www.randomsite7.com",
-    "http://www.randomsite8.com",
+    "http://www.randomsite1.com", "http://www.randomsite2.com",
+    "http://www.randomsite3.com", "http://www.randomsite4.com",
+    "http://www.randomsite5.com", "http://www.randomsite6.com",
+    "http://www.randomsite7.com", "http://www.randomsite8.com",
     "http://www.randomsite9.com",
 };
 
@@ -49,8 +47,7 @@ const char kSafeSearchApiUrl[] =
 
 std::string BuildResponse(bool is_porn) {
   base::DictionaryValue dict;
-  std::unique_ptr<base::DictionaryValue> classification_dict(
-      new base::DictionaryValue);
+  auto classification_dict = std::make_unique<base::DictionaryValue>();
   if (is_porn)
     classification_dict->SetBoolean("pornography", is_porn);
   auto classifications_list = std::make_unique<base::ListValue>();
@@ -82,7 +79,7 @@ class SafeSearchURLCheckerTest : public testing::Test {
 
  protected:
   GURL GetNewURL() {
-    CHECK(next_url_ < arraysize(kURLs));
+    CHECK(next_url_ < base::size(kURLs));
     return GURL(kURLs[next_url_++]);
   }
 
@@ -124,7 +121,7 @@ class SafeSearchURLCheckerTest : public testing::Test {
   base::MessageLoop message_loop_;
   network::TestURLLoaderFactory test_url_loader_factory_;
   scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
-  SafeSearchURLChecker checker_;
+  URLChecker checker_;
 };
 
 TEST_F(SafeSearchURLCheckerTest, Simple) {
@@ -197,3 +194,5 @@ TEST_F(SafeSearchURLCheckerTest, CacheTimeout) {
   EXPECT_CALL(*this, OnCheckDone(url, Classification::UNSAFE, false));
   ASSERT_FALSE(SendValidResponse(url, true));
 }
+
+}  // namespace safe_search_api