Moved deleting the indexed db context to the IndexedDBContext destructor.

This gives us the safety that we delete the files only when they are certainly not accessed by the webkit context anymore.

BUG=56249
TEST=IndexedDBBrowserTest.ClearLocalState

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

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

11 files changed, 93 insertions, 45 deletions

diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 33659d0..bf1a52f 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -30,7 +30,6 @@
 #include "chrome/browser/google/google_url_tracker.h"
 #include "chrome/browser/icon_manager.h"
 #include "chrome/browser/in_process_webkit/dom_storage_context.h"
-#include "chrome/browser/in_process_webkit/indexed_db_context.h"
 #include "chrome/browser/intranet_redirect_detector.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/metrics/metrics_service.h"
@@ -506,7 +505,6 @@ void BrowserProcessImpl::ClearLocalState(const FilePath& profile_path) {
   SQLitePersistentCookieStore::ClearLocalState(profile_path.Append(
       chrome::kCookieFilename));
   DOMStorageContext::ClearLocalState(profile_path, chrome::kExtensionScheme);
-  IndexedDBContext::ClearLocalState(profile_path, chrome::kExtensionScheme);
   webkit_database::DatabaseTracker::ClearLocalState(profile_path);
   ChromeAppCacheService::ClearLocalState(profile_path);
 }
diff --git a/chrome/browser/in_process_webkit/indexed_db_browsertest.cc b/chrome/browser/in_process_webkit/indexed_db_browsertest.cc
index 4eeac74..d66c820 100644
--- a/chrome/browser/in_process_webkit/indexed_db_browsertest.cc
+++ b/chrome/browser/in_process_webkit/indexed_db_browsertest.cc
@@ -9,10 +9,13 @@
 #include "base/scoped_temp_dir.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/in_process_webkit/indexed_db_context.h"
+#include "chrome/browser/in_process_webkit/webkit_context.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/in_process_browser_test.h"
+#include "chrome/test/testing_profile.h"
+#include "chrome/test/thread_test_helper.h"
 #include "chrome/test/ui_test_utils.h"
 
 // This browser test is aimed towards exercising the IndexedDB bindings and
@@ -92,9 +95,9 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, ClearLocalState) {
       IndexedDBContext::kIndexedDBDirectory);
   ASSERT_TRUE(file_util::CreateDirectory(indexeddb_dir));
 
-  FilePath::StringType file_name_1(FILE_PATH_LITERAL("http_www.google.com_0"));
+  FilePath::StringType file_name_1(FILE_PATH_LITERAL("http_foo_0"));
   file_name_1.append(IndexedDBContext::kIndexedDBExtension);
-  FilePath::StringType file_name_2(FILE_PATH_LITERAL("https_www.google.com_0"));
+  FilePath::StringType file_name_2(FILE_PATH_LITERAL("chrome-extension_foo_0"));
   file_name_2.append(IndexedDBContext::kIndexedDBExtension);
   FilePath temp_file_path_1 = indexeddb_dir.Append(file_name_1);
   FilePath temp_file_path_2 = indexeddb_dir.Append(file_name_2);
