Wrap large IndexedDB values into Blobs before writing to LevelDB.

third_party/WebKit/Source/modules/indexeddb/IDBRequest.h

Index: third_party/WebKit/Source/modules/indexeddb/IDBRequest.h
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBRequest.h b/third_party/WebKit/Source/modules/indexeddb/IDBRequest.h
index 9f271b74ea499cc7318d796a7b5fdf5c67f007cd..fdcca3e45ea43f55e0606b404a4ad5697021deaf 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBRequest.h
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBRequest.h
@@ -46,6 +46,7 @@
 #include "platform/blob/BlobData.h"
 #include "platform/heap/Handle.h"
 #include "platform/wtf/HashMap.h"
+#include "platform/wtf/RefPtr.h"
 #include "public/platform/WebBlobInfo.h"
 #include "public/platform/modules/indexeddb/WebIDBCursor.h"
 #include "public/platform/modules/indexeddb/WebIDBTypes.h"
@@ -98,24 +99,22 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   void SetPendingCursor(IDBCursor*);
   void Abort();
 
-  void EnqueueResponse(DOMException*);
-  void EnqueueResponse(std::unique_ptr<WebIDBCursor>,
-                       IDBKey*,
-                       IDBKey* primary_key,
-                       PassRefPtr<IDBValue>);
-  void EnqueueResponse(IDBKey*);
-  void EnqueueResponse(PassRefPtr<IDBValue>);
-  void EnqueueResponse(const Vector<RefPtr<IDBValue>>&);
-  void EnqueueResponse();
-  void EnqueueResponse(IDBKey*, IDBKey* primary_key, PassRefPtr<IDBValue>);
+  void HandleResponse(DOMException*);

[jsbell, 2017/05/15 23:37:38]

Please add a comment covering the general behavior of HandleResponse, i.e. these
will either queue an event for dispatch immediately if possible, or go into the
transaction's queue waiting on request loader work needed for this response or a
previous request's response.

[pwnall, 2017/05/19 18:27:34]

Done.

+  void HandleResponse(IDBKey*);
+  void HandleResponse(std::unique_ptr<WebIDBCursor>,
+                      IDBKey*,
+                      IDBKey* primary_key,
+                      RefPtr<IDBValue>&&);
+  void HandleResponse(IDBKey*, IDBKey* primary_key, RefPtr<IDBValue>&&);
+  void HandleResponse(RefPtr<IDBValue>&&);
+  void HandleResponse(const Vector<RefPtr<IDBValue>>&);
+  void HandleResponse(int64_t);

[jsbell, 2017/05/15 23:37:38]

Since it's non-obvious, add the parameter name here.

[pwnall, 2017/05/19 18:27:34]

Done.
I didn't add the parameter name before because it's ambiguous -- it can either
be a result for count() or an old database version number for openDatabase().
(there might be other meanings, I haven't fully absorbed the browser-side
IndexedDB implementation)
 

+  void HandleResponse();

[jsbell, 2017/05/15 23:37:38]

At one point we had documentation in the .h files about which IDB methods these
result types correspond to. I would not object to adding them back.

[pwnall, 2017/05/19 18:27:34]

I agree!

Can I do that in a follow-up CL? Making sure that I cover every callback
requires a bit of work, and I'd rather not hold this CL up (and having to keep
rebasing it).

[jsbell, 2017/05/22 21:54:19]

sgtm

 
   // Only used in webkitGetDatabaseNames(), which is deprecated and hopefully
   // going away soon.
   void EnqueueResponse(const Vector<String>&);
 
