Refactored cookies persistent store clean-up on shutdown.

chrome/browser/net/sqlite_persistent_cookie_store.cc

 // Copyright (c) 2010 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/sqlite_persistent_cookie_store.h"
 
 #include <list>
 
 #include "app/sql/meta_table.h"
 #include "app/sql/statement.h"
@@ skipping 13 matching lines @@
 using base::Time;
 
 // This class is designed to be shared between any calling threads and the
 // database thread.  It batches operations and commits them on a timer.
 class SQLitePersistentCookieStore::Backend
     : public base::RefCountedThreadSafe<SQLitePersistentCookieStore::Backend> {
  public:
   explicit Backend(const FilePath& path)
       : path_(path),
         db_(NULL),
-        num_pending_(0) {
+        num_pending_(0),
+        clear_local_state_on_exit_(false) {
   }
 
   // Creates or load the SQLite database.
   bool Load(std::vector<net::CookieMonster::CanonicalCookie*>* cookies);
 
   // Batch a cookie addition.
   void AddCookie(const net::CookieMonster::CanonicalCookie& cc);
 
   // Batch a cookie access time update.
   void UpdateCookieAccessTime(const net::CookieMonster::CanonicalCookie& cc);
 
   // Batch a cookie deletion.
   void DeleteCookie(const net::CookieMonster::CanonicalCookie& cc);
 
   // Commit any pending operations and close the database.  This must be called
   // before the object is destructed.
   void Close();
 
+  void SetClearLocalStateOnExit(bool clear_local_state) {
+    clear_local_state_on_exit_ = clear_local_state;
+  }
+
  private:
   friend class base::RefCountedThreadSafe<SQLitePersistentCookieStore::Backend>;
 
   // You should call Close() before destructing this object.
   ~Backend() {
     DCHECK(!db_.get()) << "Close should have already been called.";
     DCHECK(num_pending_ == 0 && pending_.empty());
   }
 
   // Database upgrade statements.
@@ skipping 29 matching lines @@
   void InternalBackgroundClose();
 
   FilePath path_;
   scoped_ptr<sql::Connection> db_;
   sql::MetaTable meta_table_;
 
   typedef std::list<PendingOperation*> PendingOperationsList;
   PendingOperationsList pending_;
   PendingOperationsList::size_type num_pending_;
   Lock pending_lock_;  // Guard pending_ and num_pending_
+  // True if the persistent store should be deleted upon destruction.
+  bool clear_local_state_on_exit_;
 
   DISALLOW_COPY_AND_ASSIGN(Backend);
 };
 
 // Version number of the database. In version 4, we migrated the time epoch.
 // If you open the DB with an older version on Mac or Linux, the times will
 // look wonky, but the file will likely be usable. On Windows version 3 and 4
 // are the same.
 //
 // Version 3 updated the database to include the last access time, so we can
@@ skipping 301 matching lines @@
       BrowserThread::DB, FROM_HERE,
       NewRunnableMethod(this, &Backend::InternalBackgroundClose));
 }
 
 void SQLitePersistentCookieStore::Backend::InternalBackgroundClose() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
   // Commit any pending operations
   Commit();
 
   db_.reset();
+
+  if (clear_local_state_on_exit_)
+    file_util::Delete(path_, false);
 }
 
 SQLitePersistentCookieStore::SQLitePersistentCookieStore(const FilePath& path)
     : backend_(new Backend(path)) {
 }
 
 SQLitePersistentCookieStore::~SQLitePersistentCookieStore() {
   if (backend_.get()) {
     backend_->Close();
     // Release our reference, it will probably still have a reference if the
@@ skipping 18 matching lines @@
   if (backend_.get())
     backend_->UpdateCookieAccessTime(cc);
 }
 
 void SQLitePersistentCookieStore::DeleteCookie(
     const net::CookieMonster::CanonicalCookie& cc) {
   if (backend_.get())
     backend_->DeleteCookie(cc);
 }
 
-// static
-void SQLitePersistentCookieStore::ClearLocalState(
-    const FilePath& path) {
-  file_util::Delete(path, false);
+void SQLitePersistentCookieStore::SetClearLocalStateOnExit(
+    bool clear_local_state) {
+  if (backend_.get())
+    backend_->SetClearLocalStateOnExit(clear_local_state);

[Randy Smith (Not in Mondays), 2010/12/03 18:42:27]

Could you protect this with the lock (and modify the comment on the lock to
indicate it protects this field as well)?  It shouldn't matter--boolean accesses
should be atomic so you don't really care who wins the possible races, and I
can't come up with a case in which the race would actually happen--but I'd
rather be extra careful around thread safety in chrome since most code is
single-threaded and hence most developers aren't used to thinking about thread
safety in detail.

[pastarmovj, 2010/12/06 08:57:40]

Done. I had exactly the same thoughts and didn't put the lock there because all
calls to this function are routed through the same thread. However maybe it's
more failsafe this way anyhow and I don't see potential for deadlocks here
either.

 }