@@ -102,7 +105,18 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, ClearLocalState) {
   ASSERT_EQ(1, file_util::WriteFile(temp_file_path_1, ".", 1));
   ASSERT_EQ(1, file_util::WriteFile(temp_file_path_2, "o", 1));
 
-  IndexedDBContext::ClearLocalState(temp_dir.path(), "https");
+  // Create the scope which will ensure we run the destructor of the webkit
+  // context which should trigger the clean up.
+  {
+    TestingProfile profile;
+    WebKitContext *webkit_context = profile.GetWebKitContext();
+    webkit_context->indexed_db_context()->set_data_path(indexeddb_dir);
+    webkit_context->set_clear_local_state_on_exit(true);
+  }
+  // Make sure we wait until the destructor has run.
+  scoped_refptr<ThreadTestHelper> helper(
+      new ThreadTestHelper(BrowserThread::WEBKIT));
+  ASSERT_TRUE(helper->Run());
 
   // Because we specified https for scheme to be skipped the second file
   // should survive and the first go into vanity.
diff --git a/chrome/browser/in_process_webkit/indexed_db_context.cc b/chrome/browser/in_process_webkit/indexed_db_context.cc
index 7f492ac..7aeb1b4 100644
--- a/chrome/browser/in_process_webkit/indexed_db_context.cc
+++ b/chrome/browser/in_process_webkit/indexed_db_context.cc
@@ -10,6 +10,7 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/in_process_webkit/webkit_context.h"
+#include "chrome/common/url_constants.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebCString.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebIDBDatabase.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebIDBFactory.h"
@@ -21,17 +22,45 @@ using WebKit::WebIDBDatabase;
 using WebKit::WebIDBFactory;
 using WebKit::WebSecurityOrigin;
 
+namespace {
+
+void ClearLocalState(const FilePath& indexeddb_path,
+                     const char* url_scheme_to_be_skipped) {
+  file_util::FileEnumerator file_enumerator(
+      indexeddb_path, false, file_util::FileEnumerator::FILES);
+  // TODO(pastarmovj): We might need to consider exchanging this loop for
+  // something more efficient in the future.
+  for (FilePath file_path = file_enumerator.Next(); !file_path.empty();
+       file_path = file_enumerator.Next()) {
+    if (file_path.Extension() != IndexedDBContext::kIndexedDBExtension)
+      continue;
+    WebSecurityOrigin origin =
+        WebSecurityOrigin::createFromDatabaseIdentifier(
+            webkit_glue::FilePathToWebString(file_path.BaseName()));
+    if (!EqualsASCII(origin.protocol(), url_scheme_to_be_skipped))
+      file_util::Delete(file_path, false);
+  }
+}
+
+}  // namespace
+
 const FilePath::CharType IndexedDBContext::kIndexedDBDirectory[] =
     FILE_PATH_LITERAL("IndexedDB");
 
 const FilePath::CharType IndexedDBContext::kIndexedDBExtension[] =
     FILE_PATH_LITERAL(".indexeddb");
 
-IndexedDBContext::IndexedDBContext(WebKitContext* webkit_context)
-    : webkit_context_(webkit_context) {
+IndexedDBContext::IndexedDBContext(WebKitContext* webkit_context) {
+  data_path_ = webkit_context->data_path().Append(kIndexedDBDirectory);
 }
 
 IndexedDBContext::~IndexedDBContext() {
+  // Not being on the WEBKIT thread here means we are running in a unit test
+  // where no clean up is needed.
+  if (clear_local_state_on_exit_ &&
+      BrowserThread::CurrentlyOn(BrowserThread::WEBKIT)) {
+    ClearLocalState(data_path_, chrome::kExtensionScheme);
+  }
 }
 
 WebIDBFactory* IndexedDBContext::GetIDBFactory() {
@@ -43,29 +72,8 @@ WebIDBFactory* IndexedDBContext::GetIDBFactory() {
 
 FilePath IndexedDBContext::GetIndexedDBFilePath(
     const string16& origin_id) const {
-  FilePath storage_dir = webkit_context_->data_path().Append(
-      kIndexedDBDirectory);
   FilePath::StringType id = webkit_glue::WebStringToFilePathString(origin_id);
-  return storage_dir.Append(id.append(kIndexedDBExtension));
-}
-
-// static
-void IndexedDBContext::ClearLocalState(const FilePath& profile_path,
-                                       const char* url_scheme_to_be_skipped) {
-  file_util::FileEnumerator file_enumerator(profile_path.Append(
-      kIndexedDBDirectory), false, file_util::FileEnumerator::FILES);
-  // TODO(pastarmovj): We might need to consider exchanging this loop for
-  // something more efficient in the future.
-  for (FilePath file_path = file_enumerator.Next(); !file_path.empty();
-       file_path = file_enumerator.Next()) {
-    if (file_path.Extension() != IndexedDBContext::kIndexedDBExtension)
-      continue;
-    WebSecurityOrigin origin =
-        WebSecurityOrigin::createFromDatabaseIdentifier(
-            webkit_glue::FilePathToWebString(file_path.BaseName()));
-    if (!EqualsASCII(origin.protocol(), url_scheme_to_be_skipped))
-      file_util::Delete(file_path, false);
-  }
+  return data_path_.Append(id.append(kIndexedDBExtension));
 }
 
 void IndexedDBContext::DeleteIndexedDBFile(const FilePath& file_path) {
diff --git a/chrome/browser/in_process_webkit/indexed_db_context.h b/chrome/browser/in_process_webkit/indexed_db_context.h
index 95f6536..16f8a0f 100644
--- a/chrome/browser/in_process_webkit/indexed_db_context.h
+++ b/chrome/browser/in_process_webkit/indexed_db_context.h
@@ -33,10 +33,9 @@ class IndexedDBContext {
   // Get the file name of the indexed db file for the given origin.
   FilePath GetIndexedDBFilePath(const string16& origin_id) const;
 
-  // Deletes all idb files except for those on the url scheme
-  // |url_scheme_to_be_skipped|.
-  static void ClearLocalState(const FilePath& profile_path,
-                              const char* url_scheme_to_be_skipped);
+  void set_clear_local_state_on_exit(bool clear_local_state) {
+    clear_local_state_on_exit_ = clear_local_state;
+  }
 
   // Deletes a single indexed db file.
   void DeleteIndexedDBFile(const FilePath& file_path);
@@ -44,11 +43,19 @@ class IndexedDBContext {
   // Deletes all indexed db files for the given origin.
   void DeleteIndexedDBForOrigin(const string16& origin_id);
 
+#ifdef UNIT_TEST
+  // For unit tests allow to override the |data_path_|.
+  void set_data_path(const FilePath& data_path) { data_path_ = data_path; }
+#endif
+
  private:
   scoped_ptr<WebKit::WebIDBFactory> idb_factory_;
 
-  // We're owned by this WebKit context.
-  WebKitContext* webkit_context_;
+  // Path where the indexed db data is stored
+  FilePath data_path_;
+
+  // True if the destructor should delete its files.
+  bool clear_local_state_on_exit_;
 
   DISALLOW_COPY_AND_ASSIGN(IndexedDBContext);
 };
diff --git a/chrome/browser/in_process_webkit/webkit_context.cc b/chrome/browser/in_process_webkit/webkit_context.cc
index 32fccf7..358c80f 100644
--- a/chrome/browser/in_process_webkit/webkit_context.cc
+++ b/chrome/browser/in_process_webkit/webkit_context.cc
@@ -9,9 +9,10 @@
 #include "chrome/browser/profile.h"
 #include "chrome/common/chrome_switches.h"
 
-WebKitContext::WebKitContext(Profile* profile)
+WebKitContext::WebKitContext(Profile* profile, bool clear_local_state_on_exit)
     : data_path_(profile->IsOffTheRecord() ? FilePath() : profile->GetPath()),
       is_incognito_(profile->IsOffTheRecord()),
+      clear_local_state_on_exit_(clear_local_state_on_exit),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           dom_storage_context_(new DOMStorageContext(this))),
       ALLOW_THIS_IN_INITIALIZER_LIST(
@@ -30,6 +31,8 @@ WebKitContext::~WebKitContext() {
     delete dom_storage_context;
   }
 
+  indexed_db_context_->set_clear_local_state_on_exit(
+      clear_local_state_on_exit_);
   IndexedDBContext* indexed_db_context = indexed_db_context_.release();
   if (!BrowserThread::DeleteSoon(
           BrowserThread::WEBKIT, FROM_HERE, indexed_db_context)) {
diff --git a/chrome/browser/in_process_webkit/webkit_context.h b/chrome/browser/in_process_webkit/webkit_context.h
index be20825c..31a90fe 100644
--- a/chrome/browser/in_process_webkit/webkit_context.h
+++ b/chrome/browser/in_process_webkit/webkit_context.h
@@ -24,9 +24,10 @@ class Profile;
 //
 // This class is created on the UI thread and accessed on the UI, IO, and WebKit
 // threads.
-class WebKitContext : public base::RefCountedThreadSafe<WebKitContext> {
+class WebKitContext
+    : public base::RefCountedThreadSafe<WebKitContext> {
  public:
-  explicit WebKitContext(Profile* profile);
+  explicit WebKitContext(Profile* profile, bool clear_local_state_on_exit);
 
   const FilePath& data_path() const { return data_path_; }
   bool is_incognito() const { return is_incognito_; }
@@ -39,6 +40,10 @@ class WebKitContext : public base::RefCountedThreadSafe<WebKitContext> {
     return indexed_db_context_.get();
   }
 
+  void set_clear_local_state_on_exit(bool clear_local_state) {
+    clear_local_state_on_exit_ = clear_local_state;
+  }
+
 #ifdef UNIT_TEST
   // For unit tests, allow specifying a DOMStorageContext directly so it can be
   // mocked.
@@ -62,12 +67,15 @@ class WebKitContext : public base::RefCountedThreadSafe<WebKitContext> {
 
  private:
   friend class base::RefCountedThreadSafe<WebKitContext>;
-  ~WebKitContext();
+  virtual ~WebKitContext();
 
   // Copies of profile data that can be accessed on any thread.
   const FilePath data_path_;
   const bool is_incognito_;
 
+  // True if the destructors of context objects should delete their files.
+  bool clear_local_state_on_exit_;
+
   scoped_ptr<DOMStorageContext> dom_storage_context_;
 
   scoped_ptr<IndexedDBContext> indexed_db_context_;
diff --git a/chrome/browser/in_process_webkit/webkit_context_unittest.cc b/chrome/browser/in_process_webkit/webkit_context_unittest.cc
index 92334d6..c8870aab 100644
--- a/chrome/browser/in_process_webkit/webkit_context_unittest.cc
+++ b/chrome/browser/in_process_webkit/webkit_context_unittest.cc
@@ -30,11 +30,11 @@ class MockDOMStorageContext : public DOMStorageContext {
 TEST(WebKitContextTest, Basic) {
   TestingProfile profile;
 
-  scoped_refptr<WebKitContext> context1(new WebKitContext(&profile));
+  scoped_refptr<WebKitContext> context1(new WebKitContext(&profile, false));
   EXPECT_TRUE(profile.GetPath() == context1->data_path());
   EXPECT_TRUE(profile.IsOffTheRecord() == context1->is_incognito());
 
-  scoped_refptr<WebKitContext> context2(new WebKitContext(&profile));
+  scoped_refptr<WebKitContext> context2(new WebKitContext(&profile, false));
   EXPECT_TRUE(context1->data_path() == context2->data_path());
   EXPECT_TRUE(context1->is_incognito() == context2->is_incognito());
 }
@@ -47,7 +47,7 @@ TEST(WebKitContextTest, PurgeMemory) {
 
   // Create the contexts.
   TestingProfile profile;
-  scoped_refptr<WebKitContext> context(new WebKitContext(&profile));
+  scoped_refptr<WebKitContext> context(new WebKitContext(&profile, false));
   MockDOMStorageContext* mock_context =
       new MockDOMStorageContext(context.get());
   context->set_dom_storage_context(mock_context);  // Takes ownership.
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index 9ccda97..c03483a 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -500,7 +500,7 @@ class OffTheRecordProfileImpl : public Profile,
 
   virtual WebKitContext* GetWebKitContext() {
     if (!webkit_context_.get())
-      webkit_context_ = new WebKitContext(this);
+      webkit_context_ = new WebKitContext(this, false);
     DCHECK(webkit_context_.get());
     return webkit_context_.get();
   }
diff --git a/chrome/browser/profile_impl.cc b/chrome/browser/profile_impl.cc
index cad400b..6e19ef4 100644
--- a/chrome/browser/profile_impl.cc
+++ b/chrome/browser/profile_impl.cc
@@ -273,6 +273,7 @@ ProfileImpl::ProfileImpl(const FilePath& path)
   pref_change_registrar_.Add(prefs::kSpellCheckDictionary, this);
   pref_change_registrar_.Add(prefs::kEnableSpellCheck, this);
   pref_change_registrar_.Add(prefs::kEnableAutoSpellCorrect, this);
+  pref_change_registrar_.Add(prefs::kClearSiteDataOnExit, this);
 
   // Convert active labs into switches. Modifies the current command line.
   about_flags::ConvertFlagsToSwitches(prefs, CommandLine::ForCurrentProcess());
@@ -318,6 +319,8 @@ ProfileImpl::ProfileImpl(const FilePath& path)
 
   GetPolicyContext()->Initialize();
 
+  clear_local_state_on_exit_ = prefs->GetBoolean(prefs::kClearSiteDataOnExit);
+
   // Log the profile size after a reasonable startup delay.
   BrowserThread::PostDelayedTask(BrowserThread::FILE, FROM_HERE,
                                  new ProfileSizeTask(path_), 112000);
@@ -1166,7 +1169,7 @@ void ProfileImpl::SpellCheckHostInitialized() {
 
 WebKitContext* ProfileImpl::GetWebKitContext() {
   if (!webkit_context_.get())
-    webkit_context_ = new WebKitContext(this);
+    webkit_context_ = new WebKitContext(this, clear_local_state_on_exit_);
   DCHECK(webkit_context_.get());
   return webkit_context_.get();
 }
@@ -1205,6 +1208,12 @@ void ProfileImpl::Observe(NotificationType type,
       NotificationService::current()->Notify(
           NotificationType::SPELLCHECK_AUTOSPELL_TOGGLED,
               Source<Profile>(this), NotificationService::NoDetails());
+    } else if (*pref_name_in == prefs::kClearSiteDataOnExit) {
+      clear_local_state_on_exit_ =
+          prefs->GetBoolean(prefs::kClearSiteDataOnExit);
+      if (webkit_context_)
+        webkit_context_->set_clear_local_state_on_exit(
+            clear_local_state_on_exit_);
     }
   } else if (NotificationType::THEME_INSTALLED == type) {
     DCHECK_EQ(Source<Profile>(source).ptr(), GetOriginalProfile());
diff --git a/chrome/browser/profile_impl.h b/chrome/browser/profile_impl.h
index b0ea586..83a4b93 100644
--- a/chrome/browser/profile_impl.h
+++ b/chrome/browser/profile_impl.h
@@ -234,6 +234,7 @@ class ProfileImpl : public Profile,
   bool created_password_store_;
   bool created_download_manager_;
   bool created_theme_provider_;
+  bool clear_local_state_on_exit_;
   // Whether or not the last session exited cleanly. This is set only once.
   bool last_session_exited_cleanly_;
 
diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc
index 6c6b035..243612e 100644
--- a/chrome/test/testing_profile.cc
+++ b/chrome/test/testing_profile.cc
@@ -473,7 +473,7 @@ void TestingProfile::set_session_service(SessionService* session_service) {
 
 WebKitContext* TestingProfile::GetWebKitContext() {
   if (webkit_context_ == NULL)
-    webkit_context_ = new WebKitContext(this);
+    webkit_context_ = new WebKitContext(this, false);
   return webkit_context_;
 }