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 7a52cdae05087f05406296abce6fbc3a34a07261..bc5c2dfdf8692886311e36ecee21217a41e7e17c 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBRequest.h
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBRequest.h
@@ -40,6 +40,7 @@
 #include "modules/ModulesExport.h"
 #include "modules/indexeddb/IDBAny.h"
 #include "modules/indexeddb/IDBTransaction.h"
+#include "modules/indexeddb/IDBValueWrapper.h"
 #include "modules/indexeddb/IndexedDB.h"
 #include "platform/bindings/ActiveScriptWrappable.h"
 #include "platform/bindings/ScriptState.h"
@@ -98,13 +99,23 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   void SetPendingCursor(IDBCursor*);
   void Abort();
 
+  void QueueResult(DOMException*);
+  void QueueResult(IDBKey*);
+  void QueueResult(std::unique_ptr<WebIDBCursor>,
+                   IDBKey*,
+                   IDBKey* primary_key,
+                   PassRefPtr<IDBValue>);
+  void QueueResult(PassRefPtr<IDBValue>);
+  void QueueResult(const Vector<RefPtr<IDBValue>>&);
+  void QueueResult(IDBKey*, IDBKey* primary_key, PassRefPtr<IDBValue>);
+
   virtual void OnError(DOMException*);

[jsbell, 2017/05/05 00:29:37]

WDYT about making most of the OnXXXX methods protected/private and friend the
QueueItem?

(Also, I don't think these all need to be virtual any more?)

[pwnall, 2017/05/11 23:54:23]

Done.
Can't say no to de-virtualizing :)

+  virtual void OnSuccess(IDBKey*);
   virtual void OnSuccess(const Vector<String>&);
   virtual void OnSuccess(std::unique_ptr<WebIDBCursor>,
                          IDBKey*,
                          IDBKey* primary_key,
                          PassRefPtr<IDBValue>);
-  virtual void OnSuccess(IDBKey*);
   virtual void OnSuccess(PassRefPtr<IDBValue>);
   virtual void OnSuccess(const Vector<RefPtr<IDBValue>>&);
   virtual void OnSuccess(int64_t);
@@ -125,6 +136,10 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
     NOTREACHED();
   }
 
+  inline void ExtractOutgoingBlobHandlesFrom(IDBValueWrapper* value_wrapper) {

[dmurph, 2017/05/04 22:27:07]

I'd prefer we don't include and depend on the value wrapper api for this
specific method call - what if we just made the blob handles accessible and
mutable?

*HashMap<String, RefPtr<BlobDataHandle>> OutgoingBlobHandles()

[pwnall, 2017/05/11 23:54:23]

Done.

+    value_wrapper->ExtractBlobDataHandles(&outgoing_blob_handles_);
+  }
+
   // ScriptWrappable
   bool HasPendingActivity() const final;
 
@@ -143,11 +158,6 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
 
   IDBCursor* GetResultCursor() const;
 
-  void StorePutOperationBlobs(
-      HashMap<String, RefPtr<BlobDataHandle>> blob_handles) {
-    transit_blob_handles_ = std::move(blob_handles);
-  }
-
  protected:
   IDBRequest(ScriptState*, IDBAny* source, IDBTransaction*);
   void EnqueueEvent(Event*);
@@ -159,7 +169,10 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   // EventTarget
   DispatchEventResult DispatchEventInternal(Event*) override;
 
+  // Can be nullptr for requests that are not associated with a transaction,
+  // i.e. delete requests and unsuccessful open requests.

[jsbell, 2017/05/05 00:29:37]

And completed open requests, IIRC?

[pwnall, 2017/05/11 23:54:23]

Done. Case added.

The "i.e." there was intended to say I'm only providing examples. The list is
non-exhaustive, because I don't know all the cases off the top of my head :)

   Member<IDBTransaction> transaction_;
+
   ReadyState ready_state_ = PENDING;
   bool request_aborted_ = false;  // May be aborted by transaction then receive
                                   // async onsuccess; ignore vs. assert.
@@ -175,8 +188,6 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   void AckReceivedBlobs(const IDBValue*);
   void AckReceivedBlobs(const Vector<RefPtr<IDBValue>>&);
 
-  void ClearPutOperationBlobs() { transit_blob_handles_.clear(); }

[dmurph, 2017/05/04 22:27:07]

Requests can take forever for a GC, so please try to actively clean them up (see
the places this was called that you removed).

[pwnall, 2017/05/11 23:54:23]

I think I was doing that. See the "outgoing_blob_handles_.clear();" calls in
QueueResult(IDBKey) and QueueResult(DOMException). AFAIK these are the only
possible callbacks for put.

FWIW, I didn't remove the DCHECK from OnSuccessInternal(), so if I missed
something, it isn't covered by tests :)

What's the best way to express that the cleanup is still taking place? Put the
method back in?

-
   Member<IDBAny> source_;
   Member<IDBAny> result_;
   Member<DOMException> error_;
@@ -196,7 +207,8 @@ class MODULES_EXPORT IDBRequest : public EventTargetWithInlineData,
   Member<IDBKey> cursor_primary_key_;
   RefPtr<IDBValue> cursor_value_;
 
-  HashMap<String, RefPtr<BlobDataHandle>> transit_blob_handles_;

[dmurph, 2017/05/04 22:27:07]

Any reason for the name change? I like 'transit' better as it specifically means
the transit system (mojo/IPC/whatever), outgoing is a little less specific.

[pwnall, 2017/05/11 23:54:23]

Done. (switched it back)

"transit" seemed generic to me. Its biggest flaw IMO is that it doesn't disclose
the direction in which data flows. After my CL, IDBRequest's handling of both
outgoing and incoming Blobs will be non-trivial, so I would've liked to clearly
distinguish between the two. 

FWIW. I thought that "outgoing" implies "in transit", so I thought the rename is
a strict upgrade (more semantics stuffed in, still one word). I'm not a native
speaker though, so ¯\_(ツ)_/¯

+  // Keeps blobs alive while they are sent to the browser process.
+  Vector<RefPtr<BlobDataHandle>> outgoing_blob_handles_;
 
   bool did_fire_upgrade_needed_event_ = false;
   bool prevent_propagation_ = false;