[IndexedDB] Integrating failures and corruption with transaction

content/browser/indexed_db/indexed_db_transaction.cc

Index: content/browser/indexed_db/indexed_db_transaction.cc
diff --git a/content/browser/indexed_db/indexed_db_transaction.cc b/content/browser/indexed_db/indexed_db_transaction.cc
index 5040dddf49fdcabdb0bc06ff18177f95b312c7a2..104886509af57aaa26fc93981c35a4e6cafa6a9f 100644
--- a/content/browser/indexed_db/indexed_db_transaction.cc
+++ b/content/browser/indexed_db/indexed_db_transaction.cc
@@ -16,6 +16,7 @@
 #include "content/browser/indexed_db/indexed_db_cursor.h"
 #include "content/browser/indexed_db/indexed_db_database.h"
 #include "content/browser/indexed_db/indexed_db_database_callbacks.h"
+#include "content/browser/indexed_db/indexed_db_factory.h"
 #include "content/browser/indexed_db/indexed_db_observation.h"
 #include "content/browser/indexed_db/indexed_db_observer_changes.h"
 #include "content/browser/indexed_db/indexed_db_tracing.h"
@@ -48,7 +49,7 @@ void IndexedDBTransaction::TaskQueue::clear() {
 
 IndexedDBTransaction::Operation IndexedDBTransaction::TaskQueue::pop() {
   DCHECK(!queue_.empty());
-  Operation task(queue_.front());
+  Operation task = std::move(queue_.front());
   queue_.pop();
   return task;
 }
@@ -61,9 +62,9 @@ void IndexedDBTransaction::TaskStack::clear() {
     stack_.pop();
 }
 
-IndexedDBTransaction::Operation IndexedDBTransaction::TaskStack::pop() {
+IndexedDBTransaction::AbortOperation IndexedDBTransaction::TaskStack::pop() {
   DCHECK(!stack_.empty());
-  Operation task(stack_.top());
+  AbortOperation task = std::move(stack_.top());
   stack_.pop();
   return task;
 }
@@ -77,14 +78,8 @@ IndexedDBTransaction::IndexedDBTransaction(
     : id_(id),
       object_store_ids_(object_store_ids),
       mode_(mode),
-      used_(false),
-      state_(CREATED),
-      commit_pending_(false),
       connection_(std::move(connection)),
-      transaction_(backing_store_transaction),
-      backing_store_transaction_begun_(false),
-      should_process_queue_(false),
-      pending_preemptive_events_(0) {
+      transaction_(backing_store_transaction) {
   callbacks_ = connection_->callbacks();
   database_ = connection_->database();
 
@@ -104,6 +99,7 @@ IndexedDBTransaction::~IndexedDBTransaction() {
   DCHECK(task_queue_.empty());
   DCHECK(abort_task_stack_.empty());
   DCHECK(pending_observers_.empty());
+  DCHECK(!processing_event_queue_);
 }
 
 void IndexedDBTransaction::ScheduleTask(blink::WebIDBTaskType type,
@@ -123,10 +119,10 @@ void IndexedDBTransaction::ScheduleTask(blink::WebIDBTaskType type,
   RunTasksIfStarted();
 }
 
-void IndexedDBTransaction::ScheduleAbortTask(Operation abort_task) {
+void IndexedDBTransaction::ScheduleAbortTask(AbortOperation abort_task) {
   DCHECK_NE(FINISHED, state_);
   DCHECK(used_);
-  abort_task_stack_.push(abort_task);
+  abort_task_stack_.push(std::move(abort_task));
 }
 
 void IndexedDBTransaction::RunTasksIfStarted() {
@@ -170,7 +166,7 @@ void IndexedDBTransaction::Abort(const IndexedDBDatabaseError& error) {
 
   // Run the abort tasks, if any.
   while (!abort_task_stack_.empty())
-    abort_task_stack_.pop().Run(NULL);
+    abort_task_stack_.pop().Run();
 
   preemptive_task_queue_.clear();
   pending_preemptive_events_ = 0;
@@ -369,7 +365,7 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
     database_->TransactionFinished(this, true);
   } else {
     while (!abort_task_stack_.empty())
-      abort_task_stack_.pop().Run(NULL);
+      abort_task_stack_.pop().Run();
 
     IndexedDBDatabaseError error;
     if (leveldb_env::IndicatesDiskFull(s)) {
@@ -383,7 +379,6 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
     callbacks_->OnAbort(id_, error);
 
     database_->TransactionFinished(this, false);
-    database_->TransactionCommitFailed(s);
   }
 
   database_ = NULL;
@@ -393,10 +388,14 @@ leveldb::Status IndexedDBTransaction::CommitPhaseTwo() {
 void IndexedDBTransaction::ProcessTaskQueue() {
   IDB_TRACE1("IndexedDBTransaction::ProcessTaskQueue", "txn.id", id());
 
+  DCHECK(!processing_event_queue_);
+
   // May have been aborted.
   if (!should_process_queue_)
     return;
 
+  processing_event_queue_ = true;
+
   DCHECK(!IsTaskQueueEmpty());
   should_process_queue_ = false;
 
@@ -415,11 +414,24 @@ void IndexedDBTransaction::ProcessTaskQueue() {
   while (!task_queue->empty() && state_ != FINISHED) {
     DCHECK_EQ(state_, STARTED);
     Operation task(task_queue->pop());
-    task.Run(this);
+    OperationResult result = task.Run(this);
     if (!pending_preemptive_events_) {
       DCHECK(diagnostics_.tasks_completed < diagnostics_.tasks_scheduled);
       ++diagnostics_.tasks_completed;
     }
+    if (!result.ok()) {
+      processing_event_queue_ = false;
+      scoped_refptr<IndexedDBDatabase> database = database_;
+      if (result.IsCorruption()) {
+        IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                     base::ASCIIToUTF16(result.ToString()));

[cmumford, 2016/11/21 18:29:22]

I think that leveldb::Status can contain paths to the profile directory, so we
need to be careful to sanitize the message if it's being sent back to the page
in any way. I think we do that someplace else in the code, but can't find it
with a quick search.

[dmurph, 2016/11/22 23:33:11]

We don't actually send this back to the page, it's just used to send to
IndexedDbBackingStore::RecordCorruptionInfo, which we save to a file.

[cmumford, 2016/11/28 20:48:25]

Yes, but that file is read on the next db open in ReadCorruptionInfo(), and the
message is sent in dataLossMessage in the upgradeneeded event.

+        database->factory()->HandleBackingStoreCorruption(database_->origin(),
+                                                          error);
+      } else {
+        database->factory()->HandleBackingStoreFailure(database_->origin());
+      }
+      return;
+    }
 
     // Event itself may change which queue should be processed next.
     task_queue =
@@ -430,12 +442,15 @@ void IndexedDBTransaction::ProcessTaskQueue() {
   // and the front-end requested a commit, it is now safe to do so.
   if (!HasPendingTasks() && state_ != FINISHED && commit_pending_) {
     Commit();
+    processing_event_queue_ = false;
     return;
   }
 
   // The transaction may have been aborted while processing tasks.
-  if (state_ == FINISHED)
+  if (state_ == FINISHED) {
+    processing_event_queue_ = false;
     return;
+  }
 
   DCHECK(state_ == STARTED);
 
@@ -446,6 +461,7 @@ void IndexedDBTransaction::ProcessTaskQueue() {
     timeout_timer_.Start(FROM_HERE, GetInactivityTimeout(),
                          base::Bind(&IndexedDBTransaction::Timeout, this));
   }
+  processing_event_queue_ = false;
 }
 
 base::TimeDelta IndexedDBTransaction::GetInactivityTimeout() const {