Improve IndexedDB's quota support

content/browser/in_process_webkit/indexed_db_context.cc

Index: content/browser/in_process_webkit/indexed_db_context.cc
diff --git a/content/browser/in_process_webkit/indexed_db_context.cc b/content/browser/in_process_webkit/indexed_db_context.cc
index 8e091c1678572dca7c3192b74addfff3e3a338cf..c027e85a3eb76e5ec410908e13037868c13322fb 100644
--- a/content/browser/in_process_webkit/indexed_db_context.cc
+++ b/content/browser/in_process_webkit/indexed_db_context.cc
@@ -8,6 +8,7 @@
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "base/string_util.h"
+#include "base/task.h"
 #include "base/utf_string_conversions.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/in_process_webkit/indexed_db_quota_client.h"
@@ -18,10 +19,12 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebIDBFactory.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h"
+#include "webkit/database/database_util.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/quota/quota_manager.h"
 #include "webkit/quota/special_storage_policy.h"
 
+using webkit_database::DatabaseUtil;
 using WebKit::WebIDBDatabase;
 using WebKit::WebIDBFactory;
 using WebKit::WebSecurityOrigin;
@@ -103,7 +106,7 @@ FilePath IndexedDBContext::GetIndexedDBFilePath(
 void IndexedDBContext::DeleteIndexedDBFile(const FilePath& file_path) {

[michaeln, 2011/08/04 00:23:27]

I don't see any external callers of this method, it would be good to make it
private (or even remove it and replace the callsites with a call to
file_util::Delete()). The reason being the origin info is buried in the
file_path and to report storage modifications we'd have to crack that path and
pull it out... but thats just busy work... would be better if external callers
expressed what they would like deleted in terms of 'origin' instead of
'file_paths'.

[dgrogan, 2011/08/04 19:47:50]

Done.

   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
   // TODO(pastarmovj): Close all database connections that use that file.
-  file_util::Delete(file_path, false);
+  file_util::Delete(file_path, true /*recursive*/);
 }
 
 void IndexedDBContext::DeleteIndexedDBForOrigin(const string16& origin_id) {
@@ -115,6 +118,17 @@ void IndexedDBContext::DeleteIndexedDBForOrigin(const string16& origin_id) {
   DeleteIndexedDBFile(GetIndexedDBFilePath(origin_id));
 }
 
+// TODO(dgrogan): Once we can delete IndexedDB directories out from underneath
+// open webkit instances, merge this and DeleteIndexedDBForOrigin.
+void IndexedDBContext::EvictOriginIfNotInUse(const GURL& origin_url) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
+  if (connection_count_.find(origin_url) == connection_count_.end()) {
+    string16 origin_id = DatabaseUtil::GetOriginIdentifier(origin_url);
+    FilePath idb_directory = GetIndexedDBFilePath(origin_id);
+    DeleteIndexedDBFile(idb_directory);
+  }

[michaeln, 2011/08/04 00:23:27]

I think a call to quota->NotifyStorageModified(origin, -size) should be here and
in the similar DeleteIndexdDBForOrigin method.

It's unfortunate to be adding a new method so similar to the existing
DeleteIndexedDbForOrigin method. Consider merging them now to avoid the code
duplication. The only callsite for DeleteIndexedDBForOrigin() is in
extension_data_delete.cc. I think that caller can just as easily pass in the
'origin_url' as it can the 'origin_id'... the extension_url == origin_url.

The Delete verb is more consistent with the corresponding stuff in other storage
silos than the Evict verb.

[dgrogan, 2011/08/04 19:47:50]

Good idea.
Merged and renamed.

+}
+
 bool IndexedDBContext::IsUnlimitedStorageGranted(
     const GURL& origin) const {
   return special_storage_policy_->IsStorageUnlimited(origin);
@@ -137,6 +151,145 @@ void IndexedDBContext::GetAllOriginIdentifiers(
   }
 }
 
+int64 IndexedDBContext::InitializeDiskUsage(const GURL& origin_url) {
+  int64 disk_usage = ReadUsageFromDisk(origin_url);
+  origin_size_map_[origin_url] = disk_usage;
+  return disk_usage;
+}
+
+int64 IndexedDBContext::ReadUsageFromDisk(const GURL& origin_url) const {
+  string16 origin_id = DatabaseUtil::GetOriginIdentifier(origin_url);
+  FilePath file_path = GetIndexedDBFilePath(origin_id);
+  return file_util::ComputeDirectorySize(file_path);
+}
+
+void IndexedDBContext::ConnectionClosed(const GURL& origin_url) {
+  if (quota_manager_proxy()) {
+    quota_manager_proxy()->NotifyStorageAccessed(
+        quota::QuotaClient::kIndexedDatabase, origin_url,
+        quota::kStorageTypeTemporary);
+  }
+  connection_count_[origin_url]--;
+  if (connection_count_[origin_url] == 0) {
+    QueryDiskAndUpdateQuotaUsage(origin_url);
+    connection_count_.erase(origin_url);
+  }
+}
+
+void IndexedDBContext::NewConnection(const GURL& origin_url) {
+  if (quota_manager_proxy()) {
+    quota_manager_proxy()->NotifyStorageAccessed(
+        quota::QuotaClient::kIndexedDatabase, origin_url,
+        quota::kStorageTypeTemporary);
+  }
+  connection_count_[origin_url]++;
+  QueryAvailableQuota(origin_url);
+}
+
+bool IndexedDBContext::WouldBeOverQuota(const GURL& origin_url,
+                                        int64 additional_bytes) {
+  if (space_available_map_.find(origin_url) == space_available_map_.end()) {
+    // We haven't heard back from the QuotaManager yet, just let it through.
+    return false;
+  }
+  bool over_quota = additional_bytes > space_available_map_[origin_url];
+  return over_quota;

[michaeln, 2011/08/04 00:23:27]

nit: could return the comparison result w/o the local var

+}
+
+bool IndexedDBContext::IsOverQuota(const GURL& origin_url) {
+  // Can we store an additional byte?
+  return WouldBeOverQuota(origin_url, 1 /*additional_bytes*/);

[michaeln, 2011/08/04 00:23:27]

nit: instead of comments, consider self-documenting code
  const int kOneAdditionalByte = 1;
  return WouldBeOverQuota(origin_url, kOneAdditionalByte);

[dgrogan, 2011/08/04 19:47:50]

Done.

+}
+
+void IndexedDBContext::TransactionComplete(const GURL& origin_url) {
+  QueryDiskAndUpdateQuotaUsage(origin_url);
+  QueryAvailableQuota(origin_url);
+}
+
+void IndexedDBContext::QueryDiskAndUpdateQuotaUsage(const GURL& origin_url) {
+  int64 former_disk_usage = origin_size_map_[origin_url];
+  int64 current_disk_usage = ReadUsageFromDisk(origin_url);
+  int64 difference = current_disk_usage - former_disk_usage;
+  if (difference) {
+    quota_manager_proxy()->NotifyStorageModified(
+        quota::QuotaClient::kIndexedDatabase,
+        origin_url,
+        quota::kStorageTypeTemporary,
+        difference);
+    origin_size_map_[origin_url] = current_disk_usage;
+  }
+}
+
+void IndexedDBContext::GotUpdatedQuota(const GURL& origin_url, int64 usage,
+                                       int64 quota) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WEBKIT));
+  space_available_map_[origin_url] = quota - usage;
+}
+
+class IndexedDBGetUsageAndQuotaCallback :

