[Refactoring] Clean up code for serializing ScriptValues

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

Index: third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
index f143d25720079fb6e9fbe46b92f409591d3cfd6e..455cea3861b42f5026264bb442c140e69095e7a1 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
@@ -36,6 +36,8 @@
 
 namespace blink {
 
+namespace {
+
 // This code implements the HTML5 Structured Clone algorithm:
 // http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#safe-passing-of-structured-data
 
@@ -65,9 +67,9 @@ private:
     ZigZag();
 };
 
-static const int maxDepth = 20000;
+const int maxDepth = 20000;
 
-static bool shouldCheckForCycles(int depth)
+bool shouldCheckForCycles(int depth)
 {
     ASSERT(depth >= 0);
     // Since we are not required to spot the cycle as soon as it
@@ -76,6 +78,60 @@ static bool shouldCheckForCycles(int depth)
     return !(depth & (depth - 1));
 }
 
+v8::Local<v8::Object> toV8Object(MessagePort* impl, v8::Local<v8::Object> creationContext, v8::Isolate* isolate)

[Yuki, 2016/06/28 08:28:47]

No need to address in this CL but just a suggestion.

1. We can use template functions for these utility functions.
2. For the empty handle, it's safe to convert it to another type.  So I think we
can write
    toV8(impl, creationContext, isolate).As<v8::Object>()
3. Is that possible that |impl| is nullptr?  If not, what we need is one-line
code above.
  If so, do we need these functions at all?

Note that toV8(nullptr, ...) returns v8::Null that is not type of v8::Object.

[peria, 2016/06/28 08:59:19]

Thank you for pointing this out.
I'll do it in another CL.

+{
+    if (!impl)
+        return v8::Local<v8::Object>();
+    v8::Local<v8::Value> wrapper = toV8(impl, creationContext, isolate);
+    if (wrapper.IsEmpty())
+        return v8::Local<v8::Object>();
+    DCHECK(wrapper->IsObject());
+    return wrapper.As<v8::Object>();
+}
+
+v8::Local<v8::Object> toV8Object(ImageBitmap* impl, v8::Local<v8::Object> creationContext, v8::Isolate* isolate)
+{
+    if (!impl)
+        return v8::Local<v8::Object>();
+    v8::Local<v8::Value> wrapper = toV8(impl, creationContext, isolate);
+    if (wrapper.IsEmpty())
+        return v8::Local<v8::Object>();
+    return wrapper.As<v8::Object>();
+}
+
+v8::Local<v8::Object> toV8Object(OffscreenCanvas* impl, v8::Local<v8::Object> creationContext, v8::Isolate* isolate)
+{
+    if (!impl)
+        return v8::Local<v8::Object>();
+    v8::Local<v8::Value> wrapper = toV8(impl, creationContext, isolate);
+    if (wrapper.IsEmpty())
+        return v8::Local<v8::Object>();
+    return wrapper.As<v8::Object>();
+}
+
+v8::Local<v8::Object> toV8Object(DOMArrayBufferBase* impl, v8::Local<v8::Object> creationContext, v8::Isolate* isolate)
+{
+    if (!impl)
+        return v8::Local<v8::Object>();
+    v8::Local<v8::Value> wrapper = toV8(impl, creationContext, isolate);
+    if (wrapper.IsEmpty())
+        return v8::Local<v8::Object>();
+    return wrapper.As<v8::Object>();
+}
+
+// Returns true if the provided object is to be considered a 'host object', as used in the
+// HTML5 structured clone algorithm.
+bool isHostObject(v8::Local<v8::Object> object)
+{
+    // If the object has any internal fields, then we won't be able to serialize or deserialize
+    // them; conveniently, this is also a quick way to detect DOM wrapper objects, because
+    // the mechanism for these relies on data stored in these fields. We should
+    // catch external array data as a special case.
+    return object->InternalFieldCount();
+}
+
+} // namespace
+
 void SerializedScriptValueWriter::writeUndefined()
 {
     append(UndefinedTag);
@@ -651,59 +707,7 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::CollectionState<T>::adv
     return serializer.writeCollection<T>(m_length, this);
 }
 
