Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(319)

Issue 134693005: Revert of IndexedDBFactory now ForceCloses databases. (Closed)

Created:
6 years, 11 months ago by cmumford
Modified:
6 years, 11 months ago
Reviewers:
dgrogan, jsbell
CC:
chromium-reviews, jam, alecflett, joi+watch-content_chromium.org, darin-cc_chromium.org, dgrogan
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+98 lines, -139 lines) Patch
M content/browser/indexed_db/indexed_db_context_impl.h View 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/indexed_db/indexed_db_context_impl.cc View 5 chunks +31 lines, -10 lines 0 comments Download
M content/browser/indexed_db/indexed_db_database.h View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/indexed_db/indexed_db_database.cc View 1 chunk +0 lines, -11 lines 0 comments Download
M content/browser/indexed_db/indexed_db_factory.h View 4 chunks +2 lines, -8 lines 0 comments Download
M content/browser/indexed_db/indexed_db_factory.cc View 4 chunks +18 lines, -53 lines 0 comments Download
M content/browser/indexed_db/indexed_db_unittest.cc View 3 chunks +45 lines, -56 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
cmumford
Created Revert of IndexedDBFactory now ForceCloses databases.
6 years, 11 months ago (2014-01-10 21:56:12 UTC) #1
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
6 years, 11 months ago (2014-01-10 22:03:17 UTC) #2
jsbell
lgtm
6 years, 11 months ago (2014-01-10 22:23:18 UTC) #3
jsbell
On 2014/01/10 22:23:18, jsbell wrote: > lgtm I couldn't "git cl patch" this for some ...
6 years, 11 months ago (2014-01-10 23:14:26 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cmumford@chromium.org/134693005/1
6 years, 11 months ago (2014-01-10 23:53:42 UTC) #5
commit-bot: I haz the power
6 years, 11 months ago (2014-01-10 23:53:44 UTC) #6
Message was sent while issue was closed.
Failed to apply patch for content/browser/indexed_db/indexed_db_context_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/browser/indexed_db/indexed_db_context_impl.cc
  Hunk #1 FAILED at 187.
  Hunk #2 FAILED at 337.
  Hunk #3 FAILED at 347.
  Hunk #4 FAILED at 372.
  Hunk #5 FAILED at 390.
  5 out of 5 hunks FAILED -- saving rejects to file
content/browser/indexed_db/indexed_db_context_impl.cc.rej

Patch:       content/browser/indexed_db/indexed_db_context_impl.cc
Index: content/browser/indexed_db/indexed_db_context_impl.cc
diff --git a/content/browser/indexed_db/indexed_db_context_impl.cc
b/content/browser/indexed_db/indexed_db_context_impl.cc
index
df99bf42a9fd1362c90c7e97a29d916f3b4ddc1f..fc1c932ac2a012bec315c88b3a6d0bd8bb1cbc92
100644
--- a/content/browser/indexed_db/indexed_db_context_impl.cc
+++ b/content/browser/indexed_db/indexed_db_context_impl.cc
@@ -187,17 +187,16 @@
     // origins in the outer loop.
 
     if (factory_) {
-      std::pair<IndexedDBFactory::OriginDBMapIterator,
-                IndexedDBFactory::OriginDBMapIterator> range =
+      std::vector<IndexedDBDatabase*> databases =
           factory_->GetOpenDatabasesForOrigin(origin_url);
       // TODO(jsbell): Sort by name?
       scoped_ptr<base::ListValue> database_list(new base::ListValue());
 
-      for (IndexedDBFactory::OriginDBMapIterator it = range.first;
-           it != range.second;
+      for (std::vector<IndexedDBDatabase*>::iterator it = databases.begin();
+           it != databases.end();
            ++it) {
 
-        const IndexedDBDatabase* db = it->second;
+        const IndexedDBDatabase* db = *it;
         scoped_ptr<base::DictionaryValue> db_info(new base::DictionaryValue());
 
         db_info->SetString("name", db->name());
@@ -337,9 +336,21 @@
   if (data_path_.empty() || !IsInOriginSet(origin_url))
     return;
 
+  if (connections_.find(origin_url) != connections_.end()) {
+    ConnectionSet& connections = connections_[origin_url];
+    ConnectionSet::iterator it = connections.begin();
+    while (it != connections.end()) {
+      // Remove before closing so callbacks don't double-erase
+      IndexedDBConnection* connection = *it;
+      DCHECK(connection->IsConnected());
+      connections.erase(it++);
+      connection->ForceClose();
+    }
+    DCHECK_EQ(connections_[origin_url].size(), 0UL);
+    connections_.erase(origin_url);
+  }
   if (factory_)
     factory_->ForceClose(origin_url);
-  DCHECK_EQ(0UL, GetConnectionCount(origin_url));
 }
 
 size_t IndexedDBContextImpl::GetConnectionCount(const GURL& origin_url) {
@@ -347,10 +358,10 @@
   if (data_path_.empty() || !IsInOriginSet(origin_url))
     return 0;
 
-  if (!factory_)
+  if (connections_.find(origin_url) == connections_.end())
     return 0;
 
-  return factory_->GetConnectionCount(origin_url);
+  return connections_[origin_url].size();
 }
 
 base::FilePath IndexedDBContextImpl::GetFilePath(const GURL& origin_url) const
{
@@ -372,12 +383,14 @@
 void IndexedDBContextImpl::ConnectionOpened(const GURL& origin_url,
                                             IndexedDBConnection* connection) {
   DCHECK(TaskRunner()->RunsTasksOnCurrentThread());
+  DCHECK_EQ(connections_[origin_url].count(connection), 0UL);
   if (quota_manager_proxy()) {
     quota_manager_proxy()->NotifyStorageAccessed(
         quota::QuotaClient::kIndexedDatabase,
         origin_url,
         quota::kStorageTypeTemporary);
   }
+  connections_[origin_url].insert(connection);
   if (AddToOriginSet(origin_url)) {
     // A newly created db, notify the quota system.
     QueryDiskAndUpdateQuotaUsage(origin_url);
@@ -390,18 +403,26 @@
 void IndexedDBContextImpl::ConnectionClosed(const GURL& origin_url,
                                             IndexedDBConnection* connection) {
   DCHECK(TaskRunner()->RunsTasksOnCurrentThread());
+  // May not be in the map if connection was forced to close
+  if (connections_.find(origin_url) == connections_.end() ||
+      connections_[origin_url].count(connection) != 1)
+    return;
   if (quota_manager_proxy()) {
     quota_manager_proxy()->NotifyStorageAccessed(
         quota::QuotaClient::kIndexedDatabase,
         origin_url,
         quota::kStorageTypeTemporary);
   }
-  if (factory_ && factory_->GetConnectionCount(origin_url) == 0)
+  connections_[origin_url].erase(connection);
+  if (connections_[origin_url].size() == 0) {
     QueryDiskAndUpdateQuotaUsage(origin_url);
+    connections_.erase(origin_url);
+  }
 }
 
 void IndexedDBContextImpl::TransactionComplete(const GURL& origin_url) {
-  DCHECK(!factory_ || factory_->GetConnectionCount(origin_url) > 0);
+  DCHECK(connections_.find(origin_url) != connections_.end() &&
+         connections_[origin_url].size() > 0);
   QueryDiskAndUpdateQuotaUsage(origin_url);
   QueryAvailableQuota(origin_url);
 }

Powered by Google App Engine
This is Rietveld 408576698