Move code to clear web databases on shutdown to the database tracker

webkit/database/database_tracker.cc

Index: webkit/database/database_tracker.cc
diff --git a/webkit/database/database_tracker.cc b/webkit/database/database_tracker.cc
index 7497c3814c7262279eeac8a242b42b1c921505d1..e4228be320e9866161124e231f602e1817841a28 100644
--- a/webkit/database/database_tracker.cc
+++ b/webkit/database/database_tracker.cc
@@ -15,6 +15,7 @@
 #include "base/basictypes.h"
 #include "base/file_util.h"
 #include "base/message_loop_proxy.h"
+#include "base/platform_file.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "net/base/net_errors.h"
@@ -47,7 +48,6 @@ const FilePath::CharType kTrackerDatabaseFileName[] =
     FILE_PATH_LITERAL("Databases.db");
 static const int kCurrentVersion = 2;
 static const int kCompatibleVersion = 1;
-static const char* kExtensionOriginIdentifierPrefix = "chrome-extension_";
 
 OriginInfo::OriginInfo()
     : total_size_(0) {}
@@ -87,11 +87,13 @@ OriginInfo::OriginInfo(const string16& origin, int64 total_size)
 DatabaseTracker::DatabaseTracker(
     const FilePath& profile_path,
     bool is_incognito,
+    bool clear_local_state_on_exit,
     quota::SpecialStoragePolicy* special_storage_policy,
     quota::QuotaManagerProxy* quota_manager_proxy,
     base::MessageLoopProxy* db_tracker_thread)
     : is_initialized_(false),
       is_incognito_(is_incognito),
