IndexedDB: Timeout transactions if front-end is unresponsive

Introduce a fixed time-out that limits how long a
transaction will wait after it handles the last request
before it gives up and aborts. This is to prevent "wedged"
renderers (not crashed but not turning the event loop, etc)
from holding up the transaction queue since they never
issue a another request or ask to commit.

(TBR for new file entry in gypi)

BUG=318992
R=dgrogam
TBR=jam

Review URL: https://codereview.chromium.org/67173010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239543 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 165 insertions, 22 deletions

diff --git a/content/browser/indexed_db/indexed_db_backing_store.h b/content/browser/indexed_db/indexed_db_backing_store.h
index 60c996e..890f95eb 100644
--- a/content/browser/indexed_db/indexed_db_backing_store.h
+++ b/content/browser/indexed_db/indexed_db_backing_store.h
@@ -279,10 +279,10 @@ class CONTENT_EXPORT IndexedDBBackingStore
   class Transaction {
    public:
     explicit Transaction(IndexedDBBackingStore* backing_store);
-    ~Transaction();
-    void Begin();
-    bool Commit();
-    void Rollback();
+    virtual ~Transaction();
+    virtual void Begin();
+    virtual bool Commit();
+    virtual void Rollback();
     void Reset() {
       backing_store_ = NULL;
       transaction_ = NULL;
diff --git a/content/browser/indexed_db/indexed_db_database.cc b/content/browser/indexed_db/indexed_db_database.cc
index 258b01e..11ef4be 100644
--- a/content/browser/indexed_db/indexed_db_database.cc
+++ b/content/browser/indexed_db/indexed_db_database.cc
@@ -1402,8 +1402,14 @@ void IndexedDBDatabase::CreateTransaction(
       connection->callbacks(),
       std::set<int64>(object_store_ids.begin(), object_store_ids.end()),
       static_cast<indexed_db::TransactionMode>(mode),
-      this);
-  transactions_[transaction_id] = transaction;
+      this,
+      new IndexedDBBackingStore::Transaction(backing_store_));
+  TransactionCreated(transaction);
+}
+
+void IndexedDBDatabase::TransactionCreated(
+    scoped_refptr<IndexedDBTransaction> transaction) {
+  transactions_[transaction->id()] = transaction;
 }
 
 bool IndexedDBDatabase::IsOpenConnectionBlocked() const {
diff --git a/content/browser/indexed_db/indexed_db_database.h b/content/browser/indexed_db/indexed_db_database.h
index b9b4dac..6297f3d 100644
--- a/content/browser/indexed_db/indexed_db_database.h
+++ b/content/browser/indexed_db/indexed_db_database.h
@@ -118,6 +118,7 @@ class CONTENT_EXPORT IndexedDBDatabase
     return transaction_coordinator_;
   }
 
+  void TransactionCreated(scoped_refptr<IndexedDBTransaction> transaction);
   void TransactionStarted(IndexedDBTransaction* transaction);
   void TransactionFinished(IndexedDBTransaction* transaction);
   void TransactionFinishedAndCompleteFired(IndexedDBTransaction* transaction);
diff --git a/content/browser/indexed_db/indexed_db_fake_backing_store.cc b/content/browser/indexed_db/indexed_db_fake_backing_store.cc
index 3825982..3efd410 100644
--- a/content/browser/indexed_db/indexed_db_fake_backing_store.cc
+++ b/content/browser/indexed_db/indexed_db_fake_backing_store.cc
@@ -146,4 +146,10 @@ IndexedDBFakeBackingStore::OpenIndexCursor(
   return scoped_ptr<IndexedDBBackingStore::Cursor>();
 }
 
+IndexedDBFakeBackingStore::FakeTransaction::FakeTransaction(bool result)
+    : IndexedDBBackingStore::Transaction(NULL), result_(result) {}
+void IndexedDBFakeBackingStore::FakeTransaction::Begin() {}
+bool IndexedDBFakeBackingStore::FakeTransaction::Commit() { return result_; }
+void IndexedDBFakeBackingStore::FakeTransaction::Rollback() {}
+
 }  // namespace content
diff --git a/content/browser/indexed_db/indexed_db_fake_backing_store.h b/content/browser/indexed_db/indexed_db_fake_backing_store.h
index 93d047c..14b9562 100644
--- a/content/browser/indexed_db/indexed_db_fake_backing_store.h
+++ b/content/browser/indexed_db/indexed_db_fake_backing_store.h
@@ -107,6 +107,17 @@ class IndexedDBFakeBackingStore : public IndexedDBBackingStore {
                                              indexed_db::CursorDirection)
       OVERRIDE;
 