[michaeln, 2011/08/04 00:23:27]

can this be in the anon namespace too, or maybe we can simplify this away... see
comment below

[dgrogan, 2011/08/04 19:47:50]

I made it a nested class.

+    public quota::QuotaManager::GetUsageAndQuotaCallback {
+ public:
+  IndexedDBGetUsageAndQuotaCallback(IndexedDBContext* context,
+                                    const GURL& origin_url)
+      : context_(context),
+        origin_url_(origin_url) {
+  }
+
+  void Run(quota::QuotaStatusCode status, int64 usage, int64 quota) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+    DCHECK(status == quota::kQuotaStatusOk || status == quota::kQuotaErrorAbort)
+        << "status was " << status;
+    if (status == quota::kQuotaErrorAbort) {
+      // We seem to no longer care to wait around for the answer.
+      return;
+    }
+    BrowserThread::PostTask(BrowserThread::WEBKIT, FROM_HERE,
+        NewRunnableMethod(context_.get(),
+                          &IndexedDBContext::GotUpdatedQuota,
+                          origin_url_,
+                          usage,
+                          quota));
+  }
+
+  virtual void RunWithParams(
+        const Tuple3<quota::QuotaStatusCode, int64, int64>& params) {
+    Run(params.a, params.b, params.c);
+  }
+
+ private:
+  scoped_refptr<IndexedDBContext> context_;
+  const GURL origin_url_;
+};
+
+namespace {
+
+void RequestQuota(

[michaeln, 2011/08/04 00:23:27]

I think some of this can be simplified by posting runnable method tasks on the
IndexedDBContext object, boiling it down to two methods and no new classes.

void IndexedDBContext::QueryAvailableQuota(origin_url) {
  if (!IO_THREAD) {
     DCHECK(WEBKIT_THREAD);
     if (quota_manager_proxy())
         PostTask(IO_THREAD,
                  NewRunnableMethod(this,
                                QueryAvailableQuota,
                                origin_url));
     return;
  }
  if (!quota_manager_proxy()->quota_manager())
    return;
  AddRef();  // balanced in the OnQuotaCallback
  proxy->quota_manager()->GetUsageAndQuota(
      origin_url, quota::kStorageTypeTemporary,
      NewCallback(this, &IndexedDBContext::OnQuotaCallback));
}

void IndexedDBContext::OnQuotaCallback(origin, usage, quota) {
  if (!WEBKIT_THREAD) {
     DCHECK(IO_THREAD);
     PostTask(WEBKIT_THREAD,
              NewRunnableMethod(this,
                                OnQuotaCallback,
                                origin, usage, quota));
     Release();  // balanced in QueryAvailableQuota
     return;
  }
  space_available_map_[origin_url] = quota - usage;
}

[dgrogan, 2011/08/04 19:47:50]

I got rid of this RequestQuota method by following your example.
But I ran into a problem here: origin isn't one of the arguments. The quota
manager doesn't tell the callback which origin it's talking about.  So I left
the class so that it can store the origin.

+    scoped_refptr<quota::QuotaManagerProxy> proxy, const GURL& gurl,
+    IndexedDBGetUsageAndQuotaCallback* idb_quota_callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (!proxy->quota_manager()) {
+    // QuotaManager was deleted on the IO thread just before this RequestQuota
+    // task was run.
+    return;
+  }
+  proxy->quota_manager()->GetUsageAndQuota(
+      gurl,
+      quota::kStorageTypeTemporary,
+      idb_quota_callback);
+}
+
+}
+
+void IndexedDBContext::QueryAvailableQuota(const GURL& origin_url) {
+  IndexedDBGetUsageAndQuotaCallback* callback =
+      new IndexedDBGetUsageAndQuotaCallback(this, origin_url);
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+      NewRunnableFunction(&RequestQuota,
+                          make_scoped_refptr(quota_manager_proxy()),
+                          origin_url,
+                          callback));
+}
+
 quota::QuotaManagerProxy* IndexedDBContext::quota_manager_proxy() {
   return quota_manager_proxy_;
 }