Use PostDelayTask instead of timer to drop DB.

BUG=240621

Review URL: https://chromiumcodereview.appspot.com/15443002

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

4 files changed, 145 insertions, 21 deletions

diff --git a/webkit/browser/fileapi/obfuscated_file_util.cc b/webkit/browser/fileapi/obfuscated_file_util.cc
index 5346b93..0fe7776 100644
--- a/webkit/browser/fileapi/obfuscated_file_util.cc
+++ b/webkit/browser/fileapi/obfuscated_file_util.cc
@@ -16,6 +16,7 @@
 #include "base/stringprintf.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/sys_string_conversions.h"
+#include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "googleurl/src/gurl.h"
 #include "webkit/base/origin_url_conversions.h"
@@ -47,8 +48,6 @@ namespace {
 typedef SandboxDirectoryDatabase::FileId FileId;
 typedef SandboxDirectoryDatabase::FileInfo FileInfo;
 
-const int64 kFlushDelaySeconds = 10 * 60;  // 10 minutes
-
 void InitFileInfo(
     SandboxDirectoryDatabase::FileInfo* file_info,
     SandboxDirectoryDatabase::FileId parent_id,
@@ -96,6 +95,49 @@ void TouchDirectory(SandboxDirectoryDatabase* db, FileId dir_id) {
     NOTREACHED();
 }
 
+class Deleter {
+ public:
+  explicit Deleter(ObfuscatedFileUtil* util) : util_(util) {}
+
+  void operator()(bool* ptr) const {
+    if (*ptr)
+      util_->ResetObjectLifetimeTracker();
+    delete ptr;
+  }
+
+ private:
+  ObfuscatedFileUtil* util_;
+};
+
+void MaybeDropDatabases(
+    ObfuscatedFileUtil* util,
+    scoped_refptr<base::SequencedTaskRunner> file_task_runner,
+    scoped_ptr<bool, Deleter> object_still_alive,
+    int64 db_flush_delay_seconds) {
+  // If object isn't alive, DB has already been dropped by dtor.
+  if (!*object_still_alive)
+    return;
+
+  // Check to see if DB was recently used. If yes, restart timer so it triggers
+  // again in kFlushDelaySeconds from the last time it was used.
+  base::TimeDelta last_used_delta =
+      base::TimeTicks::Now() - util->db_last_use_time();
+  int64 next_timer_delta = db_flush_delay_seconds - last_used_delta.InSeconds();
+  if (next_timer_delta > 0) {
+    file_task_runner->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&MaybeDropDatabases,
+                   base::Unretained(util),
+                   file_task_runner,
+                   base::Passed(&object_still_alive),
+                   db_flush_delay_seconds),
+        base::TimeDelta::FromSeconds(next_timer_delta));
+    return;
+  }
+
+  util->DropDatabases();
+}
+
 const base::FilePath::CharType kTemporaryDirectoryName[] = FILE_PATH_LITERAL("t");
 const base::FilePath::CharType kPersistentDirectoryName[] = FILE_PATH_LITERAL("p");
 const base::FilePath::CharType kSyncableDirectoryName[] = FILE_PATH_LITERAL("s");
@@ -255,12 +297,20 @@ class ObfuscatedOriginEnumerator
 
 ObfuscatedFileUtil::ObfuscatedFileUtil(
     quota::SpecialStoragePolicy* special_storage_policy,
-    const base::FilePath& file_system_directory)
+    const base::FilePath& file_system_directory,
+    base::SequencedTaskRunner* file_task_runner)
     : special_storage_policy_(special_storage_policy),
