websql: Document thread-safe members of storage::DatabaseTracker.

This CL documents the thread-safety arguments for the thread-safe
DatabaseTracker members mentioned in the class-level comments. It makes
a member const, so that accessing it is indeed thread-safe, and renames
a getter to hacker_case, to make it slightly more obvious that it's just
accessing a const member.

Change-Id: I9b8aa97ef84c504ea6b421de1091d35cf06a48c1
Reviewed-on: https://chromium-review.googlesource.com/1137983
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575455}

content/browser/renderer_host/web_database_host_impl.cc

@@ -91,8 +91,8 @@ void WebDatabaseHostImpl::OpenFile(const base::string16& vfs_file_name,
   // open handles to them in the database tracker to make sure they're
   // around for as long as needed.
   if (vfs_file_name.empty()) {
-    file = VfsBackend::OpenTempFileInDirectory(db_tracker_->DatabaseDirectory(),
-                                               desired_flags);
+    file = VfsBackend::OpenTempFileInDirectory(
+        db_tracker_->database_directory(), desired_flags);
   } else if (DatabaseUtil::CrackVfsFileName(vfs_file_name, &origin_identifier,
                                             &database_name, nullptr) &&
              !db_tracker_->IsDatabaseScheduledForDeletion(origin_identifier,

storage/browser/database/database_tracker.h

@@ -93,7 +93,7 @@ class STORAGE_EXPORT DatabaseTracker
         const base::string16& database_name) = 0;
 
    protected:
-    virtual ~Observer() {}
+    virtual ~Observer() = default;
   };
 
   DatabaseTracker(const base::FilePath& profile_path,
@@ -121,7 +121,9 @@ class STORAGE_EXPORT DatabaseTracker
 
   void CloseTrackerDatabaseAndClearCaches();
 
-  const base::FilePath& DatabaseDirectory() const { return db_dir_; }
+  // Thread-safe getter.
+  const base::FilePath& database_directory() const { return db_dir_; }
+
   base::FilePath GetFullDBFilePath(const std::string& origin_identifier,
                                    const base::string16& database_name);
 
@@ -130,7 +132,7 @@ class STORAGE_EXPORT DatabaseTracker
   virtual bool GetAllOriginIdentifiers(std::vector<std::string>* origin_ids);
   virtual bool GetAllOriginsInfo(std::vector<OriginInfo>* origins_info);
 
-  // Safe to call on any thread.
+  // Thread-safe getter.
   storage::QuotaManagerProxy* quota_manager_proxy() const {
     return quota_manager_proxy_.get();
   }
@@ -281,7 +283,12 @@ class STORAGE_EXPORT DatabaseTracker
   bool force_keep_session_state_ = false;
   bool shutting_down_ = false;
   const base::FilePath profile_path_;
+
+  // Can be accessed from any thread via database_directory().
+  //
+  // Thread-safety argument: The member is immutable.
   const base::FilePath db_dir_;
+
   std::unique_ptr<sql::Connection> db_;
   std::unique_ptr<DatabasesTable> databases_table_;
   std::unique_ptr<sql::MetaTable> meta_table_;
@@ -294,9 +301,12 @@ class STORAGE_EXPORT DatabaseTracker
   PendingDeletionCallbacks deletion_callbacks_;
 
   // Apps and Extensions can have special rights.
-  scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy_;
+  const scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy_;
 
-  scoped_refptr<storage::QuotaManagerProxy> quota_manager_proxy_;
+  // Can be accessed from any thread via quota_manager_proxy().
+  //
+  // Thread-safety argument: The reference is immutable.
+  const scoped_refptr<storage::QuotaManagerProxy> quota_manager_proxy_;
 
   // The database tracker thread we're supposed to run file IO on.
   scoped_refptr<base::SequencedTaskRunner> task_runner_;

storage/browser/database/database_tracker_unittest.cc

@@ -218,10 +218,10 @@ class DatabaseTracker_TestHelper_Test {
     tracker->DatabaseOpened(kOrigin2, kDB3, kDescription, 0, &database_size);
 
     EXPECT_TRUE(base::CreateDirectory(
-        tracker->DatabaseDirectory().Append(base::FilePath::FromUTF16Unsafe(
+        tracker->database_directory().Append(base::FilePath::FromUTF16Unsafe(
             tracker->GetOriginDirectory(kOrigin1)))));
     EXPECT_TRUE(base::CreateDirectory(
-        tracker->DatabaseDirectory().Append(base::FilePath::FromUTF16Unsafe(
+        tracker->database_directory().Append(base::FilePath::FromUTF16Unsafe(
             tracker->GetOriginDirectory(kOrigin2)))));
     EXPECT_EQ(
         1, base::WriteFile(tracker->GetFullDBFilePath(kOrigin1, kDB1), "a", 1));
@@ -247,12 +247,12 @@ class DatabaseTracker_TestHelper_Test {
     result = callback.GetResult(result);
     EXPECT_EQ(net::OK, result);
     EXPECT_FALSE(
-        base::PathExists(tracker->DatabaseDirectory().AppendASCII(kOrigin1)));
+        base::PathExists(tracker->database_directory().AppendASCII(kOrigin1)));
 
     // Recreate db1.
     tracker->DatabaseOpened(kOrigin1, kDB1, kDescription, 0, &database_size);
     EXPECT_TRUE(base::CreateDirectory(
-        tracker->DatabaseDirectory().Append(base::FilePath::FromUTF16Unsafe(
+        tracker->database_directory().Append(base::FilePath::FromUTF16Unsafe(
             tracker->GetOriginDirectory(kOrigin1)))));
     EXPECT_EQ(
         1, base::WriteFile(tracker->GetFullDBFilePath(kOrigin1, kDB1), "a", 1));
@@ -281,7 +281,7 @@ class DatabaseTracker_TestHelper_Test {
     result = callback.GetResult(result);
     EXPECT_EQ(net::OK, result);
     EXPECT_FALSE(
-        base::PathExists(tracker->DatabaseDirectory().AppendASCII(kOrigin1)));
+        base::PathExists(tracker->database_directory().AppendASCII(kOrigin1)));
     EXPECT_TRUE(base::PathExists(tracker->GetFullDBFilePath(kOrigin2, kDB2)));
     EXPECT_TRUE(base::PathExists(tracker->GetFullDBFilePath(kOrigin2, kDB3)));
 
@@ -339,10 +339,10 @@ class DatabaseTracker_TestHelper_Test {
     // Write some data to each file and check that the listeners are
     // called with the appropriate values.
     EXPECT_TRUE(base::CreateDirectory(
-        tracker->DatabaseDirectory().Append(base::FilePath::FromUTF16Unsafe(
+        tracker->database_directory().Append(base::FilePath::FromUTF16Unsafe(
             tracker->GetOriginDirectory(kOrigin1)))));
     EXPECT_TRUE(base::CreateDirectory(
-        tracker->DatabaseDirectory().Append(base::FilePath::FromUTF16Unsafe(
+        tracker->database_directory().Append(base::FilePath::FromUTF16Unsafe(
             tracker->GetOriginDirectory(kOrigin2)))));
     EXPECT_EQ(
         1, base::WriteFile(tracker->GetFullDBFilePath(kOrigin1, kDB1), "a", 1));