Add SafeBrowsingURLRequestContext which stores cookies.

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/browser_process.h"
 #include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/net/sqlite_persistent_cookie_store.h"
 #include "chrome/browser/prefs/pref_change_registrar.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"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_client.h"
+#include "net/base/cookie_monster.h"
 #include "net/base/registry_controlled_domain.h"
+#include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 
 #if defined(OS_WIN)
 #include "chrome/installer/util/browser_distribution.h"
 #endif
 
 using content::BrowserThread;
 using content::NavigationEntry;
 using content::WebContents;
 
 namespace {
 
+// Filename suffix for the cookie database.
+const FilePath::CharType kCookiesFile[] = FILE_PATH_LITERAL(" Cookies");
+
 // The default URL prefix where browser fetches chunk updates, hashes,
 // and reports safe browsing hits.
 const char* const kSbDefaultInfoURLPrefix =
     "http://safebrowsing.clients.google.com/safebrowsing";
 
 // The default URL prefix where browser fetches MAC client key and reports
 // malware details.
 const char* const kSbDefaultMacKeyURLPrefix =
     "https://sb-ssl.google.com/safebrowsing";
 
@@ skipping 14 matching lines @@
   if (full_hashes.empty()) {
     result = SafeBrowsingProtocolManager::GET_HASH_FULL_HASH_EMPTY;
   } else if (hit) {
     result = SafeBrowsingProtocolManager::GET_HASH_FULL_HASH_HIT;
   } else {
     result = SafeBrowsingProtocolManager::GET_HASH_FULL_HASH_MISS;
   }
   SafeBrowsingProtocolManager::RecordGetHashResult(is_download, result);
 }
 
+FilePath BaseFilename() {
+  FilePath path;
+  bool result = PathService::Get(chrome::DIR_USER_DATA, &path);
+  DCHECK(result);
+  return path.Append(chrome::kSafeBrowsingBaseFilename);
+}
+
 }  // 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:
