IndexedDB: Ignore open/delete requests from terminated renderers.

content/browser/indexed_db/indexed_db_database.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/indexed_db/indexed_db_database.h"
 
 #include <math.h>
 
 #include <limits>
 #include <set>
@@ skipping 100 matching lines @@
 };
 
 class IndexedDBDatabase::OpenRequest
     : public IndexedDBDatabase::ConnectionRequest {
  public:
   OpenRequest(scoped_refptr<IndexedDBDatabase> db,
               std::unique_ptr<IndexedDBPendingConnection> pending_connection)
       : 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.
       if (!db_->OpenInternal().ok()) {
         // TODO(jsbell): Consider including sanitized leveldb status message.
         base::string16 message;
         if (pending_->version == IndexedDBDatabaseMetadata::NO_VERSION) {
           message = ASCIIToUTF16(
               "Internal error opening database with no version specified.");
         } else {
@@ skipping 152 matching lines @@
 };
 
 class IndexedDBDatabase::DeleteRequest
     : public IndexedDBDatabase::ConnectionRequest {
  public:
   DeleteRequest(scoped_refptr<IndexedDBDatabase> db,
                 scoped_refptr<IndexedDBCallbacks> callbacks)
       : ConnectionRequest(db), callbacks_(callbacks) {}
 
   void Perform() override {
+    if (!callbacks_->IsValid()) {

[cmumford, 2016/10/03 18:39:26]

I can see completing the open if disconnecting, but does this mean that if a
render process immediately closes after sending a delete request then that
request won't be implemented? This feels like something we'd still want to do.

[jsbell, 2016/10/03 19:28:37]

Could go either way, yeah. I decided this way for symmetry since we can't report
the delete as being successful and/or reporting data loss (although the renderer
could just crash later in the process, too - no guarantee the message ever gets
there).

How strongly do you feel about letting the delete proceed?

[cmumford, 2016/10/03 20:03:34]

My main worry is that a normal (i.e. not-crash) shutdown of a render process
could initiate the delete request and then shutdown before the IDB thread could
process the request. That has a privacy aspect to it, so my preference is to
honor the request even though the page is gone.

In all honesty I don't feel strongly about this, but wanted to point it out.

[jsbell, 2016/10/03 20:57:58]

That's a very good point, I hadn't considered that scenario. I agree we should
let deletes proceed.

+      db_->RequestComplete(this);
+      return;
+    }
+
     if (db_->connections_.empty()) {
       // No connections, so delete immediately.
       DoDelete();
       return;
     }
 
     // Front end ensures the event is not fired at connections that have
     // close_pending set.
     const int64_t old_version = db_->metadata_.version;
     const int64_t new_version = IndexedDBDatabaseMetadata::NO_VERSION;
@@ skipping 1825 matching lines @@
 
 void IndexedDBDatabase::VersionChangeAbortOperation(
     int64_t previous_version,
     IndexedDBTransaction* transaction) {
   DCHECK(!transaction);
   IDB_TRACE("IndexedDBDatabase::VersionChangeAbortOperation");
   metadata_.version = previous_version;
 }
 
 }  // namespace content