-  // Overridden by IDBOpenDBRequest.
-  virtual void EnqueueResponse(int64_t);
-
   // Only IDBOpenDBRequest instances should receive these:
   virtual void EnqueueBlocked(int64_t old_version) { NOTREACHED(); }
   virtual void EnqueueUpgradeNeeded(int64_t old_version,
@@ -148,11 +147,29 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
 
   IDBCursor* GetResultCursor() const;
 
-  void StorePutOperationBlobs(
-      HashMap<String, RefPtr<BlobDataHandle>> blob_handles) {
-    transit_blob_handles_ = std::move(blob_handles);
+  // Used to hang onto Blobs until the browser process handles the request.
+  //
+  // Blobs are ref-counted on the browser side, and BlobDataHandles manage
+  // references from renderers. When a BlobDataHandle gets destroyed, the
+  // browser-side Blob gets derefenced, which might cause it to be destroyed as
+  // well.
+  //
+  // After script uses a Blob in a put() request, the Blink-side Blob object
+  // (which hangs onto the BlobDataHandle) may get garbage-collected. IDBRequest
+  // needs to hang onto the BlobDataHandle as well, to avoid having the
+  // browser-side Blob get destroyed before the IndexedDB request is processed.
+  inline Vector<RefPtr<BlobDataHandle>>* transit_blob_handles() {
+    return &transit_blob_handles_;
   }
 
+#if DCHECK_IS_ON()
+  inline bool TransactionHasQueuedResults() const {

[jsbell, 2017/05/15 23:37:38]

Why did you decide to make this debug-only? Seems like it would be useful in the
HandleResult methods?

[pwnall, 2017/05/19 18:27:34]

This method does an extra check -- transaction_ != nullptr. After some struggles
getting  IDBRequestTest.EventsAfterStopping to pass, I'm starting to think that
all the methods might need to handle the nullptr case. If that's the case, I'll
switch to using this method. 

+    return transaction_ && transaction_->HasQueuedResults();
+  }
+
+  inline IDBRequestQueueItem* QueueItem() const { return queue_item_; }
+#endif  // DCHECK_IS_ON()
+
  protected:
   IDBRequest(ScriptState*, IDBAny* source, IDBTransaction*);
   void EnqueueEvent(Event*);
@@ -161,10 +178,16 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   void EnqueueResultInternal(IDBAny*);
   void SetResult(IDBAny*);
 
+  // Overridden by IDBOpenDBRequest.
+  virtual void EnqueueResponse(int64_t);
+
   // EventTarget
   DispatchEventResult DispatchEventInternal(Event*) override;
 
+  // Can be nullptr for requests that are not associated with a transaction,
+  // i.e. delete requests and completed or unsuccessful open requests.
   Member<IDBTransaction> transaction_;
+
   ReadyState ready_state_ = PENDING;
   bool request_aborted_ = false;  // May be aborted by transaction then receive
                                   // async onsuccess; ignore vs. assert.
@@ -173,14 +196,26 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   v8::Isolate* isolate_;
 
  private:
+  // Calls into OnError() and OnSuccess().

[jsbell, 2017/05/15 23:37:38]

Comment is obsolete

[pwnall, 2017/05/19 18:27:34]

Done.
Updated the method name. Thank you for catching this!

+  friend class IDBRequestQueueItem;
+
   void SetResultCursor(IDBCursor*,
                        IDBKey*,
                        IDBKey* primary_key,
-                       PassRefPtr<IDBValue>);
+                       RefPtr<IDBValue>&&);
   void AckReceivedBlobs(const IDBValue*);
   void AckReceivedBlobs(const Vector<RefPtr<IDBValue>>&);
 
-  void ClearPutOperationBlobs() { transit_blob_handles_.clear(); }
+  void EnqueueResponse(DOMException*);
+  void EnqueueResponse(IDBKey*);
+  void EnqueueResponse(std::unique_ptr<WebIDBCursor>,
+                       IDBKey*,
+                       IDBKey* primary_key,
+                       RefPtr<IDBValue>&&);
+  void EnqueueResponse(IDBKey*, IDBKey* primary_key, RefPtr<IDBValue>&&);
+  void EnqueueResponse(RefPtr<IDBValue>&&);
+  void EnqueueResponse(const Vector<RefPtr<IDBValue>>&);
+  void EnqueueResponse();
 
   Member<IDBAny> source_;
   Member<IDBAny> result_;
@@ -201,7 +236,7 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   Member<IDBKey> cursor_primary_key_;
   RefPtr<IDBValue> cursor_value_;
 
-  HashMap<String, RefPtr<BlobDataHandle>> transit_blob_handles_;
+  Vector<RefPtr<BlobDataHandle>> transit_blob_handles_;
 
   bool did_fire_upgrade_needed_event_ = false;
   bool prevent_propagation_ = false;
@@ -216,6 +251,12 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   // Pointer back to the WebIDBCallbacks that holds a persistent reference to
   // this object.
   WebIDBCallbacks* web_callbacks_ = nullptr;
+
+#if DCHECK_IS_ON()
+  // Non-null while this request's is queued behind other requests that are

[jsbell, 2017/05/15 23:37:38]

nit: grammar: 'request is'

[pwnall, 2017/05/19 18:27:34]

Done.

+  // still getting post-processed.
+  IDBRequestQueueItem* queue_item_ = nullptr;
+#endif  // DCHECK_IS_ON()
 };
 
 }  // namespace blink