+  base::debug::LeakTracker<SafeBrowsingURLRequestContext> leak_tracker_;
+};
+
+class SafeBrowsingURLRequestContextGetter
+    : public net::URLRequestContextGetter {
+ public:
+  explicit SafeBrowsingURLRequestContextGetter(
+      SafeBrowsingService* sb_service_);
+  virtual ~SafeBrowsingURLRequestContextGetter();
+
+  // Implementation for net::UrlRequestContextGetter.
+  virtual net::URLRequestContext* GetURLRequestContext() OVERRIDE;
+  virtual scoped_refptr<base::MessageLoopProxy> GetIOMessageLoopProxy() const
+      OVERRIDE;
+
+ private:
+  SafeBrowsingService* const sb_service_;  // Owned by BrowserProcess.
+  scoped_refptr<base::MessageLoopProxy> io_message_loop_proxy_;
+
+  base::debug::LeakTracker<SafeBrowsingURLRequestContextGetter> leak_tracker_;
+};
+
+SafeBrowsingURLRequestContextGetter::SafeBrowsingURLRequestContextGetter(
+    SafeBrowsingService* sb_service)
+    : sb_service_(sb_service),
+      io_message_loop_proxy_(
+          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)) {
+}
+
+SafeBrowsingURLRequestContextGetter::~SafeBrowsingURLRequestContextGetter() {}
+
+net::URLRequestContext*
+SafeBrowsingURLRequestContextGetter::GetURLRequestContext() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(sb_service_->url_request_context_);
+
+  return sb_service_->url_request_context_;
+}
+
+scoped_refptr<base::MessageLoopProxy>
+SafeBrowsingURLRequestContextGetter::GetIOMessageLoopProxy() const {
+  return io_message_loop_proxy_;
+}
+
 // static
 SafeBrowsingServiceFactory* SafeBrowsingService::factory_ = NULL;
 
 // The default SafeBrowsingServiceFactory.  Global, made a singleton so we
 // don't leak it.
 class SafeBrowsingServiceFactoryImpl : public SafeBrowsingServiceFactory {
  public:
   virtual SafeBrowsingService* CreateSafeBrowsingService() {
     return new SafeBrowsingService();
   }
@@ skipping 68 matching lines @@
       protocol_manager_(NULL),
       enabled_(false),
       enable_download_protection_(false),
       enable_csd_whitelist_(false),
       enable_download_whitelist_(false),
       update_in_progress_(false),
       database_update_in_progress_(false),
       closing_database_(false),
       download_urlcheck_timeout_ms_(kDownloadUrlCheckTimeoutMs),
       download_hashcheck_timeout_ms_(kDownloadHashCheckTimeoutMs) {
-#if !defined(OS_CHROMEOS)
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDisableClientSidePhishingDetection)) {
-    csd_service_.reset(
-        safe_browsing::ClientSideDetectionService::Create(
-            g_browser_process->system_request_context()));
-  }
-  download_service_.reset(new safe_browsing::DownloadProtectionService(
-      this,
-      g_browser_process->system_request_context()));
-#endif
 }
 
 SafeBrowsingService::~SafeBrowsingService() {
   // Deletes the PrefChangeRegistrars, whose dtors also unregister |this| as an
   // observer of the preferences.
   STLDeleteValues(&prefs_map_);
 
   // We should have already been shut down. If we're still enabled, then the
   // database isn't going to be closed properly, which could lead to corruption.
   DCHECK(!enabled_);
 }
 
 void SafeBrowsingService::Initialize() {
+  url_request_context_getter_ =
+      new SafeBrowsingURLRequestContextGetter(this);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(
+          &SafeBrowsingService::InitURLRequestContextOnIOThread, this,
+          make_scoped_refptr(g_browser_process->system_request_context())));
+#if !defined(OS_CHROMEOS)
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableClientSidePhishingDetection)) {
+    csd_service_.reset(
+        safe_browsing::ClientSideDetectionService::Create(
+            url_request_context_getter_));
+  }
+  download_service_.reset(new safe_browsing::DownloadProtectionService(
+      this,
+      url_request_context_getter_));
+#endif
+
   // Track the safe browsing preference of existing profiles.
   // The SafeBrowsingService will be started if any existing profile has the
   // preference enabled. It will also listen for updates to the preferences.
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   if (profile_manager) {
     std::vector<Profile*> profiles = profile_manager->GetLoadedProfiles();
     for (size_t i = 0; i < profiles.size(); ++i) {
       if (profiles[i]->IsOffTheRecord())
         continue;
       AddPrefService(profiles[i]->GetPrefs());
     }
   }
 
   // Track profile creation and destruction.
   prefs_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CREATED,
                        content::NotificationService::AllSources());
   prefs_registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
                        content::NotificationService::AllSources());
 }
 
 void SafeBrowsingService::ShutDown() {
   Stop();
   // The IO thread is going away, so make sure the ClientSideDetectionService
   // dtor executes now since it may call the dtor of URLFetcher which relies
   // on it.
   csd_service_.reset();
   download_service_.reset();
+
+  url_request_context_getter_ = NULL;
+  BrowserThread::PostTask(

[willchan no longer on Chromium, 2012/02/09 19:41:30]

I think this is right, but I just want to point out that this will add a
refcount that will be decremented on the IO thread. SafeBrowsingService appears
to be RefCountedThreadSafe with deletion on the UI thread. It's not clear to me
how it ever gets deleted, since hasn't the UI thread already stopped by this
point? Is this just always leaked?

That said, I don't think this is a big change in behavior because Stop() already
seems to post to the IO thread. Anyway, I found it intriguing. I'm fine with
this as long as you safe browsing folks are.

[mattm, 2012/02/09 22:40:09]

I think this works out in practice, since BrowserProcessImpl maintains a
reference to the SafeBrowsingService.  By the time BrowserProcessImpl gets
destructed, the tasks with references to SBService on the IOthread have already
been run, so the BrowserProcessImpl should be the one holding the last ref, and
the message loop doesn't need to be running if the last ref is released on the
correct thread.  (I'm not 100% sure if there isn't some case where something
else might still hold a ref to it, but in a few quick tests it did get
destructed.)

+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&SafeBrowsingService::DestroyURLRequestContextOnIOThread,
+                 this));
 }
 
 bool SafeBrowsingService::CanCheckUrl(const GURL& url) const {
   return url.SchemeIs(chrome::kFtpScheme) ||
          url.SchemeIs(chrome::kHttpScheme) ||
          url.SchemeIs(chrome::kHttpsScheme);
 }
 
 // Only report SafeBrowsing related stats when UMA is enabled. User must also
 // ensure that safe browsing is enabled from the calling profile.
