Revert "Apply Google-related request modifications for networking ser…

…vice path." This reverts commit b8dd986. Reason for revert: Findit (https://goo.gl/kROfz5) identified CL at revision 579953 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2I4ZGQ5ODY0MjE3M2YyNmUyMDMzOGU5ZGE5ZWI3NGQzMTEyNjE5OGMM Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20CFI/9356 Sample Failed Step: not_site_per_process_unit_tests Original change's description: > Apply Google-related request modifications for networking service path. > > This adds support for: > 1) Safe Search > 2) YouTube restricted mode > 3) restricting consumer accounts > through Group Policy. > > The other supporting change is to make ThrottlingURLLoader handle a throttle changing the URL in WillStartRequest. > > Bug: 841313 > Cq-Include-Trybots: luci.chromium.try:linux_mojo > Change-Id: Ic98e14dd5fa7b5b00d4befd3314aca7a67e6ba5a > Reviewed-on: https://chromium-review.googlesource.com/1152507 > Reviewed-by: Matt Menke <mmenke@chromium.org> > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> > Commit-Queue: John Abd-El-Malek <jam@chromium.org> > Cr-Commit-Position: refs/heads/master@{#579953} Change-Id: Ic1b90a4148f1d5ebb4442ce34fa5421d879ef32f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 841313 Cq-Include-Trybots: luci.chromium.try:linux_mojo Reviewed-on: https://chromium-review.googlesource.com/1159801 Cr-Commit-Position: refs/heads/master@{#580025}
qweri0p · Aug 2, 2018 · 4ec9869 · 4ec9869
1 parent 032102f
commit 4ec9869
Show file tree

Hide file tree

Showing 43 changed files with 582 additions and 667 deletions.
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
@@ -842,6 +842,8 @@ jumbo_split_static_library("browser") {
     "net/referrer.h",
     "net/reporting_permissions_checker.cc",
     "net/reporting_permissions_checker.h",
+    "net/safe_search_util.cc",
+    "net/safe_search_util.h",
     "net/service_providers_win.cc",
     "net/service_providers_win.h",
     "net/spdyproxy/data_reduction_proxy_chrome_io_data.cc",
@@ -1730,6 +1732,7 @@ jumbo_split_static_library("browser") {
     "//components/filename_generation",
     "//components/flags_ui",
     "//components/gcm_driver",
+    "//components/google/core/browser",
     "//components/handoff",
     "//components/history/content/browser",
     "//components/history/core/browser",

diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
@@ -146,7 +146,6 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/constants.mojom.h"
 #include "chrome/common/env_vars.h"
-#include "chrome/common/google_url_loader_throttle.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/pepper_permission_util.h"
 #include "chrome/common/pref_names.h"
@@ -157,6 +156,7 @@
 #include "chrome/common/secure_origin_whitelist.h"
 #include "chrome/common/stack_sampling_configuration.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/common/variations_header_url_loader_throttle.h"
 #include "chrome/grit/browser_resources.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/installer/util/google_update_settings.h"
@@ -4192,20 +4192,16 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
           base::BindOnce(GetPrerenderCanceller, wc_getter),
           BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)));
     }
-  }
 
-  ProfileIOData* io_data = ProfileIOData::FromResourceContext(resource_context);
-  if (io_data) {  // null in some tests.
+    ProfileIOData* io_data =
+        ProfileIOData::FromResourceContext(resource_context);
     bool is_off_the_record = io_data->IsOffTheRecord();
     bool is_signed_in =
         !is_off_the_record &&
         !io_data->google_services_account_id()->GetValue().empty();
 
-    result.push_back(std::make_unique<GoogleURLLoaderThrottle>(
-        is_off_the_record, is_signed_in,
-        io_data->force_google_safesearch()->GetValue(),
-        io_data->force_youtube_restrict()->GetValue(),
-        io_data->allowed_domains_for_apps()->GetValue()));
+    result.push_back(std::make_unique<VariationsHeaderURLLoaderThrottle>(
+        is_off_the_record, is_signed_in));
   }
 
 #if BUILDFLAG(ENABLE_PLUGINS)