-      file_system_directory_(file_system_directory) {
+      file_system_directory_(file_system_directory),
+      file_task_runner_(file_task_runner),
+      db_flush_delay_seconds_(10 * 60), // 10 mins.
+      object_lifetime_tracker_(NULL) {
 }
 
 ObfuscatedFileUtil::~ObfuscatedFileUtil() {
+  // Mark as deleted so that the callback doesn't run on invalid object.
+  if (object_lifetime_tracker_)
+    *object_lifetime_tracker_ = false;
+
   DropDatabases();
 }
 
@@ -989,6 +1039,17 @@ bool ObfuscatedFileUtil::DestroyDirectoryDatabase(
   return SandboxDirectoryDatabase::DestroyDatabase(path);
 }
 
+void ObfuscatedFileUtil::ResetObjectLifetimeTracker() {
+  object_lifetime_tracker_ = NULL;
+}
+
+void ObfuscatedFileUtil::DropDatabases() {
+  origin_database_.reset();
+  STLDeleteContainerPairSecondPointers(
+      directories_.begin(), directories_.end());
+  directories_.clear();
+}
+
 // static
 int64 ObfuscatedFileUtil::ComputeFilePathCost(const base::FilePath& path) {
   return UsageForPath(VirtualPath::BaseName(path).value().size());
@@ -1279,18 +1340,24 @@ void ObfuscatedFileUtil::InvalidateUsageCache(
 }
 
 void ObfuscatedFileUtil::MarkUsed() {
-  if (timer_.IsRunning())
-    timer_.Reset();
-  else
-    timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(kFlushDelaySeconds),
-                 this, &ObfuscatedFileUtil::DropDatabases);
-}
+  db_last_use_time_ = base::TimeTicks::Now();
 
-void ObfuscatedFileUtil::DropDatabases() {
-  origin_database_.reset();
-  STLDeleteContainerPairSecondPointers(
-      directories_.begin(), directories_.end());
-  directories_.clear();
+  // If object_lifetime_tracker_ is valid, then callback timer already running.
+  if (object_lifetime_tracker_)
+    return;
+
+  // Initialize object lifetime tracker for the first time.
+  object_lifetime_tracker_ = new bool(true);
+
+  scoped_ptr<bool, Deleter> scoper(object_lifetime_tracker_, Deleter(this));
+  file_task_runner_->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&MaybeDropDatabases,
+                 base::Unretained(this),
+                 file_task_runner_,
+                 base::Passed(&scoper),
+                 db_flush_delay_seconds_),
+      base::TimeDelta::FromSeconds(db_flush_delay_seconds_));
 }
 
 bool ObfuscatedFileUtil::InitOriginDatabase(bool create) {
diff --git a/webkit/browser/fileapi/obfuscated_file_util.h b/webkit/browser/fileapi/obfuscated_file_util.h
index 7cd5c59..680349e 100644
--- a/webkit/browser/fileapi/obfuscated_file_util.h
+++ b/webkit/browser/fileapi/obfuscated_file_util.h
@@ -10,9 +10,9 @@
 
 #include "base/files/file_path.h"
 #include "base/files/file_util_proxy.h"
+#include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/platform_file.h"
-#include "base/timer.h"
 #include "webkit/browser/fileapi/file_system_file_util.h"
 #include "webkit/browser/fileapi/file_system_url.h"
 #include "webkit/browser/fileapi/sandbox_directory_database.h"
@@ -21,7 +21,8 @@
 #include "webkit/storage/webkit_storage_export.h"
 
 namespace base {
-class Time;
+class SequencedTaskRunner;
+class TimeTicks;
 }
 
 namespace quota {
@@ -61,7 +62,8 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
 
   ObfuscatedFileUtil(
       quota::SpecialStoragePolicy* special_storage_policy,
-      const base::FilePath& file_system_directory);
+      const base::FilePath& file_system_directory,
+      base::SequencedTaskRunner* file_task_runner);
   virtual ~ObfuscatedFileUtil();
 
   // FileSystemFileUtil overrides.
@@ -169,6 +171,12 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
   // and destroys the database on the disk.
   bool DestroyDirectoryDatabase(const GURL& origin, FileSystemType type);
 
+  void ResetObjectLifetimeTracker();
+
+  void DropDatabases();
+
+  const base::TimeTicks& db_last_use_time() const { return db_last_use_time_; }
+
   // Computes a cost for storing a given file in the obfuscated FSFU.
   // As the cost of a file is independent of the cost of its parent directories,
   // this ignores all but the BaseName of the supplied path.  In order to
@@ -183,6 +191,9 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
   typedef SandboxDirectoryDatabase::FileInfo FileInfo;
 
   friend class ObfuscatedFileEnumerator;
+  FRIEND_TEST_ALL_PREFIXES(ObfuscatedFileUtilTest, MaybeDropDatabasesAliveCase);
+  FRIEND_TEST_ALL_PREFIXES(ObfuscatedFileUtilTest,
+                           MaybeDropDatabasesAlreadyDeletedCase);
 
   base::PlatformFileError GetFileInfoInternal(
       SandboxDirectoryDatabase* db,
@@ -241,7 +252,6 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
                             FileSystemType type);
 
   void MarkUsed();
-  void DropDatabases();
   bool InitOriginDatabase(bool create);
 
   base::PlatformFileError GenerateNewLocalPath(
@@ -263,7 +273,17 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
   scoped_ptr<SandboxOriginDatabaseInterface> origin_database_;
   scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy_;
   base::FilePath file_system_directory_;
-  base::OneShotTimer<ObfuscatedFileUtil> timer_;
+  scoped_refptr<base::SequencedTaskRunner> file_task_runner_;
+
+  // Used to delete database after a certain period of inactivity.
+  int64 db_flush_delay_seconds_;
+  base::TimeTicks db_last_use_time_;
+
+  // Owned by MaybeDropDatabase callback, set to false when dtor runs.
+  // This only becomes valid when the PostDelayedTask callback is posted and
+  // becomes invalid again if the PostDelayedTask callback finishes. (i.e. timer
+  // runs out).
+  bool* object_lifetime_tracker_;
 
   // If this instance is initialized for an isolated partition, this should
   // only see a single origin.
diff --git a/webkit/browser/fileapi/obfuscated_file_util_unittest.cc b/webkit/browser/fileapi/obfuscated_file_util_unittest.cc
index f1fa1b1..93a4015 100644
--- a/webkit/browser/fileapi/obfuscated_file_util_unittest.cc
+++ b/webkit/browser/fileapi/obfuscated_file_util_unittest.cc
@@ -25,6 +25,7 @@
 #include "webkit/browser/fileapi/mock_file_system_context.h"
 #include "webkit/browser/fileapi/obfuscated_file_util.h"
 #include "webkit/browser/fileapi/sandbox_file_system_test_helper.h"
+#include "webkit/browser/fileapi/sandbox_origin_database.h"
 #include "webkit/browser/fileapi/test_file_set.h"
 #include "webkit/browser/quota/mock_special_storage_policy.h"
 #include "webkit/browser/quota/quota_manager.h"
@@ -2251,4 +2252,39 @@ TEST_F(ObfuscatedFileUtilTest, TestQuotaOnOpen) {
   EXPECT_TRUE(base::ClosePlatformFile(file_handle));
 }
 
+TEST_F(ObfuscatedFileUtilTest, MaybeDropDatabasesAliveCase) {
+  base::ScopedTempDir data_dir;
+  ASSERT_TRUE(data_dir.CreateUniqueTempDir());
+  ObfuscatedFileUtil file_util(NULL,
+                               data_dir.path(),
+                               base::MessageLoopProxy::current());
+  file_util.InitOriginDatabase(true /*create*/);
+  ASSERT_TRUE(file_util.origin_database_ != NULL);
+
+  // Callback to Drop DB is called while ObfuscatedFileUtilTest is still alive.
+  file_util.db_flush_delay_seconds_ = 0;
+  file_util.MarkUsed();
+  base::MessageLoop::current()->RunUntilIdle();
+
+  ASSERT_TRUE(file_util.origin_database_ == NULL);
+}
+
+TEST_F(ObfuscatedFileUtilTest, MaybeDropDatabasesAlreadyDeletedCase) {
+  // Run message loop after OFU is already deleted to make sure callback doesn't
+  // cause a crash for use after free.
+  {
+    base::ScopedTempDir data_dir;
+    ASSERT_TRUE(data_dir.CreateUniqueTempDir());
+    ObfuscatedFileUtil file_util(NULL,
+                                 data_dir.path(),
+                                 base::MessageLoopProxy::current());
+    file_util.InitOriginDatabase(true /*create*/);
+    file_util.db_flush_delay_seconds_ = 0;
+    file_util.MarkUsed();
+  }
+
+  // At this point the callback is still in the message queue but OFU is gone.
+  base::MessageLoop::current()->RunUntilIdle();
+}
+
 }  // namespace fileapi
diff --git a/webkit/browser/fileapi/sandbox_mount_point_provider.cc b/webkit/browser/fileapi/sandbox_mount_point_provider.cc
index 0612ad8..f448aaa 100644
--- a/webkit/browser/fileapi/sandbox_mount_point_provider.cc
+++ b/webkit/browser/fileapi/sandbox_mount_point_provider.cc
@@ -155,7 +155,8 @@ SandboxMountPointProvider::SandboxMountPointProvider(
           new AsyncFileUtilAdapter(
               new ObfuscatedFileUtil(
                   special_storage_policy,
-                  profile_path.Append(kFileSystemDirectory)))),
+                  profile_path.Append(kFileSystemDirectory),
+                  file_task_runner))),
       file_system_usage_cache_(new FileSystemUsageCache(file_task_runner)),
       quota_observer_(new SandboxQuotaObserver(
                       quota_manager_proxy,