Updated Client-Hints patch

chrome/browser/net/chrome_network_delegate.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/net/chrome_network_delegate.h"
 
 #include <stdlib.h>
 
 #include <vector>
 
@@ skipping 10 matching lines @@
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/browser/extensions/api/proxy/proxy_api.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api.h"
 #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/extensions/extension_system.h"
 #include "chrome/browser/google/google_util.h"
+#include "chrome/browser/net/client_hints.h"
 #include "chrome/browser/net/connect_interceptor.h"
 #include "chrome/browser/net/load_time_stats.h"
 #include "chrome/browser/performance_monitor/performance_monitor.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_view_host.h"
@@ skipping 31 matching lines @@
 bool ChromeNetworkDelegate::g_allow_file_access_ = true;
 #endif
 
 // This remains false unless the --disable-extensions-http-throttling
 // flag is passed to the browser.
 bool ChromeNetworkDelegate::g_never_throttle_requests_ = false;
 
 namespace {
 
 const char kDNTHeader[] = "DNT";
+const char kClientHintsHeader[] = "CH";
 
 // If the |request| failed due to problems with a proxy, forward the error to
 // the proxy extension API.
 void ForwardProxyErrors(net::URLRequest* request,
                         extensions::EventRouterForwarder* event_router,
                         void* profile) {
   if (request->status().status() == net::URLRequestStatus::FAILED) {
     switch (request->status().error()) {
       case net::ERR_PROXY_AUTH_UNSUPPORTED:
       case net::ERR_PROXY_CONNECTION_FAILED:
@@ skipping 431 matching lines @@
     CookieSettings* cookie_settings) {
   cookie_settings_ = cookie_settings;
 }
 
 void ChromeNetworkDelegate::set_predictor(
     chrome_browser_net::Predictor* predictor) {
   connect_interceptor_.reset(
       new chrome_browser_net::ConnectInterceptor(predictor));
 }
 
+void ChromeNetworkDelegate::set_enable_client_hints() {
+  client_hints_.reset(new ClientHints());
+  client_hints_->Init();
+}
+
 // static
 void ChromeNetworkDelegate::NeverThrottleRequests() {
   g_never_throttle_requests_ = true;
 }
 
 // static
 void ChromeNetworkDelegate::InitializePrefsOnUIThread(
     BooleanPrefMember* enable_referrers,
     BooleanPrefMember* enable_do_not_track,
     BooleanPrefMember* force_google_safe_search,
@@ skipping 64 matching lines @@
   }
 #endif
 
   ForwardRequestStatus(REQUEST_STARTED, request, profile_);
 
   if (!enable_referrers_->GetValue())
     request->SetReferrer(std::string());
   if (enable_do_not_track_ && enable_do_not_track_->GetValue())
     request->SetExtraRequestHeaderByName(kDNTHeader, "1", true /* override */);
 
+  if (client_hints_ != NULL) {

[mmenke, 2013/09/09 16:55:30]

This is done a lot, but I believe "if (client_hints_) {" is the preferred style
for scoped_ptrs, now that it's supported.

+    std::string client_hints_value = client_hints_->GetScreenInfoHints();

[mmenke, 2013/09/09 16:55:30]

Suggest just inlining this call below.  Not a huge performance issue, but it
removes a string copy.

[mmenke, 2013/09/09 16:55:30]

I think this function name (GetScreenInfoHints()) is a little odd.  Seems like
client_hints_ should handle all the logic of what hints we want and don't want
itself, and the calling code shouldn't know or care if it includes screen info,
or what.

I'd suggest just calling it GetClientHintsString, or even just GetString, since
we already know it's a ClientHints object we're calling.

Another option would be to make the ClientHints object do all the work, and have
it work like client_hints_->AddHintsToRequest(request_), and then have the
ClientHints object do all the work.  Other reviewers may not like this idea.

[bengr, 2013/09/11 16:32:36]

I like GetString(), or even ToString().

+    request->SetExtraRequestHeaderByName(kClientHintsHeader,
+                                         client_hints_value, true);
+  }
+
   bool force_safe_search = force_google_safe_search_ &&
                            force_google_safe_search_->GetValue();
 
   net::CompletionCallback wrapped_callback = callback;
   if (force_safe_search) {
     wrapped_callback = base::Bind(&ForceGoogleSafeSearchCallbackWrapper,
                                   callback,
                                   base::Unretained(request),
                                   base::Unretained(new_url));
   }
@@ skipping 307 matching lines @@
     int64 received_content_length, int64 original_content_length,
     bool data_reduction_proxy_was_used) {
   DCHECK_GE(received_content_length, 0);
   DCHECK_GE(original_content_length, 0);
   StoreAccumulatedContentLength(received_content_length,
                                 original_content_length,
                                 data_reduction_proxy_was_used);
   received_content_length_ += received_content_length;
   original_content_length_ += original_content_length;
 }