+      clear_local_state_on_exit_(clear_local_state_on_exit),
       shutting_down_(false),
       profile_path_(profile_path),
       db_dir_(is_incognito_ ?
@@ -112,6 +114,33 @@ DatabaseTracker::DatabaseTracker(
 DatabaseTracker::~DatabaseTracker() {
   DCHECK(dbs_to_be_deleted_.empty());
   DCHECK(deletion_callbacks_.empty());
+
+  if (clear_local_state_on_exit_ && LazyInit()) {

[michaeln, 2011/06/22 20:15:33]

Thought of another issue that has to be considered.

This dtor is not guaranteed to be run on any particular thread. Upon true
browser shutdown, it'd probably be ok to do this on the IO thread, but in the
multiprofile case where a profile can go away while the browser is still
running... we probably have to ensure this logic is executed on the
db_tracker_thread (that's the FILE thread in chrome).

[jochen (gone - plz use gerrit), 2011/06/22 21:44:12]

The profiles (which own the db tracker) are deleted before the threads are taken
down, so we should posttask this to the db thread

[michaeln, 2011/06/22 22:35:14]

DatabaseMessageFilters also hold references to this object so it's not strictly
"owned". I'm not real found of DeleteOnThisOrThat thread trait, but it might be
an option for this class to get it deleted on the FILE thread.

Marja is looking at posting a task to the AppcacheService for this "delete on
exit" purpose too. Maybe the profile just needs to post a task to set the
"clear_on_exit" flag (no need to set the flag until then really) and the dtor
can take it from there.

[jochen (gone - plz use gerrit), 2011/06/28 10:01:24]

Done.

+    std::vector<string16> origin_identifiers;
+    databases_table_->GetAllOrigins(&origin_identifiers);
+
+    for (std::vector<string16>::iterator origin = origin_identifiers.begin();
+         origin != origin_identifiers.end(); ++origin) {
+      if (special_storage_policy_.get() &&
+          special_storage_policy_->IsStorageProtected(
+              DatabaseUtil::GetOriginFromIdentifier(*origin))) {
+        continue;
+      }
+      std::vector<DatabaseDetails> details;
+      databases_table_->GetAllDatabaseDetailsForOrigin(*origin, &details);
+      for (std::vector<DatabaseDetails>::iterator database = details.begin();
+           database != details.end(); ++database) {
+        base::PlatformFile file_handle = base::CreatePlatformFile(
+            GetFullDBFilePath(*origin, database->database_name),
+            base::PLATFORM_FILE_OPEN_ALWAYS |
+            base::PLATFORM_FILE_DELETE_ON_CLOSE |
+            base::PLATFORM_FILE_READ,
+            NULL, NULL);
+        base::ClosePlatformFile(file_handle);

[michaeln, 2011/06/22 19:43:28]

I'm not sure this will actually result in the file's deletion IF there are other
handles open at the time this handle is closed? If you don't use windows
regularly, i can compose a test and let you know of the results.

[michaeln, 2011/06/22 20:15:33]

Hmmm... if we were guaranteed that the IPC channel and it's associated
MessageFilters like the DatabaseMessageFilter weren't deleted until the child
process had terminated, we should be able to safely these files (or rather, open
handles in renderers or workers would not get in the way of deletion).

There's the OnChannelClosing() method, but i don't think that guarantees the
remote process has actually exitted yet.

[jochen (gone - plz use gerrit), 2011/06/22 21:44:12]

With the change in vfs_backend.cc, you can mark open files as delete on close.
If the last file handle to the file is closed, it will be deleted.

[michaeln, 2011/06/22 22:35:14]

But what if the last file handle to go is not one that was opened with the
DELETE_ON_CLOSE flag? If the file is still deleted in that case, great... but is
that the case?

[jochen (gone - plz use gerrit), 2011/06/28 10:01:24]

if the file was originally opened with SHARE_DELETE, this is ok

+      }
+      databases_table_->DeleteOrigin(*origin);

[michaeln, 2011/06/22 19:43:28]

As a matter of privacy, i think we need to delete the directory for the origin
as well since the host can be determined by the name.

[jochen (gone - plz use gerrit), 2011/06/22 21:44:12]

that is, however, more difficult because the files in the directory might not be
deleted immediately. I could rename them if I can't delete them, wdyt?

[michaeln, 2011/06/22 22:35:14]

we'd have to move them in order to delete the directory i suppose

[jochen (gone - plz use gerrit), 2011/06/28 10:01:24]

Done.

+    }
+  }
 }
 
 void DatabaseTracker::DatabaseOpened(const string16& origin_identifier,
@@ -747,44 +776,8 @@ void DatabaseTracker::DeleteIncognitoDBDirectory() {
     file_util::Delete(incognito_db_dir, true);
 }
 
-// static
-void DatabaseTracker::ClearLocalState(const FilePath& profile_path) {
-  // TODO(michaeln): use SpecialStoragePolicy instead of kExtensionOriginPrefix
-  FilePath db_dir = profile_path.Append(FilePath(kDatabaseDirectoryName));
-  FilePath db_tracker = db_dir.Append(FilePath(kTrackerDatabaseFileName));
-  if (file_util::DirectoryExists(db_dir) &&
-      file_util::PathExists(db_tracker)) {
-    scoped_ptr<sql::Connection> db_(new sql::Connection);
-    if (!db_->Open(db_tracker) ||
-        !db_->DoesTableExist("Databases")) {
-      db_->Close();
-      file_util::Delete(db_dir, true);
-      return;
-    } else {
-      sql::Statement delete_statement(db_->GetCachedStatement(
-            SQL_FROM_HERE, "DELETE FROM Databases WHERE origin NOT LIKE ?"));
-      std::string filter(kExtensionOriginIdentifierPrefix);
-      filter += "%";
-      delete_statement.BindString(0, filter);
-      if (!delete_statement.Run()) {
-        db_->Close();
-        file_util::Delete(db_dir, true);
-        return;
-      }
-    }
-  }
-  file_util::FileEnumerator file_enumerator(db_dir, false,
-      file_util::FileEnumerator::DIRECTORIES);
-  for (FilePath file_path = file_enumerator.Next(); !file_path.empty();
-       file_path = file_enumerator.Next()) {
-    if (file_path.BaseName() != FilePath(kTrackerDatabaseFileName)) {
-      std::string basename = file_path.BaseName().MaybeAsASCII();
-      if (!basename.empty() &&
-          !StartsWithASCII(basename, kExtensionOriginIdentifierPrefix, true)) {
-        file_util::Delete(file_path, true);
-      }
-    }
-  }
+void DatabaseTracker::SetClearLocalStateOnExit(bool clear_local_state_on_exit) {
+  clear_local_state_on_exit_ = clear_local_state_on_exit;
 }
 
 }  // namespace webkit_database