Add a SafeSearch preference, policy and implementation.

chrome/browser/net/chrome_network_delegate.cc

Index: chrome/browser/net/chrome_network_delegate.cc
diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc
index 828fe6665fb9348faaa06f9ad9780ee62a6c5ed2..f7916571b09f67903a00a54d317e5a737f52ef09 100644
--- a/chrome/browser/net/chrome_network_delegate.cc
+++ b/chrome/browser/net/chrome_network_delegate.cc
@@ -4,9 +4,12 @@
 
 #include "chrome/browser/net/chrome_network_delegate.h"
 
+#include <vector>
+
 #include "base/logging.h"
 #include "base/base_paths.h"
 #include "base/path_service.h"
+#include "base/string_util.h"
 #include "chrome/browser/api/prefs/pref_member.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
@@ -17,6 +20,7 @@
 #include "chrome/browser/extensions/event_router_forwarder.h"
 #include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
+#include "chrome/browser/google/google_util.h"
 #include "chrome/browser/net/load_time_stats.h"
 #include "chrome/browser/performance_monitor/performance_monitor.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -87,6 +91,77 @@ void ForwardProxyErrors(net::URLRequest* request,
   }
 }
 
+void ForceGoogleSafeSearchCallbackWrapper(

[battre, 2012/10/19 09:02:35]

Please add a comment describing the purpose of this function and why it works in
case new_url is not empty.

[Sergiu, 2012/10/19 12:00:18]

Done.

+    const net::CompletionCallback& callback,
+    net::URLRequest* request,
+    GURL* new_url,
+    int rv) {
+  if (rv == net::OK && new_url->is_empty())

[Joao da Silva, 2012/10/19 10:00:38]

If the extension rewrites the URL but it still lands on google search then
SafeSearch will not be enforced, right? Is that the intended action?

[Sergiu, 2012/10/19 12:00:18]

As far as I have understood if the extension rewrites the URL a redirect is
triggered and the OnBeforeURLRequest is triggered again after that, and we
should enforce it then. @battre, can you confirm this?

+    ChromeNetworkDelegate::ForceGoogleSafeSearch(request, new_url);
+  callback.Run(rv);
+}
+
+enum SafeSearchCheckState {
+  KEEP_SEARCHING = 0,
+  ADD_PARAMETER,
+  PARAMETER_PRESENT,
+};
+
+// Checks a parameter to see if it is present in the query.

[battre, 2012/10/19 09:02:35]

Please describe the parameters.

[Sergiu, 2012/10/19 12:00:18]

Done.

+SafeSearchCheckState CheckForParameter(const std::string& parameter,
+                                       const std::string& parameter_to_find,
+                                       bool* keep_element) {
+  DCHECK(parameter_to_find.find("=") != std::string::npos);
+  // prefix for "safe=off" is "safe=".
+  std::string parameter_prefix = parameter_to_find.substr(
+      0, parameter_to_find.find("=") + 1);
+  if (parameter.length() >= parameter_prefix.length()) {
+    if (parameter == parameter_to_find)
+      return PARAMETER_PRESENT;
+    // Found the parameter but not the correct value, remove it.
+    if (parameter.substr(0, parameter_prefix.length()) == parameter_prefix) {
+      *keep_element = false;
+      return ADD_PARAMETER;
+    }
+  }
+  return KEEP_SEARCHING;
+}
+
+void ExamineQuery(std::string query, std::string* new_query) {

[battre, 2012/10/19 09:02:35]

This function name is not very helpful. Please add a comment what the function
does. We can then decide whether we need a more descriptive function name.

[battre, 2012/10/19 09:02:35]

Strings should be passed as const references.

[Sergiu, 2012/10/19 12:00:18]

Done.

[Sergiu, 2012/10/19 12:00:18]

Done.

+  std::vector<std::string> parameters;

[battre, 2012/10/19 09:02:35]

Please declare variables where they are used first (does not work for all
variables obviously).

[Sergiu, 2012/10/19 12:00:18]

Done.

+  std::vector<std::string> new_parameters;
+  std::vector<std::string>::reverse_iterator rit;
+  SafeSearchCheckState safe_check = KEEP_SEARCHING;
+  SafeSearchCheckState ssui_check = KEEP_SEARCHING;
+  std::string safe_parameter = chrome::kSafeSearchSafeParameter;
+  std::string ssui_parameter = chrome::kSafeSearchSsuiParameter;
+
+  Tokenize(query, std::string("&"), &parameters);

[battre, 2012/10/19 09:02:35]

I am not sure whether this approach is sufficient. It probably does not work
with %xx encoded characters.

[Sergiu, 2012/10/19 12:00:18]

Added a percent-encoded unit test and it seems to work, don't know if I am
missing something though, shouldn't things be percent-encoded when we get them
in the request?

+
+  for (rit = parameters.rbegin(); rit < parameters.rend(); ++rit) {
+    bool keep_current_parameter = true;
+    if (safe_check == KEEP_SEARCHING) {
+      safe_check = CheckForParameter(*rit, chrome::kSafeSearchSafeParameter,
+                                     &keep_current_parameter);
+    }
+    if (ssui_check == KEEP_SEARCHING) {
+      ssui_check = CheckForParameter(*rit, chrome::kSafeSearchSsuiParameter,
+                                     &keep_current_parameter);
+    }
+    if (keep_current_parameter) {
+      // Iterating in reverse so make sure that we add it in the correct
+      // position.
+      new_parameters.insert(new_parameters.begin(), *rit);
+    }
+  }
+
+  if (safe_check != PARAMETER_PRESENT)
+      new_parameters.insert(new_parameters.end(), safe_parameter);

[battre, 2012/10/19 09:02:35]

nit: -2 spaces, also below

[Sergiu, 2012/10/19 12:00:18]

Done.

+  if (ssui_check != PARAMETER_PRESENT)
+      new_parameters.insert(new_parameters.end(), ssui_parameter);
+  *new_query = JoinString(new_parameters, std::string("&"));
+}
+
 enum RequestStatus { REQUEST_STARTED, REQUEST_DONE };
 
 // Notifies the ExtensionProcessManager that a request has started or stopped