+  class FakeTransaction : public IndexedDBBackingStore::Transaction {
+   public:
+    FakeTransaction(bool result);
+    virtual void Begin() OVERRIDE;
+    virtual bool Commit() OVERRIDE;
+    virtual void Rollback() OVERRIDE;
+
+   private:
+    bool result_;
+  };
+
  protected:
   friend class base::RefCounted<IndexedDBFakeBackingStore>;
   virtual ~IndexedDBFakeBackingStore();
diff --git a/content/browser/indexed_db/indexed_db_transaction.cc b/content/browser/indexed_db/indexed_db_transaction.cc
index 28f8378..0080ecd 100644
--- a/content/browser/indexed_db/indexed_db_transaction.cc
+++ b/content/browser/indexed_db/indexed_db_transaction.cc
@@ -18,6 +18,8 @@
 
 namespace content {
 
+const int64 kInactivityTimeoutPeriodSeconds = 60;
+
 IndexedDBTransaction::TaskQueue::TaskQueue() {}
 IndexedDBTransaction::TaskQueue::~TaskQueue() { clear(); }
 
@@ -53,7 +55,8 @@ IndexedDBTransaction::IndexedDBTransaction(
     scoped_refptr<IndexedDBDatabaseCallbacks> callbacks,
     const std::set<int64>& object_store_ids,
     indexed_db::TransactionMode mode,
-    IndexedDBDatabase* database)
+    IndexedDBDatabase* database,
+    IndexedDBBackingStore::Transaction* backing_store_transaction)
     : id_(id),
       object_store_ids_(object_store_ids),
       mode_(mode),
