Accurate transfer of SerializedScriptValue allocation costs.

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

Index: third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
index d9d92bc33daabb6ea1bc86a20eb8bcd5a5367da3..30ebd48935b15bb80db3a25cd28ed3429b728104 100644
--- a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
@@ -446,28 +446,36 @@ SerializedScriptValue::transferArrayBufferContents(
   return contents;
 }
 
-void SerializedScriptValue::unregisterMemoryAllocatedByCurrentScriptContext() {
-  // If the caller is the only one holding a reference then this serialized
-  // value hasn't transferred ownership & no unregistration of allocation
-  // costs wanted.
-  if (hasOneRef() || m_adjustTransferableExternalAllocationOnContextTransfer)
-    return;
+void SerializedScriptValue::prepareForTransferringContext() {
+  // Unlikely that this is non-zero, but discount any already registered
+  // external allocation for this SerializedScriptValue payload.
   if (m_externallyAllocatedMemory) {
     v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(
         -static_cast<int64_t>(m_externallyAllocatedMemory));
     m_externallyAllocatedMemory = 0;
   }
+
+  DCHECK(!m_adjustTransferableExternalAllocationOnContextTransfer);
+  // Mark value as needing re-registration of external allocation
+  // costs in its target context, as handled by
+  // |registerMemoryAllocatedWithCurrentScriptContext()|.
+  //
   // TODO: if other transferables start accounting for their external
   // allocations with V8, extend this with corresponding cases.
-  if (m_arrayBufferContentsArray) {
+  if (m_arrayBufferContentsArray)
+    m_adjustTransferableExternalAllocationOnContextTransfer = true;
+}
+
+void SerializedScriptValue::finalizeTransferringContext() {

[jbroman, 2017/03/13 15:01:14]

Why do we believe registerMemoryAllocatedWithCurrentScriptContext will be called
before finalizeTransferringContext? In the case that we're sending to a worker,
we don't create the MessageEvent until we arrive on the worker thread, which may
not be (and in fact, probably is not) before the sending thread runs
finalizeTransferringContext.

[sof, 2017/03/13 15:10:49]

I don't see how to accommodate that, so we will have to accept a cross-context
overestimate.

+  // The value successfully transferred context along with costs,
+  // finalize the operation by subtracting the external allocation
+  // cost of the transferables.
+  if (m_arrayBufferContentsArray &&
+      !m_adjustTransferableExternalAllocationOnContextTransfer) {
     for (auto& buffer : *m_arrayBufferContentsArray) {
       buffer.adjustExternalAllocatedMemoryUponContextTransfer(
           WTF::ArrayBufferContents::Leave);
     }
-    // Mark value as needing re-registration of external allocation
-    // costs in its target context, as handled by
-    // |registerMemoryAllocatedWithCurrentScriptContext()|.
-    m_adjustTransferableExternalAllocationOnContextTransfer = true;
   }
 }
 
@@ -481,12 +489,12 @@ void SerializedScriptValue::registerMemoryAllocatedWithCurrentScriptContext() {
   v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(diff);
   if (m_adjustTransferableExternalAllocationOnContextTransfer) {
     DCHECK(m_arrayBufferContentsArray);
+    m_adjustTransferableExternalAllocationOnContextTransfer = false;
     for (size_t i = 0; i < m_arrayBufferContentsArray->size(); ++i) {
       WTF::ArrayBufferContents& buffer = m_arrayBufferContentsArray->at(i);
       buffer.adjustExternalAllocatedMemoryUponContextTransfer(
           WTF::ArrayBufferContents::Enter);
     }
-    m_adjustTransferableExternalAllocationOnContextTransfer = false;
   }
 }