@@ skipping 281 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(enabled_);
   safe_browsing_thread_->message_loop()->PostTask(FROM_HERE, base::Bind(
       &SafeBrowsingService::OnResetDatabase, this));
 }
 
 void SafeBrowsingService::LogPauseDelay(base::TimeDelta time) {
   UMA_HISTOGRAM_LONG_TIMES("SB2.Delay", time);
 }
 
-void SafeBrowsingService::OnIOInitialize(
+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(
+          FilePath(BaseFilename().value() + kCookiesFile), false),
+      NULL);
+
+  url_request_context_ = new SafeBrowsingURLRequestContext;
+  // |system_url_request_context_getter| may be NULL during tests.
+  if (system_url_request_context_getter)

[mattm, 2012/02/07 01:28:01]

Had to update DownloadProtectionServiceTest to call Initialize and ShutDown so
that sb_service_->download_protection_service() is created, which introduces the
system_url_request_context_getter being NULL problem again.  This fix doesn't
seem great but let me know if you have a better idea.

+    url_request_context_->CopyFrom(
+        system_url_request_context_getter->GetURLRequestContext());
+  url_request_context_->set_cookie_store(cookie_store);
+}
+
+void SafeBrowsingService::DestroyURLRequestContextOnIOThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // Need to do the CheckForLeaks on IOThread instead of in ShutDown where
+  // url_request_context_getter_ is cleared,  since the URLRequestContextGetter
+  // will PostTask to IOTread to delete itself.
+  using base::debug::LeakTracker;
+  LeakTracker<SafeBrowsingURLRequestContextGetter>::CheckForLeaks();
+
+  DCHECK(url_request_context_.get());
+  url_request_context_ = NULL;
+}
+
+void SafeBrowsingService::StartOnIOThread(
     const std::string& client_key,
-    const std::string& wrapped_key,
-    net::URLRequestContextGetter* request_context_getter) {
+    const std::string& wrapped_key) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (enabled_)
     return;
   DCHECK(!safe_browsing_thread_.get());
   safe_browsing_thread_.reset(new base::Thread("Chrome_SafeBrowsingThread"));
   if (!safe_browsing_thread_->Start())
     return;
   enabled_ = true;
 
   registrar_.reset(new content::NotificationRegistrar);
@@ skipping 27 matching lines @@
       cmdline->HasSwitch(switches::kSbMacKeyURLPrefix) ?
       cmdline->GetSwitchValueASCII(switches::kSbMacKeyURLPrefix) :
       kSbDefaultMacKeyURLPrefix;
 
   DCHECK(!protocol_manager_);
   protocol_manager_ =
       SafeBrowsingProtocolManager::Create(this,
                                           client_name,
                                           client_key,
                                           wrapped_key,
-                                          request_context_getter,
+                                          url_request_context_getter_,
                                           info_url_prefix,
                                           mackey_url_prefix,
                                           disable_auto_update);
 
   protocol_manager_->Initialize();
 }
 
-void SafeBrowsingService::OnIOShutdown() {
+void SafeBrowsingService::StopOnIOThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!enabled_)
     return;
 
   enabled_ = false;
 
   registrar_.reset();
 
   // This cancels all in-flight GetHash requests.
   delete protocol_manager_;