@@ -62,7 +65,7 @@ IndexedDBTransaction::IndexedDBTransaction(
       commit_pending_(false),
       callbacks_(callbacks),
       database_(database),
-      transaction_(database->backing_store()),
+      transaction_(backing_store_transaction),
       backing_store_transaction_begun_(false),
       should_process_queue_(false),
       pending_preemptive_events_(0) {
@@ -86,6 +89,7 @@ void IndexedDBTransaction::ScheduleTask(Operation task, Operation abort_task) {
   if (state_ == FINISHED)
     return;
 
+  timeout_timer_.Stop();
   used_ = true;
   task_queue_.push(task);
   ++diagnostics_.tasks_scheduled;
@@ -98,6 +102,7 @@ void IndexedDBTransaction::ScheduleTask(IndexedDBDatabase::TaskType type,
   if (state_ == FINISHED)
     return;
 
+  timeout_timer_.Stop();
   used_ = true;
   if (type == IndexedDBDatabase::NORMAL_TASK) {
     task_queue_.push(task);
@@ -134,6 +139,8 @@ void IndexedDBTransaction::Abort(const IndexedDBDatabaseError& error) {
   if (state_ == FINISHED)
     return;
 
+  timeout_timer_.Stop();
+
   // The last reference to this object may be released while performing the
   // abort steps below. We therefore take a self reference to keep ourselves
   // alive while executing this method.
@@ -143,7 +150,7 @@ void IndexedDBTransaction::Abort(const IndexedDBDatabaseError& error) {
   should_process_queue_ = false;
 
   if (backing_store_transaction_begun_)
-    transaction_.Rollback();
+    transaction_->Rollback();
 
   // Run the abort tasks, if any.
   while (!abort_task_stack_.empty()) {
@@ -158,7 +165,7 @@ void IndexedDBTransaction::Abort(const IndexedDBDatabaseError& error) {
   // release references and allow the backing store itself to be
   // released, and order is critical.
   CloseOpenCursors();
-  transaction_.Reset();
+  transaction_->Reset();
 
   // Transactions must also be marked as completed before the
   // front-end is notified, as the transaction completion unblocks
@@ -224,6 +231,8 @@ void IndexedDBTransaction::Commit() {
   if (HasPendingTasks())
     return;
 
+  timeout_timer_.Stop();
+
   // The last reference to this object may be released while performing the
   // commit steps below. We therefore take a self reference to keep ourselves
   // alive while executing this method.
@@ -234,14 +243,14 @@ void IndexedDBTransaction::Commit() {
 
   state_ = FINISHED;
 
-  bool committed = !used_ || transaction_.Commit();
+  bool committed = !used_ || transaction_->Commit();
 
   // Backing store resources (held via cursors) must be released
   // before script callbacks are fired, as the script callbacks may
   // release references and allow the backing store itself to be
   // released, and order is critical.
   CloseOpenCursors();
-  transaction_.Reset();
+  transaction_->Reset();
 
   // Transactions must also be marked as completed before the
   // front-end is notified, as the transaction completion unblocks
@@ -275,7 +284,7 @@ void IndexedDBTransaction::ProcessTaskQueue() {
   should_process_queue_ = false;
 
   if (!backing_store_transaction_begun_) {
-    transaction_.Begin();
+    transaction_->Begin();
     backing_store_transaction_begun_ = true;
   }
 
@@ -302,8 +311,23 @@ void IndexedDBTransaction::ProcessTaskQueue() {
 
   // If there are no pending tasks, we haven't already committed/aborted,
   // and the front-end requested a commit, it is now safe to do so.
-  if (!HasPendingTasks() && state_ != FINISHED && commit_pending_)
+  if (!HasPendingTasks() && state_ != FINISHED && commit_pending_) {
     Commit();
+    return;
+  }
+
+  // Otherwise, start a timer in case the front-end gets wedged and
+  // never requests further activity.
+  timeout_timer_.Start(
+      FROM_HERE,
+      base::TimeDelta::FromSeconds(kInactivityTimeoutPeriodSeconds),
+      base::Bind(&IndexedDBTransaction::Timeout, this));
+}
+
+void IndexedDBTransaction::Timeout() {
+  Abort(IndexedDBDatabaseError(
+      blink::WebIDBDatabaseExceptionTimeoutError,
+      ASCIIToUTF16("Transaction timed out due to inactivity.")));
 }
 
 void IndexedDBTransaction::CloseOpenCursors() {
diff --git a/content/browser/indexed_db/indexed_db_transaction.h b/content/browser/indexed_db/indexed_db_transaction.h
index 111101f..6684856 100644
--- a/content/browser/indexed_db/indexed_db_transaction.h
+++ b/content/browser/indexed_db/indexed_db_transaction.h
@@ -13,6 +13,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
 #include "content/browser/indexed_db/indexed_db_database.h"
 #include "content/browser/indexed_db/indexed_db_database_error.h"
@@ -22,15 +23,18 @@ namespace content {
 class IndexedDBCursor;
 class IndexedDBDatabaseCallbacks;
 
-class IndexedDBTransaction : public base::RefCounted<IndexedDBTransaction> {
+class CONTENT_EXPORT IndexedDBTransaction
+    : public NON_EXPORTED_BASE(base::RefCounted<IndexedDBTransaction>) {
  public:
   typedef base::Callback<void(IndexedDBTransaction*)> Operation;
 
-  IndexedDBTransaction(int64 id,
-                       scoped_refptr<IndexedDBDatabaseCallbacks> callbacks,
-                       const std::set<int64>& object_store_ids,
-                       indexed_db::TransactionMode,
-                       IndexedDBDatabase* db);
+  IndexedDBTransaction(
+      int64 id,
+      scoped_refptr<IndexedDBDatabaseCallbacks> callbacks,
+      const std::set<int64>& object_store_ids,
+      indexed_db::TransactionMode,
+      IndexedDBDatabase* db,
+      IndexedDBBackingStore::Transaction* backing_store_transaction);
 
   virtual void Abort();
   void Commit();
@@ -55,7 +59,7 @@ class IndexedDBTransaction : public base::RefCounted<IndexedDBTransaction> {
     DCHECK_GE(pending_preemptive_events_, 0);
   }
   IndexedDBBackingStore::Transaction* BackingStoreTransaction() {
-    return &transaction_;
+    return transaction_.get();
   }
   int64 id() const { return id_; }
 
@@ -69,6 +73,7 @@ class IndexedDBTransaction : public base::RefCounted<IndexedDBTransaction> {
   };
 
   State state() const { return state_; }
+  bool IsTimeoutTimerRunning() const { return timeout_timer_.IsRunning(); }
 
   struct Diagnostics {
     base::Time creation_time;
@@ -90,6 +95,7 @@ class IndexedDBTransaction : public base::RefCounted<IndexedDBTransaction> {
 
   void ProcessTaskQueue();
   void CloseOpenCursors();
+  void Timeout();
 
   const int64 id_;
   const std::set<int64> object_store_ids_;
@@ -131,13 +137,19 @@ class IndexedDBTransaction : public base::RefCounted<IndexedDBTransaction> {
   TaskQueue preemptive_task_queue_;
   TaskStack abort_task_stack_;
 
-  IndexedDBBackingStore::Transaction transaction_;
+  scoped_ptr<IndexedDBBackingStore::Transaction> transaction_;
   bool backing_store_transaction_begun_;
 
   bool should_process_queue_;
   int pending_preemptive_events_;
 
   std::set<IndexedDBCursor*> open_cursors_;
+
+  // This timer is started after requests have been processed. If no subsequent
+  // requests are processed before the timer fires, assume the script is
+  // unresponsive and abort to unblock the transaction queue.
+  base::OneShotTimer<IndexedDBTransaction> timeout_timer_;
+
   Diagnostics diagnostics_;
 };
 
diff --git a/content/browser/indexed_db/indexed_db_transaction_unittest.cc b/content/browser/indexed_db/indexed_db_transaction_unittest.cc
new file mode 100644
index 0000000..3fc7271
--- /dev/null
+++ b/content/browser/indexed_db/indexed_db_transaction_unittest.cc
@@ -0,0 +1,82 @@
+// Copyright 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_transaction.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/message_loop/message_loop.h"
+#include "base/strings/utf_string_conversions.h"
+#include "content/browser/indexed_db/indexed_db_fake_backing_store.h"
+#include "content/browser/indexed_db/mock_indexed_db_database_callbacks.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+namespace {
+
+class IndexedDBTransactionTest : public testing::Test {
+ public:
+  IndexedDBTransactionTest() {
+    IndexedDBFactory* factory = NULL;
+    backing_store_ = new IndexedDBFakeBackingStore();
+    db_ = IndexedDBDatabase::Create(ASCIIToUTF16("db"),
+                                    backing_store_,
+                                    factory,
+                                    IndexedDBDatabase::Identifier());
+  }
+
+  void RunPostedTasks() { message_loop_.RunUntilIdle(); }
+  void DummyOperation(IndexedDBTransaction* transaction) {}
+
+ protected:
+  scoped_refptr<IndexedDBFakeBackingStore> backing_store_;
+  scoped_refptr<IndexedDBDatabase> db_;
+
+ private:
+  base::MessageLoop message_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(IndexedDBTransactionTest);
+};
+
+TEST_F(IndexedDBTransactionTest, Timeout) {
+  const int64 id = 0;
+  const std::set<int64> scope;
+  const bool commit_success = true;
+  scoped_refptr<IndexedDBTransaction> transaction = new IndexedDBTransaction(
+      id,
+      new MockIndexedDBDatabaseCallbacks(),
+      scope,
+      indexed_db::TRANSACTION_READ_ONLY,
+      db_,
+      new IndexedDBFakeBackingStore::FakeTransaction(commit_success));
+  db_->TransactionCreated(transaction);
+
+  // No conflicting transactions, so coordinator will start it immediately:
+  EXPECT_EQ(IndexedDBTransaction::STARTED, transaction->state());
+  EXPECT_FALSE(transaction->IsTimeoutTimerRunning());
+
+  // Schedule a task - timer won't be started until it's processed.
+  transaction->ScheduleTask(base::Bind(
+      &IndexedDBTransactionTest::DummyOperation, base::Unretained(this)));
+  EXPECT_FALSE(transaction->IsTimeoutTimerRunning());
+
+  RunPostedTasks();
+  EXPECT_TRUE(transaction->IsTimeoutTimerRunning());
+
+  // Abort should cancel the timer.
+  transaction->Abort();
+  EXPECT_EQ(IndexedDBTransaction::FINISHED, transaction->state());
+  EXPECT_FALSE(transaction->IsTimeoutTimerRunning());
+
+  // This task will be ignored.
+  transaction->ScheduleTask(base::Bind(
+      &IndexedDBTransactionTest::DummyOperation, base::Unretained(this)));
+  EXPECT_EQ(IndexedDBTransaction::FINISHED, transaction->state());
+  EXPECT_FALSE(transaction->IsTimeoutTimerRunning());
+}
+
+}  // namespace
+
+}  // namespace content
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index 3c405f7..ba42635 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -394,6 +394,7 @@
         'browser/indexed_db/indexed_db_fake_backing_store.h',
         'browser/indexed_db/indexed_db_leveldb_coding_unittest.cc',
         'browser/indexed_db/indexed_db_quota_client_unittest.cc',
+        'browser/indexed_db/indexed_db_transaction_unittest.cc',
         'browser/indexed_db/indexed_db_unittest.cc',
         'browser/indexed_db/mock_indexed_db_callbacks.cc',
         'browser/indexed_db/mock_indexed_db_callbacks.h',