-static v8::Local<v8::Object> toV8Object(MessagePort* impl, v8::Local<v8::Object> creationContext, v8::Isolate* isolate)
-{
-    if (!impl)
-        return v8::Local<v8::Object>();
-    v8::Local<v8::Value> wrapper = toV8(impl, creationContext, isolate);
-    if (wrapper.IsEmpty())
-        return v8::Local<v8::Object>();
-    ASSERT(wrapper->IsObject());
-    return wrapper.As<v8::Object>();
-}
-
-static v8::Local<v8::Object> toV8Object(ImageBitmap* impl, v8::Local<v8::Object> creationContext, v8::Isolate* isolate)
-{
-    if (!impl)
-        return v8::Local<v8::Object>();
-    v8::Local<v8::Value> wrapper = toV8(impl, creationContext, isolate);
-    if (wrapper.IsEmpty())
-        return v8::Local<v8::Object>();
-    return wrapper.As<v8::Object>();
-}
-
-static v8::Local<v8::Object> toV8Object(OffscreenCanvas* impl, v8::Local<v8::Object> creationContext, v8::Isolate* isolate)
-{
-    if (!impl)
-        return v8::Local<v8::Object>();
-    v8::Local<v8::Value> wrapper = toV8(impl, creationContext, isolate);
-    if (wrapper.IsEmpty())
-        return v8::Local<v8::Object>();
-    return wrapper.As<v8::Object>();
-}
-
-static v8::Local<v8::Object> toV8Object(DOMArrayBufferBase* impl, v8::Local<v8::Object> creationContext, v8::Isolate* isolate)
-{
-    if (!impl)
-        return v8::Local<v8::Object>();
-    v8::Local<v8::Value> wrapper = toV8(impl, creationContext, isolate);
-    if (wrapper.IsEmpty())
-        return v8::Local<v8::Object>();
-    return wrapper.As<v8::Object>();
-}
-
-// Returns true if the provided object is to be considered a 'host object', as used in the
-// HTML5 structured clone algorithm.
-static bool isHostObject(v8::Local<v8::Object> object)
-{
-    // If the object has any internal fields, then we won't be able to serialize or deserialize
-    // them; conveniently, this is also a quick way to detect DOM wrapper objects, because
-    // the mechanism for these relies on data stored in these fields. We should
-    // catch external array data as a special case.
-    return object->InternalFieldCount();
-}
-
-ScriptValueSerializer::ScriptValueSerializer(SerializedScriptValueWriter& writer, const Transferables* transferables, WebBlobInfoArray* blobInfo, ScriptState* scriptState)
+ScriptValueSerializer::ScriptValueSerializer(SerializedScriptValueWriter& writer, WebBlobInfoArray* blobInfo, ScriptState* scriptState)
     : m_scriptState(scriptState)
     , m_writer(writer)
     , m_tryCatch(scriptState->isolate())
