Separate memory purger from safe browsing.

The memory purger already sends a notification when it's purging memory;
safe browsing can listen for this notification instead of the memory
purger closing the safe browsing database itself.


Review URL: http://codereview.chromium.org/7313003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@91688 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 64 insertions, 40 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index e4564bf..31b2512 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -27,6 +27,7 @@
 #include "chrome/common/url_constants.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/tab_contents/tab_contents.h"
+#include "content/common/notification_service.h"
 #include "net/base/registry_controlled_domain.h"
 #include "net/url_request/url_request_context_getter.h"
 
@@ -468,37 +469,6 @@ void SafeBrowsingService::RegisterPrefs(PrefService* prefs) {
   prefs->RegisterStringPref(prefs::kSafeBrowsingWrappedKey, "");
 }
 
-void SafeBrowsingService::CloseDatabase() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
-  // Cases to avoid:
-  //  * If |closing_database_| is true, continuing will queue up a second
-  //    request, |closing_database_| will be reset after handling the first
-  //    request, and if any functions on the db thread recreate the database, we
-  //    could start using it on the IO thread and then have the second request
-  //    handler delete it out from under us.
-  //  * If |database_| is NULL, then either no creation request is in flight, in
-  //    which case we don't need to do anything, or one is in flight, in which
-  //    case the database will be recreated before our deletion request is
-  //    handled, and could be used on the IO thread in that time period, leading
-  //    to the same problem as above.
-  //  * If |queued_checks_| is non-empty and |database_| is non-NULL, we're
-  //    about to be called back (in DatabaseLoadComplete()).  This will call
-  //    CheckUrl(), which will want the database.  Closing the database here
-  //    would lead to an infinite loop in DatabaseLoadComplete(), and even if it
-  //    didn't, it would be pointless since we'd just want to recreate.
-  //
-  // The first two cases above are handled by checking DatabaseAvailable().
-  if (!DatabaseAvailable() || !queued_checks_.empty())
-    return;
-
-  closing_database_ = true;
-  if (safe_browsing_thread_.get()) {
-    safe_browsing_thread_->message_loop()->PostTask(FROM_HERE,
-        NewRunnableMethod(this, &SafeBrowsingService::OnCloseDatabase));
-  }
-}
-
 void SafeBrowsingService::ResetDatabase() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(enabled_);
