postMessage(): transfer allocation costs along with value.

third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.h

Index: third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.h b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.h
index ceb9d63e64389f54d7ef4cec4ed2bf23c86c38de..271ec9a9d72cf2dd0f7238a647fbf720fdb67e77 100644
--- a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.h
+++ b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.h
@@ -128,13 +128,21 @@ class CORE_EXPORT SerializedScriptValue
       const ImageBitmapArray&,
       ExceptionState&);
 
-  // Informs the V8 about external memory allocated and owned by this object.
+  // Informs V8 about external memory allocated and owned by this object.
   // Large values should contribute to GC counters to eventually trigger a GC,
   // otherwise flood of postMessage() can cause OOM.
   // Ok to invoke multiple times (only adds memory once).
   // The memory registration is revoked automatically in destructor.
   void registerMemoryAllocatedWithCurrentScriptContext();
 
+  // Upon passing a serialized value from one context to another (via a
+  // postMessage()), the allocation amounts it has registered with the
+  // 'origining' context must be discharged, as the 'target' context will assume
+  // ownership of value. This method takes care of the first part of the
+  // external allocation bookkeeping, the above registration method the other
+  // half.
+  void unregisterMemoryAllocatedByCurrentScriptContext();

[jbroman, 2017/03/07 18:00:04]

Can you clarify why this is needed now? It looks like the MessageEvent (which
calls registerMemoryAllocatedWithCurrentScriptContext) isn't created until it's
received by the second thread, so would there ever be external memory registered
at the time this is called?

[sof, 2017/03/07 19:14:03]

 - transferables (arraybuffers) will be accounted wrt v8 when created.
 - when serialized and passed along via postMessage(), we neuter the "source"
   side of the transferables, passing along the array buffer contents.
 - the allocation cost still resides with the "source" and not the "target".
 - ..yet the "source" cannot access the buffer contents, the "target" can.
 - to address that unbalanced accounting, unregister*() discharges the
   external allocation cost from "source". Cost includes both that of
   the serialized payload (if registered) and the transferable buffers.
 - register*() will now _additionally_ also account for the external
   allocation cost of the transferables buffers when the "target" dispatches
   the incoming message from a postMessage(). It will however only do so
   if the allocation cost for these have been discharged via a
   corresponding unregister*(). This is, like before, done upon
   MessageEvent construction.

The upshot is that for a testcase like seen on the associated bug, v8 is now
able to schedule GCs better for the "target" worker context, preventing OOMs for
some workloads.

[jbroman, 2017/03/07 19:29:05]

Ah, it wasn't obvious to me that this was intended to balance
WTF::ArrayBufferContents::adjustAmountOfExternalAllocatedMemory.

Might it be reasonable to add a comment, or possibly even delegate the call to
v8::Isolate::AdjustAmountOfExternalAllocatedMemory to a method on
ArrayBufferContents, so that it's clear that the adjustments are intended to
balance the ones done there (which is fairly distant from here)?
Yeah, agreed that something along these lines will fix the bug.

[sof, 2017/03/08 07:07:17]

Added a method on ArrayBufferContents to handle the +/- of costs when
entering&leaving a context.

+
   const uint8_t* data() const { return m_dataBuffer.get(); }
   size_t dataLengthInBytes() const { return m_dataBufferSize; }