Wrap large IndexedDB values into Blobs before writing to LevelDB.

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

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 22 matching lines @@
 
 #include "bindings/core/v8/ScriptValue.h"
 #include "core/dom/DOMStringList.h"
 #include "core/dom/SuspendableObject.h"
 #include "core/events/EventListener.h"
 #include "core/events/EventTarget.h"
 #include "modules/EventModules.h"
 #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"
 #include "platform/blob/BlobData.h"
 #include "platform/heap/Handle.h"
 #include "platform/wtf/HashMap.h"
 #include "public/platform/WebBlobInfo.h"
 #include "public/platform/modules/indexeddb/WebIDBCursor.h"
 #include "public/platform/modules/indexeddb/WebIDBTypes.h"
 
@@ skipping 38 matching lines @@
   std::unique_ptr<WebIDBCallbacks> CreateWebCallbacks();
   void WebCallbacksDestroyed();
 
   DEFINE_ATTRIBUTE_EVENT_LISTENER(success);
   DEFINE_ATTRIBUTE_EVENT_LISTENER(error);
 
   void SetCursorDetails(IndexedDB::CursorType, WebIDBCursorDirection);
   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);
   virtual void OnSuccess();

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

Doesn't this one need a QueueResult() ? if not, can we delete it?

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

Done.

Thank you very much for catching this!!

FWIW, couldn't trigger OnSuccess() by iterating a cursor to the end of an index.
I did trigger it by issuing a get() for a non-existing key.

OnSuccess(int64_t) also needs to go through the queue, because it's used for
count(). I updated the patch.

   virtual void OnSuccess(IDBKey*, IDBKey* primary_key, PassRefPtr<IDBValue>);
 
   // Only IDBOpenDBRequest instances should receive these:
   virtual void OnBlocked(int64_t old_version) { NOTREACHED(); }
   virtual void OnUpgradeNeeded(int64_t old_version,
                                std::unique_ptr<WebIDBDatabase>,
                                const IDBDatabaseMetadata&,
                                WebIDBDataLoss,
                                String data_loss_message) {
     NOTREACHED();
   }
   virtual void OnSuccess(std::unique_ptr<WebIDBDatabase>,
                          const IDBDatabaseMetadata&) {
     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;
 
   // SuspendableObject
   void ContextDestroyed(ExecutionContext*) override;
 
   // EventTarget
   const AtomicString& InterfaceName() const override;
   ExecutionContext* GetExecutionContext() const final;
   void UncaughtExceptionInEventHandler() final;
 
   // Called by a version change transaction that has finished to set this
   // request back from DONE (following "upgradeneeded") back to PENDING (for
   // the upcoming "success" or "error").
   void TransactionDidFinishAndDispatch();
 
   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*);
   void DequeueEvent(Event*);
   virtual bool ShouldEnqueueEvent() const;
   void OnSuccessInternal(IDBAny*);
   void SetResult(IDBAny*);
 
   // 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.
   // Maintain the isolate so that all externally allocated memory can be
   // registered against it.
   v8::Isolate* isolate_;
 
  private:
   void SetResultCursor(IDBCursor*,
                        IDBKey*,
                        IDBKey* primary_key,
                        PassRefPtr<IDBValue>);
   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_;
 
   bool has_pending_activity_ = true;
   HeapVector<Member<Event>> enqueued_events_;
 
   // Only used if the result type will be a cursor.
   IndexedDB::CursorType cursor_type_ = IndexedDB::kCursorKeyAndValue;
   WebIDBCursorDirection cursor_direction_ = kWebIDBCursorDirectionNext;
   // When a cursor is continued/advanced, |result_| is cleared and
   // |pendingCursor_| holds it.
   Member<IDBCursor> pending_cursor_;
   // New state is not applied to the cursor object until the event is
   // dispatched.
   Member<IDBKey> cursor_key_;
   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;
   bool result_dirty_ = true;
 
   // Transactions should be aborted after event dispatch if an exception was
   // not caught. This is cleared before dispatch, set by a call to
   // UncaughtExceptionInEventHandler() during dispatch, and checked afterwards
   // to abort if necessary.
   bool did_throw_in_event_handler_ = false;
 
   // Pointer back to the WebIDBCallbacks that holds a persistent reference to
   // this object.
   WebIDBCallbacks* web_callbacks_ = nullptr;
 };
 
 }  // namespace blink
 
 #endif  // IDBRequest_h