Provide mutable members of UrlRequestContext via pure-virtual interface

chrome/browser/net/chrome_url_request_context.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_url_request_context.h"
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
+#include "base/string_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/io_thread.h"
+#include "chrome/browser/net/basic_http_user_agent_settings.h"
 #include "chrome/browser/net/load_time_stats.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/common/content_client.h"
 #include "net/cookies/cookie_store.h"
 #include "net/http/http_util.h"
 
 using content::BrowserThread;
 
 class ChromeURLRequestContextFactory {
  public:
   ChromeURLRequestContextFactory() {}
   virtual ~ChromeURLRequestContextFactory() {}
 
   // Called to create a new instance (will only be called once).
   virtual ChromeURLRequestContext* Create() = 0;
 
  protected:
   DISALLOW_COPY_AND_ASSIGN(ChromeURLRequestContextFactory);
 };
 
+// Implementation of HttpUserAgentSettings for ChromeURLRequestContext
+// that mirrors changes in Accept-Language and Accept-Charset from
+// content's PrefService.
+class ChromeHttpUserAgentSettings : public net::HttpUserAgentSettings {
+ public:
+  ChromeHttpUserAgentSettings() {}
+  virtual ~ChromeHttpUserAgentSettings() {}
+
+  // net::HttpUserAgentSettings implementation.
+  virtual std::string GetAcceptLanguage() OVERRIDE {
+    return accept_language_;
+  }
+  virtual std::string GetAcceptCharset() OVERRIDE {
+    return accept_charset_;
+  }
+  virtual std::string GetUserAgent(const GURL& url) OVERRIDE {
+    return content::GetUserAgent(url);
+  }
+

[willchan no longer on Chromium, 2012/10/06 19:32:49]

too many newlines

+
+  // Sets the value of 'Accept-Charset' header field.
+  void set_accept_charset(const std::string& accept_charset) {
+    accept_charset_ = accept_charset;
+  }
+
+  // Sets the value of 'Accept-Language' header field.
+  void set_accept_language(const std::string& accept_language) {
+    accept_language_ = accept_language;
+  }
+
+ private:
+  std::string accept_language_;
+  std::string accept_charset_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeHttpUserAgentSettings);
+};
+
 namespace {
 
 // ----------------------------------------------------------------------------
 // Helper factories
 // ----------------------------------------------------------------------------
 
 // Factory that creates the main ChromeURLRequestContext.
 class FactoryForMain : public ChromeURLRequestContextFactory {
  public:
   explicit FactoryForMain(const ProfileIOData* profile_io_data)
@@ skipping 280 matching lines @@
 
 // ----------------------------------------------------------------------------
 // ChromeURLRequestContext
 // ----------------------------------------------------------------------------
 
 ChromeURLRequestContext::ChromeURLRequestContext(
     ContextType type,
     chrome_browser_net::LoadTimeStats* load_time_stats)
     : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
       is_incognito_(false),
-      load_time_stats_(load_time_stats) {
+      load_time_stats_(load_time_stats),
+      chrome_http_user_agent_settings_(new ChromeHttpUserAgentSettings) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (load_time_stats_)
     load_time_stats_->RegisterURLRequestContext(this, type);
+  set_http_user_agent_settings(chrome_http_user_agent_settings_.get());

[willchan no longer on Chromium, 2012/10/06 19:32:49]

Sorry if someone called this out earlier and there's an explanation. I didn't
read comments, just wondering about this here. If I've missed something, please
call out my stupidity.

How does this work exactly? It looks like net::URLRequestContext does not own
HttpUserAgentSettings, which I think is the correct thing to do. However,
ChromeURLRequestContext owns its own ChromeHttpUserAgentSettings, which it sets
|http_user_agent_settings_| to point to in its constructor, this line right
here. Afterward, when CopyFrom() gets called, net::URLRequestContext::CopyFrom()
will overwrite |http_user_agent_settings_| with the source URLRequestContext's
|http_user_agent_settings_|. That means this object's
|http_user_agent_settings_| will no longer point to
|chrome_http_user_agent_settings_|.

Is my understanding correct? If so, I think that's the wrong behavior. It seems
to me that, like we do for almost everything else in ChromeURLRequestContext, we
should have a single ChromeHttpUserAgentSettings instance, which is owned by
ProfileIOData, and have each of the ChromeHttpUserAgentSettings point to that
guy. CopyFrom() will then work correctly.

[erikwright (departed), 2012/10/09 15:24:46]

When you call CopyFrom, you get all of the other context's pointers, which
include a number of complex stateful objects.

This one is not any different, and I think the caller of CopyFrom gets the
behaviour they expect.

I'm actually surprised that anyone calls ChromeURLRequestContext::CopyFrom.. is
that part of a recent refactoring?

In any case, I think it is definitely more correct to have the
ChromeHttpUserAgentSettings owned by the ProfileIOData and for there to be a
single one.

[willchan no longer on Chromium, 2012/10/09 22:58:56]

I think it's been the case for a long time. See profile_impl_io_data.cc for
example.
Great.

 }
 
 ChromeURLRequestContext::~ChromeURLRequestContext() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (load_time_stats_)
     load_time_stats_->UnregisterURLRequestContext(this);
 }
 
 void ChromeURLRequestContext::CopyFrom(ChromeURLRequestContext* other) {
   URLRequestContext::CopyFrom(other);
 
   // Copy ChromeURLRequestContext parameters.
   // ChromeURLDataManagerBackend is unique per context.
   set_is_incognito(other->is_incognito());
 }
 
 ChromeURLDataManagerBackend*
 ChromeURLRequestContext::chrome_url_data_manager_backend() const {
     return chrome_url_data_manager_backend_;
 }
 
 void ChromeURLRequestContext::set_chrome_url_data_manager_backend(
         ChromeURLDataManagerBackend* backend) {
   DCHECK(backend);
   chrome_url_data_manager_backend_ = backend;
 }
 
-const std::string& ChromeURLRequestContext::GetUserAgent(
-    const GURL& url) const {
-  return content::GetUserAgent(url);
-}
-
 void ChromeURLRequestContext::OnAcceptLanguageChange(
     const std::string& accept_language) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  set_accept_language(
+  chrome_http_user_agent_settings_->set_accept_language(
       net::HttpUtil::GenerateAcceptLanguageHeader(accept_language));
 }
 
 void ChromeURLRequestContext::OnDefaultCharsetChange(
     const std::string& default_charset) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   set_referrer_charset(default_charset);
-  set_accept_charset(
+  chrome_http_user_agent_settings_->set_accept_charset(
       net::HttpUtil::GenerateAcceptCharsetHeader(default_charset));
 }