diff --git a/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc b/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
@@ -229,6 +229,8 @@ class ExtensionWebRequestTest : public testing::Test {
  protected:
   void SetUp() override {
     ASSERT_TRUE(profile_manager_.SetUp());
+    ChromeNetworkDelegate::InitializePrefsOnUIThread(
+        nullptr, nullptr, nullptr, profile_.GetTestingPrefService());
     network_delegate_.reset(new ChromeNetworkDelegate(event_router_.get()));
     network_delegate_->set_profile(&profile_);
     network_delegate_->set_cookie_settings(
@@ -1120,6 +1122,8 @@ class ExtensionWebRequestHeaderModificationTest
  protected:
   void SetUp() override {
     ASSERT_TRUE(profile_manager_.SetUp());
+    ChromeNetworkDelegate::InitializePrefsOnUIThread(
+        nullptr, nullptr, nullptr, profile_.GetTestingPrefService());
     network_delegate_.reset(new ChromeNetworkDelegate(event_router_.get()));
     network_delegate_->set_profile(&profile_);
     network_delegate_->set_cookie_settings(

diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
@@ -279,6 +279,11 @@ IOThread::IOThread(
       weak_factory_(this) {
   scoped_refptr<base::SingleThreadTaskRunner> io_thread_proxy =
       BrowserThread::GetTaskRunnerForThread(BrowserThread::IO);
+  ChromeNetworkDelegate::InitializePrefsOnUIThread(
+      nullptr,
+      nullptr,
+      nullptr,
+      local_state);
 
   BrowserThread::SetIOThreadDelegate(this);
 

diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc
@@ -33,6 +33,8 @@
 #include "chrome/common/pref_names.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/domain_reliability/monitor.h"
+#include "components/prefs/pref_member.h"
+#include "components/prefs/pref_service.h"
 #include "components/variations/net/variations_http_headers.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
@@ -76,6 +78,19 @@ namespace {
 
 bool g_access_to_all_files_enabled = false;
 
+// Gets called when the extensions finish work on the URL. If the extensions
+// did not do a redirect (so |new_url| is empty) then we enforce the
+// SafeSearch parameters. Otherwise we will get called again after the
+// redirect and we enforce SafeSearch then.
+void ForceGoogleSafeSearchCallbackWrapper(net::CompletionOnceCallback callback,
+                                          net::URLRequest* request,
+                                          GURL* new_url,
+                                          int rv) {
+  if (rv == net::OK && new_url->is_empty())
+    safe_search_util::ForceGoogleSafeSearch(request, new_url);
+  std::move(callback).Run(rv);
+}
+
 bool IsAccessAllowedInternal(const base::FilePath& path,
                              const base::FilePath& profile_path) {
 #if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
@@ -152,6 +167,9 @@ ChromeNetworkDelegate::ChromeNetworkDelegate(
     : extensions_delegate_(
           ChromeExtensionsNetworkDelegate::Create(event_router)),
       profile_(nullptr),
+      force_google_safe_search_(nullptr),
+      force_youtube_restrict_(nullptr),
+      allowed_domains_for_apps_(nullptr),
       experimental_web_platform_features_enabled_(
           base::CommandLine::ForCurrentProcess()->HasSwitch(
               switches::kEnableExperimentalWebPlatformFeatures)) {}
@@ -173,19 +191,78 @@ void ChromeNetworkDelegate::set_cookie_settings(
   cookie_settings_ = cookie_settings;
 }
 
+// static
+void ChromeNetworkDelegate::InitializePrefsOnUIThread(
+    BooleanPrefMember* force_google_safe_search,
+    IntegerPrefMember* force_youtube_restrict,
+    StringPrefMember* allowed_domains_for_apps,
+    PrefService* pref_service) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (force_google_safe_search) {
+    force_google_safe_search->Init(prefs::kForceGoogleSafeSearch, pref_service);
+    force_google_safe_search->MoveToThread(
+        BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
+  }
+  if (force_youtube_restrict) {
+    force_youtube_restrict->Init(prefs::kForceYouTubeRestrict, pref_service);
+    force_youtube_restrict->MoveToThread(
+        BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
+  }
+  if (allowed_domains_for_apps) {
+    allowed_domains_for_apps->Init(prefs::kAllowedDomainsForApps, pref_service);
+    allowed_domains_for_apps->MoveToThread(
+        BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
+  }
+}
+
 int ChromeNetworkDelegate::OnBeforeURLRequest(
     net::URLRequest* request,
     net::CompletionOnceCallback callback,
     GURL* new_url) {
   extensions_delegate_->ForwardStartRequestStatus(request);
-  return extensions_delegate_->NotifyBeforeURLRequest(
-      request, std::move(callback), new_url);
+
+  bool force_safe_search =
+      (force_google_safe_search_ && force_google_safe_search_->GetValue());
+
+  net::CompletionOnceCallback wrapped_callback = std::move(callback);
+  if (force_safe_search) {
+    wrapped_callback = base::BindOnce(
+        &ForceGoogleSafeSearchCallbackWrapper, std::move(wrapped_callback),
+        base::Unretained(request), base::Unretained(new_url));
+  }
+
+  int rv = extensions_delegate_->NotifyBeforeURLRequest(
+      request, std::move(wrapped_callback), new_url);
+
+  if (force_safe_search && rv == net::OK && new_url->is_empty())
+    safe_search_util::ForceGoogleSafeSearch(request, new_url);
+
+  if (allowed_domains_for_apps_ &&
+      !allowed_domains_for_apps_->GetValue().empty() &&
+      request->url().DomainIs("google.com")) {
+    request->SetExtraRequestHeaderByName("X-GoogApps-Allowed-Domains",
+                                         allowed_domains_for_apps_->GetValue(),
+                                         true);
+  }
+
+  return rv;
 }
 
 int ChromeNetworkDelegate::OnBeforeStartTransaction(
     net::URLRequest* request,
     net::CompletionOnceCallback callback,
     net::HttpRequestHeaders* headers) {
+  if (force_youtube_restrict_) {
+    int value = force_youtube_restrict_->GetValue();
+    static_assert(safe_search_util::YOUTUBE_RESTRICT_OFF == 0,
+                  "OFF must be first");
+    if (value > safe_search_util::YOUTUBE_RESTRICT_OFF &&
+        value < safe_search_util::YOUTUBE_RESTRICT_COUNT) {
+      safe_search_util::ForceYouTubeRestrict(request, headers,
+          static_cast<safe_search_util::YouTubeRestrictMode>(value));
+    }
+  }
+
   return extensions_delegate_->NotifyBeforeStartTransaction(
       request, std::move(callback), headers);
 }

diff --git a/chrome/browser/net/chrome_network_delegate.h b/chrome/browser/net/chrome_network_delegate.h
@@ -18,10 +18,17 @@
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/net/reporting_permissions_checker.h"
+#include "chrome/browser/net/safe_search_util.h"
 #include "components/domain_reliability/monitor.h"
+#include "components/prefs/pref_member.h"
 #include "net/base/network_delegate_impl.h"
 
 class ChromeExtensionsNetworkDelegate;
+class PrefService;
+
+template<class T> class PrefMember;
+
+typedef PrefMember<bool> BooleanPrefMember;
 
 namespace content_settings {
 class CookieSettings;
@@ -44,6 +51,9 @@ class URLRequest;
 // add hooks into the network stack.
 class ChromeNetworkDelegate : public net::NetworkDelegateImpl {
  public:
+  // All optional PrefMembers should be initialized on the UI thread (see below)
+  // before first use. This object's owner is responsible for cleaning them up
+  // at shutdown.
   explicit ChromeNetworkDelegate(
       extensions::EventRouterForwarder* event_router);
   ~ChromeNetworkDelegate() override;
@@ -69,6 +79,21 @@ class ChromeNetworkDelegate : public net::NetworkDelegateImpl {
   // the header file. Here we just forward-declare it.
   void set_cookie_settings(content_settings::CookieSettings* cookie_settings);
 
+  void set_force_google_safe_search(
+      BooleanPrefMember* force_google_safe_search) {
+    force_google_safe_search_ = force_google_safe_search;
+  }
+
+  void set_force_youtube_restrict(
+      IntegerPrefMember* force_youtube_restrict) {
+    force_youtube_restrict_ = force_youtube_restrict;
+  }
+
+  void set_allowed_domains_for_apps(
+      StringPrefMember* allowed_domains_for_apps) {
+    allowed_domains_for_apps_ = allowed_domains_for_apps;
+  }
+
   void set_domain_reliability_monitor(
       std::unique_ptr<domain_reliability::DomainReliabilityMonitor> monitor) {
     domain_reliability_monitor_ = std::move(monitor);
@@ -88,6 +113,15 @@ class ChromeNetworkDelegate : public net::NetworkDelegateImpl {
     return reporting_permissions_checker_.get();
   }
 
+  // Binds the pref members to |pref_service| and moves them to the IO thread.
+  // All arguments can be nullptr. This method should be called on the UI
+  // thread.
+  static void InitializePrefsOnUIThread(
+      BooleanPrefMember* force_google_safe_search,
+      IntegerPrefMember* force_youtube_restrict,
+      StringPrefMember* allowed_domains_for_apps,
+      PrefService* pref_service);
+
   // Returns true if access to |path| is allowed. |profile_path| is used to
   // locate certain paths on Chrome OS. See set_profile_path() for details.
   static bool IsAccessAllowed(const base::FilePath& path,
@@ -167,6 +201,11 @@ class ChromeNetworkDelegate : public net::NetworkDelegateImpl {
   base::FilePath profile_path_;
   scoped_refptr<content_settings::CookieSettings> cookie_settings_;
 
+  // Weak, owned by our owner.
+  BooleanPrefMember* force_google_safe_search_;
+  IntegerPrefMember* force_youtube_restrict_;
+  StringPrefMember* allowed_domains_for_apps_;
+
   // Weak, owned by our owner.
   std::unique_ptr<domain_reliability::DomainReliabilityMonitor>
       domain_reliability_monitor_;