@@ -714,8 +718,6 @@ ScriptValueSerializer::ScriptValueSerializer(SerializedScriptValueWriter& writer
     , m_blobDataHandles(nullptr)
 {
     DCHECK(!m_tryCatch.HasCaught());
-    if (transferables)
-        copyTransferables(*transferables);
 }
 
 void ScriptValueSerializer::copyTransferables(const Transferables& transferables)
@@ -756,8 +758,13 @@ void ScriptValueSerializer::copyTransferables(const Transferables& transferables
 
 PassRefPtr<SerializedScriptValue> ScriptValueSerializer::serialize(v8::Local<v8::Value> value, Transferables* transferables, ExceptionState& exceptionState)
 {
+    DCHECK(!m_blobDataHandles);
+
     RefPtr<SerializedScriptValue> serializedValue = SerializedScriptValue::create();
+
     m_blobDataHandles = &serializedValue->blobDataHandles();
+    if (transferables)
+        copyTransferables(*transferables);
 
     v8::HandleScope scope(isolate());
     writer().writeVersion();
@@ -771,7 +778,7 @@ PassRefPtr<SerializedScriptValue> ScriptValueSerializer::serialize(v8::Local<v8:
         break;
     case Status::InputError:
     case Status::DataCloneError:
-        exceptionState.throwDOMException(blink::DataCloneError, errorMessage());
+        exceptionState.throwDOMException(blink::DataCloneError, m_errorMessage);
         break;
     case Status::JSException:
         exceptionState.rethrowV8Exception(m_tryCatch.Exception());
@@ -780,8 +787,6 @@ PassRefPtr<SerializedScriptValue> ScriptValueSerializer::serialize(v8::Local<v8:
         NOTREACHED();
     }
 
-    m_blobDataHandles = nullptr;
-
     return serializedValue.release();
 }
 
@@ -817,7 +822,7 @@ String ScriptValueSerializer::serializeNullValue()
     return valueWriter.takeWireString();
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerialize(v8::Local<v8::Value> value, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerialize(v8::Local<v8::Value> value, StateBase* next)
 {
     m_writer.writeReferenceCount(m_nextObjectReference);
 
@@ -858,7 +863,7 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerialize(v8::Local<v
     return nullptr;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeObject(v8::Local<v8::Object> object, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeObject(v8::Local<v8::Object> object, StateBase* next)
 {
     DCHECK(!object.IsEmpty());
 
@@ -954,62 +959,60 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeObject(v8::L
     return startObjectState(object, next);
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeArrayBuffer(v8::Local<v8::Value> arrayBuffer, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeArrayBuffer(v8::Local<v8::Value> arrayBuffer, StateBase* next)
 {
     return doSerialize(arrayBuffer, next);
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::checkException(ScriptValueSerializer::StateBase* state)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::checkException(StateBase* state)
 {
     return m_tryCatch.HasCaught() ? handleError(Status::JSException, "", state) : nullptr;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeObject(uint32_t numProperties, ScriptValueSerializer::StateBase* state)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeObject(uint32_t numProperties, StateBase* state)
 {
     m_writer.writeObject(numProperties);
     return pop(state);
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeSparseArray(uint32_t numProperties, uint32_t length, ScriptValueSerializer::StateBase* state)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeSparseArray(uint32_t numProperties, uint32_t length, StateBase* state)
 {
     m_writer.writeSparseArray(numProperties, length);
     return pop(state);
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeDenseArray(uint32_t numProperties, uint32_t length, ScriptValueSerializer::StateBase* state)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeDenseArray(uint32_t numProperties, uint32_t length, StateBase* state)
 {
     m_writer.writeDenseArray(numProperties, length);
     return pop(state);
 }
 
 template <>
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeCollection<v8::Map>(uint32_t length, ScriptValueSerializer::StateBase* state)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeCollection<v8::Map>(uint32_t length, StateBase* state)
 {
     m_writer.writeMap(length);
     return pop(state);
 }
 
 template <>
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeCollection<v8::Set>(uint32_t length, ScriptValueSerializer::StateBase* state)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeCollection<v8::Set>(uint32_t length, StateBase* state)
 {
     m_writer.writeSet(length);
     return pop(state);
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::handleError(ScriptValueSerializer::Status errorStatus, const String& message, ScriptValueSerializer::StateBase* state)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::handleError(ScriptValueSerializer::Status errorStatus, const String& message, StateBase* state)
 {
     DCHECK(errorStatus != Status::Success);
     m_status = errorStatus;
     m_errorMessage = message;
     while (state) {
-        StateBase* tmp = state->nextState();
-        delete state;
-        state = tmp;
+        state = pop(state);
     }
     return new ErrorState;
 }
 
-bool ScriptValueSerializer::checkComposite(ScriptValueSerializer::StateBase* top)
+bool ScriptValueSerializer::checkComposite(StateBase* top)
 {
     ASSERT(top);
     if (m_depth > maxDepth)
@@ -1052,11 +1055,11 @@ void ScriptValueSerializer::writeBooleanObject(v8::Local<v8::Value> value)
     m_writer.writeBooleanObject(booleanObject->ValueOf());
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeBlob(v8::Local<v8::Value> value, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeBlob(v8::Local<v8::Value> value, StateBase* next)
 {
     Blob* blob = V8Blob::toImpl(value.As<v8::Object>());
     if (!blob)
-        return 0;
+        return nullptr;
     if (blob->isClosed())
         return handleError(Status::DataCloneError, "A Blob object has been closed, and could therefore not be cloned.", next);
     int blobIndex = -1;
@@ -1065,10 +1068,10 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeBlob(v8::Local<v8:
         m_writer.writeBlobIndex(blobIndex);
     else
         m_writer.writeBlob(blob->uuid(), blob->type(), blob->size());
-    return 0;
+    return nullptr;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeCompositorProxy(v8::Local<v8::Value> value, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeCompositorProxy(v8::Local<v8::Value> value, StateBase* next)
 {
     CompositorProxy* compositorProxy = V8CompositorProxy::toImpl(value.As<v8::Object>());
     if (!compositorProxy)
@@ -1079,11 +1082,11 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeCompositorProxy(v8
     return nullptr;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeFile(v8::Local<v8::Value> value, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeFile(v8::Local<v8::Value> value, StateBase* next)
 {
     File* file = V8File::toImpl(value.As<v8::Object>());
     if (!file)
-        return 0;
+        return nullptr;
     if (file->isClosed())
         return handleError(Status::DataCloneError, "A File object has been closed, and could therefore not be cloned.", next);
     int blobIndex = -1;
@@ -1094,14 +1097,14 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeFile(v8::Local<v8:
     } else {
         m_writer.writeFile(*file);
     }
-    return 0;
+    return nullptr;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeFileList(v8::Local<v8::Value> value, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeFileList(v8::Local<v8::Value> value, StateBase* next)
 {
     FileList* fileList = V8FileList::toImpl(value.As<v8::Object>());
     if (!fileList)
-        return 0;
+        return nullptr;
     unsigned length = fileList->length();
     Vector<int> blobIndices;
     for (unsigned i = 0; i < length; ++i) {
@@ -1120,7 +1123,7 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeFileList(v8::Local
         m_writer.writeFileListIndex(blobIndices);
     else
         m_writer.writeFileList(*fileList);
-    return 0;
+    return nullptr;
 }
 
 void ScriptValueSerializer::writeImageData(v8::Local<v8::Value> value)
@@ -1132,7 +1135,7 @@ void ScriptValueSerializer::writeImageData(v8::Local<v8::Value> value)
     m_writer.writeImageData(imageData->width(), imageData->height(), pixelArray->data(), pixelArray->length());
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyImageBitmap(v8::Local<v8::Object> object, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyImageBitmap(v8::Local<v8::Object> object, StateBase* next)
 {
     ImageBitmap* imageBitmap = V8ImageBitmap::toImpl(object);
     if (!imageBitmap)
@@ -1157,12 +1160,12 @@ void ScriptValueSerializer::writeRegExp(v8::Local<v8::Value> value)
     m_writer.writeRegExp(regExp->GetSource(), regExp->GetFlags());
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBufferView(v8::Local<v8::Object> object, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBufferView(v8::Local<v8::Object> object, StateBase* next)
 {
     ASSERT(!object.IsEmpty());
     DOMArrayBufferView* arrayBufferView = V8ArrayBufferView::toImpl(object);
     if (!arrayBufferView)
-        return 0;
+        return nullptr;
     if (!arrayBufferView->bufferBase())
         return handleError(Status::DataCloneError, "An ArrayBuffer could not be cloned.", next);
     v8::Local<v8::Value> underlyingBuffer = toV8(arrayBufferView->bufferBase(), m_scriptState->context()->Global(), isolate());
@@ -1183,10 +1186,10 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBuffer
     // (or without an additional tag that would allow us to do two-stage construction
     // like we do for Objects and Arrays).
     greyObject(object);
-    return 0;
+    return nullptr;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBuffer(v8::Local<v8::Object> object, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBuffer(v8::Local<v8::Object> object, StateBase* next)
 {
     DOMArrayBuffer* arrayBuffer = V8ArrayBuffer::toImpl(object);
     if (!arrayBuffer)
@@ -1204,7 +1207,7 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBuffer
     return nullptr;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeTransferredOffscreenCanvas(v8::Local<v8::Value> value, uint32_t index, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeTransferredOffscreenCanvas(v8::Local<v8::Value> value, uint32_t index, StateBase* next)
 {
     OffscreenCanvas* offscreenCanvas = V8OffscreenCanvas::toImpl(value.As<v8::Object>());
     if (!offscreenCanvas)
@@ -1217,14 +1220,14 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::writeTransferredOffscre
     return nullptr;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::writeTransferredSharedArrayBuffer(v8::Local<v8::Value> value, uint32_t index, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::writeTransferredSharedArrayBuffer(v8::Local<v8::Value> value, uint32_t index, StateBase* next)
 {
     ASSERT(RuntimeEnabledFeatures::sharedArrayBufferEnabled());
     DOMSharedArrayBuffer* sharedArrayBuffer = V8SharedArrayBuffer::toImpl(value.As<v8::Object>());
     if (!sharedArrayBuffer)
         return 0;
     m_writer.writeTransferredSharedArrayBuffer(index);
-    return 0;
+    return nullptr;
 }
 
 bool ScriptValueSerializer::shouldSerializeDensely(uint32_t length, uint32_t propertyCount)
@@ -1236,7 +1239,7 @@ bool ScriptValueSerializer::shouldSerializeDensely(uint32_t length, uint32_t pro
     return 6 * propertyCount >= length;
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::startArrayState(v8::Local<v8::Array> array, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::startArrayState(v8::Local<v8::Array> array, StateBase* next)
 {
     v8::Local<v8::Array> propertyNames;
     if (!array->GetOwnPropertyNames(context()).ToLocal(&propertyNames))
@@ -1244,11 +1247,12 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::startArrayState(v8::Loc
     uint32_t length = array->Length();
 
     if (shouldSerializeDensely(length, propertyNames->Length())) {
-        m_writer.writeGenerateFreshDenseArray(length);
         // In serializing a dense array, indexed properties are ignored, so we get
         // non indexed own property names here.
         if (!array->GetPropertyNames(context(), v8::KeyCollectionMode::kIncludePrototypes, static_cast<v8::PropertyFilter>(v8::ONLY_ENUMERABLE | v8::SKIP_SYMBOLS), v8::IndexFilter::kSkipIndices).ToLocal(&propertyNames))
             return checkException(next);
+
+        m_writer.writeGenerateFreshDenseArray(length);
         return push(new DenseArrayState(array, propertyNames, next, isolate()));
     }
 
@@ -1256,19 +1260,19 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::startArrayState(v8::Loc
     return push(new SparseArrayState(array, propertyNames, next, isolate()));
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::startMapState(v8::Local<v8::Map> map, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::startMapState(v8::Local<v8::Map> map, StateBase* next)
 {
     m_writer.writeGenerateFreshMap();
     return push(new MapState(map, next));
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::startSetState(v8::Local<v8::Set> set, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::startSetState(v8::Local<v8::Set> set, StateBase* next)
 {
     m_writer.writeGenerateFreshSet();
     return push(new SetState(set, next));
 }
 
-ScriptValueSerializer::StateBase* ScriptValueSerializer::startObjectState(v8::Local<v8::Object> object, ScriptValueSerializer::StateBase* next)
+ScriptValueSerializer::StateBase* ScriptValueSerializer::startObjectState(v8::Local<v8::Object> object, StateBase* next)
 {
     m_writer.writeGenerateFreshObject();
     // FIXME: check not a wrapper