IndexedDB: Ignore open/delete requests from terminated renderers.

Tasks associated with transactions are terminated when a dispatcher
host's connection goes away, but queued connection open/delete
requests are not associated with transactions, meaning attempting to
send the results will fail. To avoid this, check to see if the
dispatcher is still connected when a request gets to the head of the
queue.

BUG=650635
R=cmumford@chromium.org

Review-Url: https://codereview.chromium.org/2386683003
Cr-Commit-Position: refs/heads/master@{#422840}

content/browser/indexed_db/indexed_db_callbacks.cc

@@ -604,6 +604,12 @@ void IndexedDBCallbacks::OnSuccess() {
   dispatcher_host_ = NULL;
 }
 
+bool IndexedDBCallbacks::IsValid() const {
+  DCHECK(dispatcher_host_.get());
+
+  return dispatcher_host_->IsOpen();
+}
+
 void IndexedDBCallbacks::SetConnectionOpenStartTime(
     const base::TimeTicks& start_time) {
   connection_open_start_time_ = start_time;

content/browser/indexed_db/indexed_db_callbacks.h

@@ -107,6 +107,10 @@ class CONTENT_EXPORT IndexedDBCallbacks
   // IndexedDBCursor::Continue / Advance (when complete)
   virtual void OnSuccess();
 
+  // Checks to see if the associated dispatcher host is still connected. If
+  // not this request can be dropped.
+  virtual bool IsValid() const;
+
   void SetConnectionOpenStartTime(const base::TimeTicks& start_time);
 
  protected:

content/browser/indexed_db/indexed_db_database.cc

@@ -118,6 +118,11 @@ class IndexedDBDatabase::OpenRequest
       : ConnectionRequest(db), pending_(std::move(pending_connection)) {}
 
   void Perform() override {
+    if (!pending_->callbacks->IsValid()) {
+      db_->RequestComplete(this);
+      return;
+    }
+
     if (db_->metadata_.id == kInvalidId) {
       // The database was deleted then immediately re-opened; OpenInternal()
       // recreates it in the backing store.

content/browser/indexed_db/indexed_db_database_unittest.cc

@@ -150,26 +150,31 @@ TEST(IndexedDBDatabaseTest, ForcedClose) {
   EXPECT_TRUE(callbacks->abort_called());
 }
 
-class MockDeleteCallbacks : public IndexedDBCallbacks {
+class MockCallbacks : public IndexedDBCallbacks {
  public:
-  MockDeleteCallbacks()
-      : IndexedDBCallbacks(NULL, 0, 0),
-        blocked_called_(false),
-        success_called_(false) {}
+  MockCallbacks() : IndexedDBCallbacks(NULL, 0, 0) {}
 
   void OnBlocked(int64_t existing_version) override { blocked_called_ = true; }
   void OnSuccess(int64_t result) override { success_called_ = true; }
+  void OnError(const IndexedDBDatabaseError& error) override {
+    error_called_ = true;
+  }
+  bool IsValid() const override { return valid_; }
 
   bool blocked_called() const { return blocked_called_; }
   bool success_called() const { return success_called_; }
+  bool error_called() const { return error_called_; }
+  void set_valid(bool valid) { valid_ = valid; }
 
  private:
-  ~MockDeleteCallbacks() override {}
+  ~MockCallbacks() override {}
 
-  bool blocked_called_;
-  bool success_called_;
+  bool blocked_called_ = false;
+  bool success_called_ = false;
+  bool error_called_ = false;
+  bool valid_ = true;
 
-  DISALLOW_COPY_AND_ASSIGN(MockDeleteCallbacks);
+  DISALLOW_COPY_AND_ASSIGN(MockCallbacks);
 };
 
 TEST(IndexedDBDatabaseTest, PendingDelete) {
@@ -203,7 +208,7 @@ TEST(IndexedDBDatabaseTest, PendingDelete) {
   EXPECT_EQ(db->PendingOpenDeleteCount(), 0UL);
   EXPECT_FALSE(backing_store->HasOneRef());  // local and db
 
-  scoped_refptr<MockDeleteCallbacks> request2(new MockDeleteCallbacks());
+  scoped_refptr<MockCallbacks> request2(new MockCallbacks());
   db->DeleteDatabase(request2);
   EXPECT_EQ(db->ConnectionCount(), 1UL);
   EXPECT_EQ(db->ActiveOpenDeleteCount(), 1UL);
@@ -228,6 +233,107 @@ TEST(IndexedDBDatabaseTest, PendingDelete) {
   EXPECT_TRUE(request2->success_called());
 }
 
+TEST(IndexedDBDatabaseTest, ConnectionRequestsNoLongerValid) {
+  scoped_refptr<IndexedDBFakeBackingStore> backing_store =
+      new IndexedDBFakeBackingStore();
+
+  const int64_t transaction_id1 = 1;
+  scoped_refptr<MockIndexedDBFactory> factory = new MockIndexedDBFactory();
+  leveldb::Status s;
+  scoped_refptr<IndexedDBDatabase> db = IndexedDBDatabase::Create(
+      ASCIIToUTF16("db"), backing_store.get(), factory.get(),
+      IndexedDBDatabase::Identifier(), &s);
+
+  // Make a connection request. This will be processed immediately.
+  scoped_refptr<MockIndexedDBCallbacks> request1(new MockIndexedDBCallbacks());
+  {
+    std::unique_ptr<IndexedDBPendingConnection> connection(
+        base::MakeUnique<IndexedDBPendingConnection>(
+            request1, make_scoped_refptr(new MockIndexedDBDatabaseCallbacks()),
+            kFakeChildProcessId, transaction_id1,
+            IndexedDBDatabaseMetadata::DEFAULT_VERSION));
+    db->OpenConnection(std::move(connection));
+  }
+
+  EXPECT_EQ(db->ConnectionCount(), 1UL);
+  EXPECT_EQ(db->ActiveOpenDeleteCount(), 0UL);
+  EXPECT_EQ(db->PendingOpenDeleteCount(), 0UL);
+
+  // Make a delete request. This will be blocked by the open.
+  scoped_refptr<MockCallbacks> request2(new MockCallbacks());
+  db->DeleteDatabase(request2);
+
+  EXPECT_EQ(db->ConnectionCount(), 1UL);
+  EXPECT_EQ(db->ActiveOpenDeleteCount(), 1UL);
+  EXPECT_EQ(db->PendingOpenDeleteCount(), 0UL);
+
+  db->VersionChangeIgnored();
+
+  EXPECT_TRUE(request2->blocked_called());
+
+  // Make another delete request. This will be waiting in the queue.
+  scoped_refptr<MockCallbacks> request3(new MockCallbacks());
+  db->DeleteDatabase(request3);
+  EXPECT_FALSE(request3->HasOneRef());  // local, db
+
+  EXPECT_EQ(db->ConnectionCount(), 1UL);
+  EXPECT_EQ(db->ActiveOpenDeleteCount(), 1UL);
+  EXPECT_EQ(db->PendingOpenDeleteCount(), 1UL);
+
+  // Make another connection request. This will also be waiting in the queue.
+  scoped_refptr<MockCallbacks> request4(new MockCallbacks());
+  {
+    std::unique_ptr<IndexedDBPendingConnection> connection(
+        base::MakeUnique<IndexedDBPendingConnection>(
+            request4, make_scoped_refptr(new MockIndexedDBDatabaseCallbacks()),
+            kFakeChildProcessId, transaction_id1,
+            IndexedDBDatabaseMetadata::DEFAULT_VERSION));
+    db->OpenConnection(std::move(connection));
+  }
+
+  EXPECT_EQ(db->ConnectionCount(), 1UL);
+  EXPECT_EQ(db->ActiveOpenDeleteCount(), 1UL);
+  EXPECT_EQ(db->PendingOpenDeleteCount(), 2UL);
+
+  // Finally yet another delete request, also waiting in the queue.
+  scoped_refptr<MockCallbacks> request5(new MockCallbacks());
+  db->DeleteDatabase(request2);
+
+  EXPECT_EQ(db->ConnectionCount(), 1UL);
+  EXPECT_EQ(db->ActiveOpenDeleteCount(), 1UL);
+  EXPECT_EQ(db->PendingOpenDeleteCount(), 3UL);
+
+  // Simulate renderer going away.
+  request3->set_valid(false);
+  request4->set_valid(false);
+
+  // Close the blocking connection. First delete request should succeed.
+  EXPECT_FALSE(request2->success_called());
+  db->Close(request1->connection(), true /* forced */);
+  EXPECT_TRUE(request2->success_called());
+
+  // Delete requests that have lost dispatcher should still be processed so
+  // that e.g. a delete followed by a window close is not ignored.
+  EXPECT_FALSE(request3->blocked_called());
+  EXPECT_TRUE(request3->success_called());
+  EXPECT_FALSE(request3->error_called());
+  EXPECT_TRUE(request3->HasOneRef());  // local
+
+  // Open requests that have lost dispatcher should not be processed.
+  EXPECT_FALSE(request4->blocked_called());
+  EXPECT_FALSE(request4->success_called());
+  EXPECT_FALSE(request4->error_called());
+  EXPECT_TRUE(request4->HasOneRef());  // local
+
+  // Final delete request should also run.
+  EXPECT_TRUE(request2->success_called());
+
+  // And everything else should be complete.
+  EXPECT_EQ(db->ConnectionCount(), 0UL);
+  EXPECT_EQ(db->ActiveOpenDeleteCount(), 0UL);
+  EXPECT_EQ(db->PendingOpenDeleteCount(), 0UL);
+}
+
 void DummyOperation(IndexedDBTransaction* transaction) {
 }
 

content/browser/indexed_db/indexed_db_dispatcher_host.cc

@@ -95,6 +95,9 @@ void IndexedDBDispatcherHost::ResetDispatcherHosts() {
   // messages are processed.
   DCHECK(indexed_db_context_->TaskRunner()->RunsTasksOnCurrentThread());
 
+  // Prevent any pending connections from being processed.
+  is_open_ = false;
+
   // Note that we explicitly separate CloseAll() from destruction of the
   // DatabaseDispatcherHost, since CloseAll() can invoke callbacks which need to
   // be dispatched through database_dispatcher_host_.
@@ -248,6 +251,11 @@ void IndexedDBDispatcherHost::DropBlobData(const std::string& uuid) {
     --iter->second.second;
 }
 
+bool IndexedDBDispatcherHost::IsOpen() const {
+  DCHECK(indexed_db_context_->TaskRunner()->RunsTasksOnCurrentThread());
+  return is_open_;
+}
+
 IndexedDBCursor* IndexedDBDispatcherHost::GetCursorFromId(
     int32_t ipc_cursor_id) {
   DCHECK(indexed_db_context_->TaskRunner()->RunsTasksOnCurrentThread());

content/browser/indexed_db/indexed_db_dispatcher_host.h

@@ -115,6 +115,10 @@ class IndexedDBDispatcherHost : public BrowserMessageFilter {
 
   std::string HoldBlobData(const IndexedDBBlobInfo& blob_info);
 
+  // True if the channel is closing/closed and outstanding requests
+  // can be abandoned. Only access on IndexedDB thread.
+  bool IsOpen() const;
+
  private:
   // Friends to enable OnDestruct() delegation.
   friend class BrowserThread;
@@ -316,6 +320,7 @@ class IndexedDBDispatcherHost : public BrowserMessageFilter {
       blob_data_handle_map_;
 
   // Only access on IndexedDB thread.
+  bool is_open_ = true;
   std::unique_ptr<DatabaseDispatcherHost> database_dispatcher_host_;
   std::unique_ptr<CursorDispatcherHost> cursor_dispatcher_host_;
 

content/browser/indexed_db/indexed_db_unittest.cc

@@ -144,6 +144,7 @@ class ForceCloseDBCallbacks : public IndexedDBCallbacks {
     connection_ = std::move(connection);
     idb_context_->ConnectionOpened(origin_, connection_.get());
   }
+  bool IsValid() const override { return true; }
 
   IndexedDBConnection* connection() { return connection_.get(); }
 

content/browser/indexed_db/mock_indexed_db_callbacks.cc

@@ -33,4 +33,8 @@ void MockIndexedDBCallbacks::OnSuccess(
   connection_ = std::move(connection);
 }
 
+bool MockIndexedDBCallbacks::IsValid() const {
+  return true;
+}
+
 }  // namespace content

content/browser/indexed_db/mock_indexed_db_callbacks.h

@@ -26,6 +26,7 @@ class MockIndexedDBCallbacks : public IndexedDBCallbacks {
   void OnSuccess(const IndexedDBKey& key) override;
   void OnSuccess(std::unique_ptr<IndexedDBConnection> connection,
                  const IndexedDBDatabaseMetadata& metadata) override;
+  bool IsValid() const override;
   IndexedDBConnection* connection() { return connection_.get(); }
 
  protected: