Provide mutable members of UrlRequestContext via pure-virtual interface

chrome/browser/safe_browsing/safe_browsing_service.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/safe_browsing/safe_browsing_service.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/debug/leak_tracker.h"
 #include "base/lazy_instance.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "chrome/browser/api/prefs/pref_change_registrar.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/net/basic_http_user_agent_settings.h"
 #include "chrome/browser/net/sqlite_persistent_cookie_store.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/safe_browsing/client_side_detection_service.h"
 #include "chrome/browser/safe_browsing/download_protection_service.h"
 #include "chrome/browser/safe_browsing/malware_details.h"
 #include "chrome/browser/safe_browsing/protocol_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_blocking_page.h"
 #include "chrome/browser/safe_browsing/safe_browsing_database.h"
@@ skipping 62 matching lines @@
   DCHECK(result);
   return path.Append(chrome::kSafeBrowsingBaseFilename);
 }
 
 FilePath CookieFilePath() {
   return FilePath(BaseFilename().value() + kCookiesFile);
 }
 
 }  // namespace
 
-// Custom URLRequestContext used by SafeBrowsing requests, which are not
-// associated with a particular profile. We need to use a subclass of
-// URLRequestContext in order to provide the correct User-Agent.
-class SafeBrowsingURLRequestContext : public net::URLRequestContext {
- public:
-  virtual const std::string& GetUserAgent(
-      const GURL& url) const OVERRIDE {
-    return content::GetUserAgent(url);
-  }
-
- private:
-  virtual ~SafeBrowsingURLRequestContext() {}
-
-  base::debug::LeakTracker<SafeBrowsingURLRequestContext> leak_tracker_;
-};
-
 class SafeBrowsingURLRequestContextGetter
     : public net::URLRequestContextGetter {
  public:
   explicit SafeBrowsingURLRequestContextGetter(
       SafeBrowsingService* sb_service_);
 
   // Implementation for net::UrlRequestContextGetter.
   virtual net::URLRequestContext* GetURLRequestContext() OVERRIDE;
   virtual scoped_refptr<base::SingleThreadTaskRunner>
       GetNetworkTaskRunner() const OVERRIDE;
@@ skipping 486 matching lines @@
 
 void SafeBrowsingService::InitURLRequestContextOnIOThread(
     net::URLRequestContextGetter* system_url_request_context_getter) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(!url_request_context_.get());
 
   scoped_refptr<net::CookieStore> cookie_store = new net::CookieMonster(
       new SQLitePersistentCookieStore(CookieFilePath(), false, NULL),
       NULL);
 
-  url_request_context_.reset(new SafeBrowsingURLRequestContext);
+  url_request_context_.reset(new net::URLRequestContext);
+  http_user_agent_settings_.reset(
+      new BasicHttpUserAgentSettings(EmptyString(), EmptyString()));
+  url_request_context_->set_http_user_agent_settings(
+      http_user_agent_settings_.get());
   // |system_url_request_context_getter| may be NULL during tests.
   if (system_url_request_context_getter)
     url_request_context_->CopyFrom(

[erikwright (departed), 2012/09/25 21:06:38]

Is this copying the HttpUserAgentSettings from the system context? If so (I
think it probably should) the above set_http_user_agent_settings is meaningless.

It should either be moved after line 618 or even removed (since it appears the
legacy behaviour would have been to _freeze_ the accept_charset/language values,
not to unset them, and it's probably OK if, instead of freezing, they continue
mirroring).

[pauljensen, 2012/09/26 14:08:19]

I was inserting a BasicHttpUserAgentSettings to provide the GetUserAgent()
implementation when !system_url_request_context_getter.  Now that I think about
it, if system_url_request_context_getter is only NULL for tests and I'd guess
tests don't need a proper User-Agent, then I can just get rid of this.

[erikwright (departed), 2012/09/26 14:19:36]

I see. Good catch, actually.
Yes, assuming that this doesn't cause any tests to fail, I agree we don't need
it.

         system_url_request_context_getter->GetURLRequestContext());
   url_request_context_->set_cookie_store(cookie_store);
 }
 
 void SafeBrowsingService::DestroyURLRequestContextOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   url_request_context_->AssertNoURLRequests();
 
   // Need to do the CheckForLeaks on IOThread instead of in ShutDown where
@@ skipping 805 matching lines @@
     Stop();
 
   if (csd_service_.get())
     csd_service_->SetEnabledAndRefreshState(enable);
   if (download_service_.get()) {
     download_service_->SetEnabled(
         enable && !CommandLine::ForCurrentProcess()->HasSwitch(
             switches::kDisableImprovedDownloadProtection));
   }
 }