@@ -523,6 +493,9 @@ void SafeBrowsingService::OnIOInitialize(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   enabled_ = true;
 
+  registrar_.Add(this, NotificationType::PURGE_MEMORY,
+                 NotificationService::AllSources());
+
   MakeDatabaseAvailable();
 
   // On Windows, get the safe browsing client name from the browser
@@ -572,6 +545,8 @@ void SafeBrowsingService::OnIOShutdown() {
 
   enabled_ = false;
 
+  registrar_.RemoveAll();
+
   // This cancels all in-flight GetHash requests.
   delete protocol_manager_;
   protocol_manager_ = NULL;
@@ -642,6 +617,37 @@ bool SafeBrowsingService::MakeDatabaseAvailable() {
   return false;
 }
 
+void SafeBrowsingService::CloseDatabase() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // Cases to avoid:
+  //  * If |closing_database_| is true, continuing will queue up a second
+  //    request, |closing_database_| will be reset after handling the first
+  //    request, and if any functions on the db thread recreate the database, we
+  //    could start using it on the IO thread and then have the second request
+  //    handler delete it out from under us.
+  //  * If |database_| is NULL, then either no creation request is in flight, in
+  //    which case we don't need to do anything, or one is in flight, in which
+  //    case the database will be recreated before our deletion request is
+  //    handled, and could be used on the IO thread in that time period, leading
+  //    to the same problem as above.
+  //  * If |queued_checks_| is non-empty and |database_| is non-NULL, we're
+  //    about to be called back (in DatabaseLoadComplete()).  This will call
+  //    CheckUrl(), which will want the database.  Closing the database here
+  //    would lead to an infinite loop in DatabaseLoadComplete(), and even if it
+  //    didn't, it would be pointless since we'd just want to recreate.
+  //
+  // The first two cases above are handled by checking DatabaseAvailable().
+  if (!DatabaseAvailable() || !queued_checks_.empty())
+    return;
+
+  closing_database_ = true;
+  if (safe_browsing_thread_.get()) {
+    safe_browsing_thread_->message_loop()->PostTask(FROM_HERE,
+        NewRunnableMethod(this, &SafeBrowsingService::OnCloseDatabase));
+  }
+}
+
 SafeBrowsingDatabase* SafeBrowsingService::GetDatabase() {
   DCHECK_EQ(MessageLoop::current(), safe_browsing_thread_->message_loop());
   if (database_)
@@ -1202,6 +1208,14 @@ void SafeBrowsingService::UpdateWhitelist(const UnsafeResource& resource) {
   white_listed_entries_.push_back(entry);
 }
 
+void SafeBrowsingService::Observe(NotificationType type,
+                                  const NotificationSource& source,
+                                  const NotificationDetails& details) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(type == NotificationType::PURGE_MEMORY);
+  CloseDatabase();
+}
+
 bool SafeBrowsingService::IsWhitelisted(const UnsafeResource& resource) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // Check if the user has already ignored our warning for this render_view
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index 65b7ea2..0d29f46 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -21,6 +21,8 @@
 #include "base/task.h"
 #include "base/time.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
+#include "content/common/notification_observer.h"
+#include "content/common/notification_registrar.h"
 #include "googleurl/src/gurl.h"
 
 class MalwareDetails;
@@ -39,7 +41,8 @@ class URLRequestContextGetter;
 
 // Construction needs to happen on the main thread.
 class SafeBrowsingService
-    : public base::RefCountedThreadSafe<SafeBrowsingService> {
+    : public base::RefCountedThreadSafe<SafeBrowsingService>,
+      public NotificationObserver {
  public:
   class Client;
   // Users of this service implement this interface to be notified
@@ -236,14 +239,6 @@ class SafeBrowsingService
   // Preference handling.
   static void RegisterPrefs(PrefService* prefs);
 
-  // Called on the IO thread to try to close the database, freeing the memory
-  // associated with it.  The database will be automatically reopened as needed.
-  //
-  // NOTE: Actual database closure is asynchronous, and until it happens, the IO
-  // thread is not allowed to access it; may not actually trigger a close if one
-  // is already pending or doing so would cause problems.
-  void CloseDatabase();
-
   // Called on the IO thread to reset the database.
   void ResetDatabase();
 
@@ -311,6 +306,14 @@ class SafeBrowsingService
   // db thread can call GetDatabase() directly.
   bool MakeDatabaseAvailable();
 
+  // Called on the IO thread to try to close the database, freeing the memory
+  // associated with it.  The database will be automatically reopened as needed.
+  //
+  // NOTE: Actual database closure is asynchronous, and until it happens, the IO
+  // thread is not allowed to access it; may not actually trigger a close if one
+  // is already pending or doing so would cause problems.
+  void CloseDatabase();
+
   // Should only be called on db thread as SafeBrowsingDatabase is not
   // threadsafe.
   SafeBrowsingDatabase* GetDatabase();
@@ -410,6 +413,11 @@ class SafeBrowsingService
   // Adds the given entry to the whitelist.  Called on the UI thread.
   void UpdateWhitelist(const UnsafeResource& resource);
 
+  // NotificationObserver override
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) OVERRIDE;
+
   // The factory used to instanciate a SafeBrowsingService object.
   // Useful for tests, so they can provide their own implementation of
   // SafeBrowsingService.
@@ -472,6 +480,8 @@ class SafeBrowsingService
   // Similar to |download_urlcheck_timeout_ms_|, but for download hash checks.
   int64 download_hashcheck_timeout_ms_;
 
+  NotificationRegistrar registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingService);
 };