@@ -146,6 +221,7 @@ ChromeNetworkDelegate::ChromeNetworkDelegate(
     CookieSettings* cookie_settings,
     BooleanPrefMember* enable_referrers,
     BooleanPrefMember* enable_do_not_track,
+    BooleanPrefMember* force_google_safesearch,
     chrome_browser_net::LoadTimeStats* load_time_stats)
     : event_router_(event_router),
       profile_(profile),
@@ -153,6 +229,7 @@ ChromeNetworkDelegate::ChromeNetworkDelegate(
       extension_info_map_(extension_info_map),
       enable_referrers_(enable_referrers),
       enable_do_not_track_(enable_do_not_track),
+      force_google_safesearch_(force_google_safesearch),
       url_blacklist_manager_(url_blacklist_manager),
       managed_mode_url_filter_(managed_mode_url_filter),
       load_time_stats_(load_time_stats) {
@@ -172,6 +249,7 @@ void ChromeNetworkDelegate::NeverThrottleRequests() {
 void ChromeNetworkDelegate::InitializePrefsOnUIThread(
     BooleanPrefMember* enable_referrers,
     BooleanPrefMember* enable_do_not_track,
+    BooleanPrefMember* force_google_safesearch,
     PrefService* pref_service) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   enable_referrers->Init(prefs::kEnableReferrers, pref_service, NULL);
@@ -180,6 +258,10 @@ void ChromeNetworkDelegate::InitializePrefsOnUIThread(
     enable_do_not_track->Init(prefs::kEnableDoNotTrack, pref_service, NULL);
     enable_do_not_track->MoveToThread(BrowserThread::IO);
   }
+  if (force_google_safesearch) {
+    force_google_safesearch->Init(prefs::kForceSafeSearch, pref_service, NULL);
+    force_google_safesearch->MoveToThread(BrowserThread::IO);
+  }
 }
 
 // static
@@ -187,6 +269,28 @@ void ChromeNetworkDelegate::AllowAccessToAllFiles() {
   g_allow_file_access_ = true;
 }
 
+// static
+void ChromeNetworkDelegate::ForceGoogleSafeSearch(net::URLRequest* request,
+                                                  GURL* new_url) {
+  // Force SafeSearch.
+  const bool is_search_url = google_util::IsGoogleSearchUrl(
+      request->url().spec());
+  const bool is_homepage_url = google_util::IsGoogleHomePageUrl(
+      request->url().spec());
+  if (is_search_url || is_homepage_url) {
+    std::string query = request->url().query();
+    std::string new_query;
+
+    ExamineQuery(query, &new_query);
+    if (query == new_query)
+      return;
+
+    GURL::Replacements replacements;
+    replacements.SetQueryStr(new_query);
+    *new_url = request->url().ReplaceComponents(replacements);
+  }
+}
+
 int ChromeNetworkDelegate::OnBeforeURLRequest(
     net::URLRequest* request,
     const net::CompletionCallback& callback,
@@ -220,6 +324,26 @@ int ChromeNetworkDelegate::OnBeforeURLRequest(
     request->set_referrer(std::string());
   if (enable_do_not_track_ && enable_do_not_track_->GetValue())
     request->SetExtraRequestHeaderByName(kDNTHeader, "1", true /* override */);
+
+  if (force_google_safesearch_ && force_google_safesearch_->GetValue()) {
+    net::CompletionCallback wrapped_callback =
+      base::Bind(&ForceGoogleSafeSearchCallbackWrapper,

[battre, 2012/10/19 09:02:35]

nit: +2 spaces

[Sergiu, 2012/10/19 12:00:18]

Done.

+                 callback,
+                 base::Unretained(request),
+                 base::Unretained(new_url));
+
+    int rv = ExtensionWebRequestEventRouter::GetInstance()->OnBeforeRequest(
+        profile_, extension_info_map_.get(), request, wrapped_callback,
+        new_url);
+
+    if (rv == net::OK && new_url->is_empty()) {
+      ForceGoogleSafeSearch(request, new_url);
+      return net::OK;
+    }
+
+    return rv;
+  }

[Joao da Silva, 2012/10/19 10:00:38]

There are now two paths exiting this function from here, and future code will
have to handle both. How about this:

bool force_safesearch = force_google_safesearch_ &&
                        force_google_safesearch_->GetValue();
if (force_safesearch) {
  // Wrap |callback|.
  callback = base::Bind(&ForceGoogleSafeSearchCallbackWrapper,
                        callback,
                        base::Unretained(request),
                        base::Unretained(new_url));
}

int rv = ExtensionWebRequestEventRouter:: ... callback, ...;

if (force_safesearch && rv == net::OK && new_url->is_empty())
  ForceGoogleSafeSearch(request, new_url);

return rv;

[Sergiu, 2012/10/19 12:00:18]

Rewritten, but using wrapped_callback since callback is const.

+
   return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeRequest(
       profile_, extension_info_map_.get(), request, callback, new_url);
 }