Refactor DOM storage context clean-up on shutdown.

Changed from a call to DOMStorageContext::ClearLocalStorage
from BrowserImpl to a call to that code from the destructor
of DOMStorageContext.

BUG=64627
TEST=DOMStorageBrowserTest.ClearLocalState

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

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

6 files changed, 113 insertions, 35 deletions

diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index bf1a52f..44f459b 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -29,7 +29,6 @@
 #include "chrome/browser/first_run/first_run.h"
 #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/intranet_redirect_detector.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/metrics/metrics_service.h"
@@ -504,7 +503,6 @@ bool BrowserProcessImpl::have_inspector_files() const {
 void BrowserProcessImpl::ClearLocalState(const FilePath& profile_path) {
   SQLitePersistentCookieStore::ClearLocalState(profile_path.Append(
       chrome::kCookieFilename));
-  DOMStorageContext::ClearLocalState(profile_path, chrome::kExtensionScheme);
   webkit_database::DatabaseTracker::ClearLocalState(profile_path);
   ChromeAppCacheService::ClearLocalState(profile_path);
 }
diff --git a/chrome/browser/in_process_webkit/dom_storage_browsertest.cc b/chrome/browser/in_process_webkit/dom_storage_browsertest.cc
new file mode 100644
index 0000000..fac7d42
--- /dev/null
+++ b/chrome/browser/in_process_webkit/dom_storage_browsertest.cc
@@ -0,0 +1,53 @@
+// 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 "base/file_path.h"
+#include "base/file_util.h"
+#include "base/scoped_temp_dir.h"
+#include "chrome/browser/in_process_webkit/dom_storage_context.h"
+#include "chrome/browser/in_process_webkit/webkit_context.h"
+#include "chrome/test/in_process_browser_test.h"
+#include "chrome/test/testing_profile.h"
+#include "chrome/test/thread_test_helper.h"
+
+typedef InProcessBrowserTest DOMStorageBrowserTest;
+
+// In proc browser test is needed here because ClearLocalState indirectly calls
+// WebKit's isMainThread through WebSecurityOrigin->SecurityOrigin.
+IN_PROC_BROWSER_TEST_F(DOMStorageBrowserTest, ClearLocalState) {
+  // Create test files.
+  ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  FilePath domstorage_dir = temp_dir.path().Append(
+      DOMStorageContext::kLocalStorageDirectory);
+  ASSERT_TRUE(file_util::CreateDirectory(domstorage_dir));
+
+  FilePath::StringType file_name_1(FILE_PATH_LITERAL("http_foo_0"));
+  file_name_1.append(DOMStorageContext::kLocalStorageExtension);
+  FilePath::StringType file_name_2(FILE_PATH_LITERAL("chrome-extension_foo_0"));
+  file_name_2.append(DOMStorageContext::kLocalStorageExtension);
+  FilePath temp_file_path_1 = domstorage_dir.Append(file_name_1);
+  FilePath temp_file_path_2 = domstorage_dir.Append(file_name_2);
+
+  ASSERT_EQ(1, file_util::WriteFile(temp_file_path_1, ".", 1));
+  ASSERT_EQ(1, file_util::WriteFile(temp_file_path_2, "o", 1));
+
+  // 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->dom_storage_context()->set_data_path(temp_dir.path());
+    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.
+  ASSERT_FALSE(file_util::PathExists(temp_file_path_1));
+  ASSERT_TRUE(file_util::PathExists(temp_file_path_2));
+}
diff --git a/chrome/browser/in_process_webkit/dom_storage_context.cc b/chrome/browser/in_process_webkit/dom_storage_context.cc
index 28f1d84..fe30f18 100644
--- a/chrome/browser/in_process_webkit/dom_storage_context.cc
+++ b/chrome/browser/in_process_webkit/dom_storage_context.cc
@@ -14,12 +14,35 @@
 #include "chrome/browser/in_process_webkit/dom_storage_namespace.h"
 #include "chrome/browser/in_process_webkit/webkit_context.h"
 #include "chrome/common/dom_storage_common.h"
+#include "chrome/common/url_constants.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebSecurityOrigin.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebString.h"
 #include "webkit/glue/webkit_glue.h"
 
 using WebKit::WebSecurityOrigin;
 
+namespace {
+
+void ClearLocalState(const FilePath& domstorage_path,
+                     const char* url_scheme_to_be_skipped) {
+  file_util::FileEnumerator file_enumerator(
+      domstorage_path, false, file_util::FileEnumerator::FILES);
+  for (FilePath file_path = file_enumerator.Next(); !file_path.empty();
+       file_path = file_enumerator.Next()) {
+    if (file_path.Extension() == DOMStorageContext::kLocalStorageExtension) {
+      WebSecurityOrigin web_security_origin =
+          WebSecurityOrigin::createFromDatabaseIdentifier(
+              webkit_glue::FilePathToWebString(file_path.BaseName()));
+      if (!EqualsASCII(web_security_origin.protocol(),
+                       url_scheme_to_be_skipped)) {
+        file_util::Delete(file_path, false);
+      }
+    }
+  }
+}
+
+}  // namespace
+
 const FilePath::CharType DOMStorageContext::kLocalStorageDirectory[] =
     FILE_PATH_LITERAL("Local Storage");
 
@@ -44,7 +67,8 @@ DOMStorageContext::DOMStorageContext(WebKitContext* webkit_context)
     : last_storage_area_id_(0),
       last_session_storage_namespace_id_on_ui_thread_(kLocalStorageNamespaceId),
       last_session_storage_namespace_id_on_io_thread_(kLocalStorageNamespaceId),
-      webkit_context_(webkit_context) {
+      clear_local_state_on_exit_(false) {
+  data_path_ = webkit_context->data_path();
 }
 
 DOMStorageContext::~DOMStorageContext() {
@@ -56,6 +80,14 @@ DOMStorageContext::~DOMStorageContext() {
        iter != storage_namespace_map_.end(); ++iter) {
     delete iter->second;
   }
+
+  // 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_.Append(kLocalStorageDirectory),
+                    chrome::kExtensionScheme);
+  }
 }
 
 int64 DOMStorageContext::AllocateStorageAreaId() {
@@ -167,7 +199,7 @@ void DOMStorageContext::DeleteDataModifiedSince(
   PurgeMemory();
 
   file_util::FileEnumerator file_enumerator(
-      webkit_context_->data_path().Append(kLocalStorageDirectory), false,
+      data_path_.Append(kLocalStorageDirectory), false,
       file_util::FileEnumerator::FILES);
   for (FilePath path = file_enumerator.Next(); !path.value().empty();
        path = file_enumerator.Next()) {
@@ -215,7 +247,7 @@ void DOMStorageContext::DeleteAllLocalStorageFiles() {
   PurgeMemory();
 
   file_util::FileEnumerator file_enumerator(
-      webkit_context_->data_path().Append(kLocalStorageDirectory), false,
+      data_path_.Append(kLocalStorageDirectory), false,
       file_util::FileEnumerator::FILES);
   for (FilePath file_path = file_enumerator.Next(); !file_path.empty();
        file_path = file_enumerator.Next()) {
@@ -225,11 +257,10 @@ void DOMStorageContext::DeleteAllLocalStorageFiles() {
 }
 
 DOMStorageNamespace* DOMStorageContext::CreateLocalStorage() {
-  FilePath data_path = webkit_context_->data_path();
   FilePath dir_path;
-  if (!data_path.empty()) {
-    MigrateLocalStorageDirectory(data_path);
-    dir_path = data_path.Append(kLocalStorageDirectory);
+  if (!data_path_.empty()) {
+    MigrateLocalStorageDirectory(data_path_);
+    dir_path = data_path_.Append(kLocalStorageDirectory);
   }
   DOMStorageNamespace* new_namespace =
       DOMStorageNamespace::CreateLocalStorageNamespace(this, dir_path);
@@ -264,28 +295,9 @@ void DOMStorageContext::CompleteCloningSessionStorage(
     context->RegisterStorageNamespace(existing_namespace->Copy(clone_id));
 }
 
-// static
-void DOMStorageContext::ClearLocalState(const FilePath& profile_path,
-                                        const char* url_scheme_to_be_skipped) {
-  file_util::FileEnumerator file_enumerator(profile_path.Append(
-      kLocalStorageDirectory), false, file_util::FileEnumerator::FILES);
-  for (FilePath file_path = file_enumerator.Next(); !file_path.empty();
-       file_path = file_enumerator.Next()) {
-    if (file_path.Extension() == kLocalStorageExtension) {
-      WebSecurityOrigin web_security_origin =
-          WebSecurityOrigin::createFromDatabaseIdentifier(
-              webkit_glue::FilePathToWebString(file_path.BaseName()));
-      if (!EqualsASCII(web_security_origin.protocol(),
-                       url_scheme_to_be_skipped)) {
-        file_util::Delete(file_path, false);
-      }
-    }
-  }
-}
-
 FilePath DOMStorageContext::GetLocalStorageFilePath(
     const string16& origin_id) const {
-  FilePath storageDir = webkit_context_->data_path().Append(
+  FilePath storageDir = data_path_.Append(
       DOMStorageContext::kLocalStorageDirectory);
   FilePath::StringType id =
       webkit_glue::WebStringToFilePathString(origin_id);
diff --git a/chrome/browser/in_process_webkit/dom_storage_context.h b/chrome/browser/in_process_webkit/dom_storage_context.h
index 1552a918..49ff1f5 100644
--- a/chrome/browser/in_process_webkit/dom_storage_context.h
+++ b/chrome/browser/in_process_webkit/dom_storage_context.h
@@ -85,13 +85,18 @@ class DOMStorageContext {
   // The local storage file extension.
   static const FilePath::CharType kLocalStorageExtension[];
 
-  // Delete all non-extension local storage files.
-  static void ClearLocalState(const FilePath& profile_path,
-                              const char* url_scheme_to_be_skipped);
-
   // Get the file name of the local storage file for the given origin.
   FilePath GetLocalStorageFilePath(const string16& origin_id) const;
 
+  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 to override the |data_path_|.
+  void set_data_path(const FilePath& data_path) { data_path_ = data_path; }
+#endif
+
  private:
   // Get the local storage instance.  The object is owned by this class.
   DOMStorageNamespace* CreateLocalStorage();
@@ -118,8 +123,15 @@ class DOMStorageContext {
   int64 last_session_storage_namespace_id_on_ui_thread_;
   int64 last_session_storage_namespace_id_on_io_thread_;
 
-  // We're owned by this WebKit context.  Used while instantiating LocalStorage.
-  WebKitContext* webkit_context_;
+  // True if the destructor should delete its files.
+  bool clear_local_state_on_exit_;
+
+  // Path where the profile data is stored.
+  // TODO(pastarmovj): Keep in mind that unlike indexed db data_path_ variable
+  // this one still has to point to the upper level dir because of the
+  // MigrateLocalStorageDirectory function. Once this function disappears we can
+  // make it point directly to the dom storage path.
+  FilePath data_path_;
 
   // All the DOMStorageDispatcherHosts that are attached to us. ONLY USE ON THE
   // IO THREAD!
diff --git a/chrome/browser/in_process_webkit/webkit_context.cc b/chrome/browser/in_process_webkit/webkit_context.cc
index 358c80f..0f3c397 100644
--- a/chrome/browser/in_process_webkit/webkit_context.cc
+++ b/chrome/browser/in_process_webkit/webkit_context.cc
@@ -23,6 +23,8 @@ WebKitContext::~WebKitContext() {
   // If the WebKit thread was ever spun up, delete the object there.  The task
   // will just get deleted if the WebKit thread isn't created (which only
   // happens during testing).
+  dom_storage_context_->set_clear_local_state_on_exit_(
+      clear_local_state_on_exit_);
   DOMStorageContext* dom_storage_context = dom_storage_context_.release();
   if (!BrowserThread::DeleteSoon(
           BrowserThread::WEBKIT, FROM_HERE, dom_storage_context)) {
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 289ab1a..6289d10 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -2066,6 +2066,7 @@
         'browser/gtk/view_id_util_browsertest.cc',
         'browser/history/history_browsertest.cc',
         'browser/idbbindingutilities_browsertest.cc',
+        'browser/in_process_webkit/dom_storage_browsertest.cc',
         'browser/in_process_webkit/indexed_db_browsertest.cc',
         'browser/net/cookie_policy_browsertest.cc',
         'browser/net/ftp_browsertest.cc',