Remove PurgeMemory() from the safe browsing database manager.

BUG=350455
TEST=none
R=shess@chromium.org

Review URL: https://codereview.chromium.org/194003002

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

2 files changed, 17 insertions, 53 deletions

diff --git a/chrome/browser/safe_browsing/database_manager.cc b/chrome/browser/safe_browsing/database_manager.cc
index acf922f..f1b921b 100644
--- a/chrome/browser/safe_browsing/database_manager.cc
+++ b/chrome/browser/safe_browsing/database_manager.cc
@@ -470,11 +470,6 @@ void SafeBrowsingDatabaseManager::ResetDatabase() {
       &SafeBrowsingDatabaseManager::OnResetDatabase, this));
 }
 
-void SafeBrowsingDatabaseManager::PurgeMemory() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  CloseDatabase();
-}
-
 void SafeBrowsingDatabaseManager::LogPauseDelay(base::TimeDelta time) {
   UMA_HISTOGRAM_LONG_TIMES("SB2.Delay", time);
 }
@@ -533,7 +528,6 @@ void SafeBrowsingDatabaseManager::DoStopOnIOThread() {
   enabled_ = false;
 
   // Delete queued checks, calling back any clients with 'SB_THREAT_TYPE_SAFE'.
-  // If we don't do this here we may fail to close the database below.
   while (!queued_checks_.empty()) {
     QueuedCheck queued = queued_checks_.front();
     if (queued.client) {
@@ -547,11 +541,23 @@ void SafeBrowsingDatabaseManager::DoStopOnIOThread() {
     queued_checks_.pop_front();
   }
 
-  // Close the database.  We don't simply DeleteSoon() because if a close is
-  // already pending, we'll double-free, and we don't set |database_| to NULL
-  // because if there is still anything running on the db thread, it could
-  // create a new database object (via GetDatabase()) that would then leak.
-  CloseDatabase();
+  // Close the database.  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.
+  // Checking DatabaseAvailable() avoids both of these.
+  if (DatabaseAvailable()) {
+    closing_database_ = true;
+    safe_browsing_thread_->message_loop()->PostTask(FROM_HERE,
+        base::Bind(&SafeBrowsingDatabaseManager::OnCloseDatabase, this));
+  }
 
   // Flush the database thread. Any in-progress database check results will be
   // ignored and cleaned up below.
@@ -600,37 +606,6 @@ bool SafeBrowsingDatabaseManager::MakeDatabaseAvailable() {
   return false;
 }
 
-void SafeBrowsingDatabaseManager::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,
-        base::Bind(&SafeBrowsingDatabaseManager::OnCloseDatabase, this));
-  }
-}
-
 SafeBrowsingDatabase* SafeBrowsingDatabaseManager::GetDatabase() {
   DCHECK_EQ(base::MessageLoop::current(),
             safe_browsing_thread_->message_loop());
diff --git a/chrome/browser/safe_browsing/database_manager.h b/chrome/browser/safe_browsing/database_manager.h
index bba2d67..a8d8bad 100644
--- a/chrome/browser/safe_browsing/database_manager.h
+++ b/chrome/browser/safe_browsing/database_manager.h
@@ -184,9 +184,6 @@ class SafeBrowsingDatabaseManager
       const std::vector<SBFullHashResult>& full_hashes,
       bool can_cache);
 
-  // Called on the IO thread to release memory.
-  void PurgeMemory();
-
   // Log the user perceived delay caused by SafeBrowsing. This delay is the time
   // delta starting from when we would have started reading data from the
   // network, and ending when the SafeBrowsing check completes indicating that
@@ -250,14 +247,6 @@ class SafeBrowsingDatabaseManager
   // 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();