@@ skipping 92 matching lines @@
   if (safe_browsing_thread_.get()) {
     safe_browsing_thread_->message_loop()->PostTask(FROM_HERE,
         base::Bind(&SafeBrowsingService::OnCloseDatabase, this));
   }
 }
 
 SafeBrowsingDatabase* SafeBrowsingService::GetDatabase() {
   DCHECK_EQ(MessageLoop::current(), safe_browsing_thread_->message_loop());
   if (database_)
     return database_;
-
-  FilePath path;
-  bool result = PathService::Get(chrome::DIR_USER_DATA, &path);
-  DCHECK(result);
-  path = path.Append(chrome::kSafeBrowsingBaseFilename);
-
   const base::TimeTicks before = base::TimeTicks::Now();
 
   SafeBrowsingDatabase* database =
       SafeBrowsingDatabase::Create(enable_download_protection_,
                                    enable_csd_whitelist_,
                                    enable_download_whitelist_);
 
-  database->Init(path);
+  database->Init(BaseFilename());
   {
     // Acquiring the lock here guarantees correct ordering between the writes to
     // the new database object above, and the setting of |databse_| below.
     base::AutoLock lock(database_lock_);
     database_ = database;
   }
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&SafeBrowsingService::DatabaseLoadComplete, this));
@@ skipping 166 matching lines @@
   PrefService* local_state = g_browser_process->local_state();
   DCHECK(local_state);
   std::string client_key, wrapped_key;
   if (local_state) {
     client_key =
       local_state->GetString(prefs::kSafeBrowsingClientKey);
     wrapped_key =
       local_state->GetString(prefs::kSafeBrowsingWrappedKey);
   }
 
-  // We will issue network fetches using the system request context.
-  scoped_refptr<net::URLRequestContextGetter> request_context_getter(
-    g_browser_process->system_request_context());
-
   CommandLine* cmdline = CommandLine::ForCurrentProcess();
   enable_download_protection_ =
       !cmdline->HasSwitch(switches::kSbDisableDownloadProtection);
 
   // We only download the csd-whitelist if client-side phishing detection is
   // enabled.
 #ifdef OS_CHROMEOS
   // Client-side detection is disabled on ChromeOS for now, so don't bother
   // downloading the whitelist.
   enable_csd_whitelist_ = false;
 #else
   enable_csd_whitelist_ =
       !cmdline->HasSwitch(switches::kDisableClientSidePhishingDetection);
 #endif
 
   // TODO(noelutz): remove this boolean variable since it should always be true
   // if SafeBrowsing is enabled.  Unfortunately, we have no test data for this
   // list right now.  This means that we need to be able to disable this list
   // for the SafeBrowsing test to pass.
   enable_download_whitelist_ = enable_csd_whitelist_;
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&SafeBrowsingService::OnIOInitialize,
-                 this, client_key, wrapped_key, request_context_getter));
+      base::Bind(&SafeBrowsingService::StartOnIOThread,
+                 this, client_key, wrapped_key));
 }
 
 void SafeBrowsingService::Stop() {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&SafeBrowsingService::OnIOShutdown, this));
+      base::Bind(&SafeBrowsingService::StopOnIOThread, this));
 }
 
 void SafeBrowsingService::OnCloseDatabase() {
   DCHECK_EQ(MessageLoop::current(), safe_browsing_thread_->message_loop());
   DCHECK(closing_database_);
 
   // Because |closing_database_| is true, nothing on the IO thread will be
   // accessing the database, so it's safe to delete and then NULL the pointer.
   delete database_;
   database_ = NULL;
@@ skipping 404 matching lines @@
     Stop();
 
   if (csd_service_.get())
     csd_service_->SetEnabledAndRefreshState(enable);
   if (download_service_.get()) {
     download_service_->SetEnabled(
         enable && !CommandLine::ForCurrentProcess()->HasSwitch(
             switches::kDisableImprovedDownloadProtection));
   }
 }