IndexedDB: Make getAll() requests fail if result exceeds IPC limits

The (experimental) IDBObjectStore/IDBIndex.getAll()/getAllKeys()
methods can return an arbitrary number of results. When the resulting
payload would exceed the IPC message, fail the request with an
UnknownError per discussion w/ Moz.

BUG=478949
R=cmumford@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#345920}

11 files changed, 146 insertions, 30 deletions

diff --git a/content/browser/indexed_db/indexed_db_browsertest.cc b/content/browser/indexed_db/indexed_db_browsertest.cc
index b0097ed..ff400ec 100644
--- a/content/browser/indexed_db/indexed_db_browsertest.cc
+++ b/content/browser/indexed_db/indexed_db_browsertest.cc
@@ -187,6 +187,16 @@ class IndexedDBBrowserTest : public ContentBrowserTest {
   DISALLOW_COPY_AND_ASSIGN(IndexedDBBrowserTest);
 };
 
+class IndexedDBBrowserTestWithExperimentalAPIs
+    : public IndexedDBBrowserTest,
+      public ::testing::WithParamInterface<const char*> {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(
+        switches::kEnableExperimentalWebPlatformFeatures);
+  }
+};
+
 IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CursorTest) {
   SimpleTest(GetTestUrl("indexeddb", "cursor_test.html"));
 }
@@ -232,6 +242,11 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CallbackAccounting) {
   SimpleTest(GetTestUrl("indexeddb", "callback_accounting.html"));
 }
 
+IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTestWithExperimentalAPIs,
+                       GetAllMaxMessageSize) {
+  SimpleTest(GetTestUrl("indexeddb", "getall_max_message_size.html"));
+}
+
 IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, DoesntHangTest) {
   SimpleTest(GetTestUrl("indexeddb", "transaction_run_forever.html"));
   CrashTab(shell()->web_contents());
@@ -679,16 +694,8 @@ static scoped_ptr<net::test_server::HttpResponse> CorruptDBRequestHandler(
 
 }  // namespace
 
-class IndexedDBBrowserCorruptionTest
-    : public IndexedDBBrowserTest,
-      public ::testing::WithParamInterface<const char*> {
- public:
-  void SetUpCommandLine(base::CommandLine* command_line) override {
-    // Experimental for IDBObjectStore.getAll()
-    command_line->AppendSwitch(
-        switches::kEnableExperimentalWebPlatformFeatures);
-  }
-};
+// Experimental for IDBObjectStore.getAll()
+using IndexedDBBrowserCorruptionTest = IndexedDBBrowserTestWithExperimentalAPIs;
 
 IN_PROC_BROWSER_TEST_P(IndexedDBBrowserCorruptionTest,
                        OperationOnCorruptedOpenDatabase) {
diff --git a/content/browser/indexed_db/indexed_db_class_factory.cc b/content/browser/indexed_db/indexed_db_class_factory.cc
index 284b40c..cb77cca 100644
--- a/content/browser/indexed_db/indexed_db_class_factory.cc
+++ b/content/browser/indexed_db/indexed_db_class_factory.cc
@@ -24,6 +24,14 @@ IndexedDBClassFactory* IndexedDBClassFactory::Get() {
     return s_factory.Pointer();
 }
 
+IndexedDBDatabase* IndexedDBClassFactory::CreateIndexedDBDatabase(
+    const base::string16& name,
+    IndexedDBBackingStore* backing_store,
+    IndexedDBFactory* factory,
+    const IndexedDBDatabase::Identifier& unique_identifier) {
+  return new IndexedDBDatabase(name, backing_store, factory, unique_identifier);
+}
+
 IndexedDBTransaction* IndexedDBClassFactory::CreateIndexedDBTransaction(
     int64 id,
     scoped_refptr<IndexedDBDatabaseCallbacks> callbacks,
diff --git a/content/browser/indexed_db/indexed_db_class_factory.h b/content/browser/indexed_db/indexed_db_class_factory.h
index 2fe0089..b2d2026 100644
--- a/content/browser/indexed_db/indexed_db_class_factory.h
+++ b/content/browser/indexed_db/indexed_db_class_factory.h
@@ -10,6 +10,7 @@
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
+#include "content/browser/indexed_db/indexed_db_database.h"
 #include "content/common/content_export.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBTypes.h"
 
@@ -19,8 +20,9 @@ class Iterator;
 
 namespace content {
 
-class IndexedDBDatabase;
+class IndexedDBBackingStore;
 class IndexedDBDatabaseCallbacks;
+class IndexedDBFactory;
 class IndexedDBTransaction;
 class LevelDBDatabase;
 class LevelDBIteratorImpl;
@@ -36,6 +38,12 @@ class CONTENT_EXPORT IndexedDBClassFactory {
 
   static void SetIndexedDBClassFactoryGetter(GetterCallback* cb);
 
+  virtual IndexedDBDatabase* CreateIndexedDBDatabase(
+      const base::string16& name,
+      IndexedDBBackingStore* backing_store,
+      IndexedDBFactory* factory,
+      const IndexedDBDatabase::Identifier& unique_identifier);
+
   virtual IndexedDBTransaction* CreateIndexedDBTransaction(
       int64 id,
       scoped_refptr<IndexedDBDatabaseCallbacks> callbacks,
diff --git a/content/browser/indexed_db/indexed_db_database.cc b/content/browser/indexed_db/indexed_db_database.cc
index 8628768..31d0e69 100644
--- a/content/browser/indexed_db/indexed_db_database.cc
+++ b/content/browser/indexed_db/indexed_db_database.cc
@@ -132,7 +132,8 @@ scoped_refptr<IndexedDBDatabase> IndexedDBDatabase::Create(
     const Identifier& unique_identifier,
     leveldb::Status* s) {
   scoped_refptr<IndexedDBDatabase> database =
-      new IndexedDBDatabase(name, backing_store, factory, unique_identifier);
+      IndexedDBClassFactory::Get()->CreateIndexedDBDatabase(
+          name, backing_store, factory, unique_identifier);
   *s = database->OpenInternal();
   if (s->ok())
     return database;
@@ -227,6 +228,10 @@ IndexedDBDatabase::~IndexedDBDatabase() {
   DCHECK(pending_delete_calls_.empty());
 }
 
+size_t IndexedDBDatabase::GetMaxMessageSizeInBytes() const {
+  return kMaxIDBMessageSizeInBytes;
+}
+
 scoped_ptr<IndexedDBConnection> IndexedDBDatabase::CreateConnection(
     scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
     int child_process_id) {
@@ -853,9 +858,11 @@ void IndexedDBDatabase::GetAllOperation(
       response_size += return_key.size_estimate();
     else
       response_size += return_value.SizeEstimate();
-    if (response_size > IPC::Channel::kMaximumMessageSize) {
-      // TODO(cmumford): Reach this limit more gracefully (crbug.com/478949)
-      break;
+    if (response_size > GetMaxMessageSizeInBytes()) {
+      callbacks->OnError(
+          IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
+                                 "Maximum IPC message size exceeded."));
+      return;
     }
 
     if (cursor_type == indexed_db::CURSOR_KEY_ONLY)
diff --git a/content/browser/indexed_db/indexed_db_database.h b/content/browser/indexed_db/indexed_db_database.h
index d9e7f66..66bfe5d 100644
--- a/content/browser/indexed_db/indexed_db_database.h
+++ b/content/browser/indexed_db/indexed_db_database.h
@@ -235,8 +235,19 @@ class CONTENT_EXPORT IndexedDBDatabase
                       scoped_refptr<IndexedDBCallbacks> callbacks,
                       IndexedDBTransaction* transaction);
 
+ protected:
+  IndexedDBDatabase(const base::string16& name,
+                    IndexedDBBackingStore* backing_store,
+                    IndexedDBFactory* factory,
+                    const Identifier& unique_identifier);
+  virtual ~IndexedDBDatabase();
+
+  // May be overridden in tests.
+  virtual size_t GetMaxMessageSizeInBytes() const;
+
  private:
   friend class base::RefCounted<IndexedDBDatabase>;
+  friend class IndexedDBClassFactory;
 
   class PendingDeleteCall;
   class PendingSuccessCall;
@@ -247,12 +258,6 @@ class CONTENT_EXPORT IndexedDBDatabase
   typedef std::list<PendingDeleteCall*> PendingDeleteCallList;
   typedef list_set<IndexedDBConnection*> ConnectionSet;
 
-  IndexedDBDatabase(const base::string16& name,
-                    IndexedDBBackingStore* backing_store,
-                    IndexedDBFactory* factory,
-                    const Identifier& unique_identifier);
-  ~IndexedDBDatabase();
-
   bool IsOpenConnectionBlocked() const;
   leveldb::Status OpenInternal();
   void RunVersionChangeTransaction(scoped_refptr<IndexedDBCallbacks> callbacks,
diff --git a/content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.cc b/content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.cc
index 4349ee2..dc02ff7 100644
--- a/content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.cc
+++ b/content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.cc
@@ -41,6 +41,22 @@ class FunctionTracer {
 
 namespace content {
 
+class IndexedDBTestDatabase : public IndexedDBDatabase {
+ public:
+  IndexedDBTestDatabase(const base::string16& name,
+                        IndexedDBBackingStore* backing_store,
+                        IndexedDBFactory* factory,
+                        const IndexedDBDatabase::Identifier& unique_identifier)
+      : IndexedDBDatabase(name, backing_store, factory, unique_identifier) {}
+
+ protected:
+  ~IndexedDBTestDatabase() override {}
+
+  size_t GetMaxMessageSizeInBytes() const override {
+    return 10 * 1024 * 1024;  // 10MB
+  }
+};
+
 class IndexedDBTestTransaction : public IndexedDBTransaction {
  public:
   IndexedDBTestTransaction(
@@ -234,6 +250,16 @@ MockBrowserTestIndexedDBClassFactory::MockBrowserTestIndexedDBClassFactory()
 MockBrowserTestIndexedDBClassFactory::~MockBrowserTestIndexedDBClassFactory() {
 }
 
+IndexedDBDatabase*
+MockBrowserTestIndexedDBClassFactory::CreateIndexedDBDatabase(
+    const base::string16& name,
+    IndexedDBBackingStore* backing_store,
+    IndexedDBFactory* factory,
+    const IndexedDBDatabase::Identifier& unique_identifier) {
+  return new IndexedDBTestDatabase(name, backing_store, factory,
+                                   unique_identifier);
+}
+
 IndexedDBTransaction*
 MockBrowserTestIndexedDBClassFactory::CreateIndexedDBTransaction(
     int64 id,
diff --git a/content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.h b/content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.h
index 1ce5c0e..1b77071 100644
--- a/content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.h
+++ b/content/browser/indexed_db/mock_browsertest_indexed_db_class_factory.h
@@ -11,6 +11,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "content/browser/indexed_db/indexed_db_backing_store.h"
 #include "content/browser/indexed_db/indexed_db_class_factory.h"
+#include "content/browser/indexed_db/indexed_db_database.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBTypes.h"
 
 namespace content {
@@ -36,6 +37,12 @@ class MockBrowserTestIndexedDBClassFactory : public IndexedDBClassFactory {
  public:
   MockBrowserTestIndexedDBClassFactory();
   ~MockBrowserTestIndexedDBClassFactory() override;
+
+  IndexedDBDatabase* CreateIndexedDBDatabase(
+      const base::string16& name,
+      IndexedDBBackingStore* backing_store,
+      IndexedDBFactory* factory,
+      const IndexedDBDatabase::Identifier& unique_identifier) override;
   IndexedDBTransaction* CreateIndexedDBTransaction(
       int64 id,
       scoped_refptr<IndexedDBDatabaseCallbacks> callbacks,
diff --git a/content/child/indexed_db/indexed_db_dispatcher.cc b/content/child/indexed_db/indexed_db_dispatcher.cc
index 78400d4..3862e3b 100644
--- a/content/child/indexed_db/indexed_db_dispatcher.cc
+++ b/content/child/indexed_db/indexed_db_dispatcher.cc
@@ -14,7 +14,6 @@
 #include "content/child/indexed_db/webidbcursor_impl.h"
 #include "content/child/indexed_db/webidbdatabase_impl.h"
 #include "content/child/thread_safe_sender.h"
-#include "content/common/indexed_db/indexed_db_constants.h"
 #include "content/common/indexed_db/indexed_db_messages.h"
 #include "ipc/ipc_channel.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBDatabaseCallbacks.h"
@@ -47,12 +46,8 @@ IndexedDBDispatcher* const kHasBeenDeleted =
 
 }  // unnamed namespace
 
-const size_t kMaxIDBValueSizeInBytes =
-    IPC::Channel::kMaximumMessageSize - kMaxIDBMessageOverhead;
-
 IndexedDBDispatcher::IndexedDBDispatcher(ThreadSafeSender* thread_safe_sender)
-    : thread_safe_sender_(thread_safe_sender),
-      max_put_value_size_(kMaxIDBValueSizeInBytes) {
+    : thread_safe_sender_(thread_safe_sender) {
   g_idb_dispatcher_tls.Pointer()->Set(this);
 }
 
diff --git a/content/child/indexed_db/indexed_db_dispatcher.h b/content/child/indexed_db/indexed_db_dispatcher.h
index 55f74ef..854878d 100644
--- a/content/child/indexed_db/indexed_db_dispatcher.h
+++ b/content/child/indexed_db/indexed_db_dispatcher.h
@@ -15,6 +15,7 @@
 #include "base/strings/nullable_string16.h"
 #include "content/child/worker_task_runner.h"
 #include "content/common/content_export.h"
+#include "content/common/indexed_db/indexed_db_constants.h"
 #include "ipc/ipc_sync_message_filter.h"
 #include "third_party/WebKit/public/platform/WebBlobInfo.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBCallbacks.h"
@@ -41,8 +42,6 @@ class WebIDBCursorImpl;
 class WebIDBDatabaseImpl;
 class ThreadSafeSender;
 
-CONTENT_EXPORT extern const size_t kMaxIDBValueSizeInBytes;
-
 // Handle the indexed db related communication for this context thread - the
 // main thread and each worker thread have their own copies.
 class CONTENT_EXPORT IndexedDBDispatcher : public WorkerTaskRunner::Observer {
@@ -257,7 +256,7 @@ class CONTENT_EXPORT IndexedDBDispatcher : public WorkerTaskRunner::Observer {
   // requests larger than this size will be rejected.
   // Used by unit tests to exercise behavior without allocating huge chunks
   // of memory.
-  size_t max_put_value_size_;
+  size_t max_put_value_size_ = kMaxIDBMessageSizeInBytes;
 
   // Careful! WebIDBCallbacks wraps non-threadsafe data types. It must be
   // destroyed and used on the same thread it was created on.
diff --git a/content/common/indexed_db/indexed_db_constants.h b/content/common/indexed_db/indexed_db_constants.h
index b6c6363..4cb4c7e 100644
--- a/content/common/indexed_db/indexed_db_constants.h
+++ b/content/common/indexed_db/indexed_db_constants.h
@@ -5,12 +5,17 @@
 #ifndef CONTENT_COMMON_INDEXED_DB_INDEXED_DB_CONSTANTS_H_
 #define CONTENT_COMMON_INDEXED_DB_INDEXED_DB_CONSTANTS_H_
 
+#include "ipc/ipc_channel.h"
+
 namespace content {
 
 const int32 kNoDatabase = -1;
 
 const size_t kMaxIDBMessageOverhead = 1024 * 1024;  // 1MB; arbitrarily chosen.
 
+const size_t kMaxIDBMessageSizeInBytes =
+    IPC::Channel::kMaximumMessageSize - kMaxIDBMessageOverhead;
+
 }  // namespace content
 
 #endif  // CONTENT_COMMON_INDEXED_DB_INDEXED_DB_CONSTANTS_H_
diff --git a/content/test/data/indexeddb/getall_max_message_size.html b/content/test/data/indexeddb/getall_max_message_size.html
new file mode 100644
index 0000000..3b35a02
--- /dev/null
+++ b/content/test/data/indexeddb/getall_max_message_size.html
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<!--
+  Copyright 2015 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.
+-->
+<head>
+<title>IDB Test that getAll() results exceeding IPC message size result in error</title>
+<script src="common.js"></script>
+<script>
+
+function test() {
+  indexedDBTest(prepareDatabase, runTest);
+}
+
+function prepareDatabase(event) {
+  var db = event.target.result;
+  var store = db.createObjectStore('store');
+
+  var ten_kilobytes = new Uint8Array(10 * 1024);
+
+  // 20MB total; in browser tests, the message limit is 10MB
+  for (var i = 0; i < 2000; ++i) {
+    store.put(ten_kilobytes, i);
+  }
+}
+
+var tx, request;
+function runTest(event) {
+  var db = event.target.result;
+  tx = db.transaction('store');
+  request = tx.objectStore('store').getAll();
+
+  request.onsuccess = unexpectedSuccessCallback;
+  request.onerror = function(e) {
+    shouldBeEqualToString('request.error.name', 'UnknownError');
+  };
+
+  tx.oncomplete = unexpectedCompleteCallback;
+  tx.onabort = done;
+}
+
+</script>
+</head>
+<body onLoad="test()">
+  <div id="status">Starting...</div>
+</body>
+</html>