[Refactoring] Clean up code for serializing ScriptValues

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "bindings/core/v8/ScriptValueSerializer.h"
 
 #include "bindings/core/v8/Transferables.h"
 #include "bindings/core/v8/V8ArrayBuffer.h"
 #include "bindings/core/v8/V8ArrayBufferView.h"
 #include "bindings/core/v8/V8Blob.h"
@@ skipping 18 matching lines @@
 #include "wtf/DateMath.h"
 #include "wtf/text/StringHash.h"
 #include "wtf/text/StringUTF8Adaptor.h"
 #include <memory>
 
 // FIXME: consider crashing in debug mode on deserialization errors
 // NOTE: be sure to change wireFormatVersion as necessary!
 
 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
 
 // ZigZag encoding helps VarInt encoding stay small for negative
 // numbers with small absolute values.
 class ZigZag {
 public:
     static uint32_t encode(uint32_t value)
     {
         if (value & (1U << 31))
             value = ((~value) << 1) + 1;
         else
             value <<= 1;
         return value;
     }
 
     static uint32_t decode(uint32_t value)
     {
         if (value & 1)
             value = ~(value >> 1);
         else
             value >>= 1;
         return value;
     }
 
 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
     // happens we can check for cycles only when the current depth
     // is a power of two.
     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);
 }
 
 void SerializedScriptValueWriter::writeNull()
 {
     append(NullTag);
 }
 
@@ skipping 555 matching lines @@
             return serializer.handleError(Status::JSException, "Failed to get an element while cloning a collection.", this);
         m_index++;
         if (StateBase* newState = serializer.checkException(this))
             return newState;
         if (StateBase* newState = serializer.doSerialize(value, this))
             return newState;
     }
     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())
     , m_depth(0)
     , m_status(Status::Success)
     , m_nextObjectReference(0)
     , m_blobInfo(blobInfo)
     , m_blobDataHandles(nullptr)
 {
     DCHECK(!m_tryCatch.HasCaught());
-    if (transferables)
-        copyTransferables(*transferables);
 }
 
 void ScriptValueSerializer::copyTransferables(const Transferables& transferables)
 {
     v8::Local<v8::Object> creationContext = m_scriptState->context()->Global();
 
     // Also kept in separate ObjectPools, iterate and copy the contents
     // of each kind of transferable vector.
 
     const auto& messagePorts = transferables.messagePorts;
@@ skipping 20 matching lines @@
     const auto& offscreenCanvases = transferables.offscreenCanvases;
     for (size_t i = 0; i < offscreenCanvases.size(); ++i) {
         v8::Local<v8::Object> v8OffscreenCanvas = toV8Object(offscreenCanvases[i].get(), creationContext, isolate());
         if (!m_transferredOffscreenCanvas.contains(v8OffscreenCanvas))
             m_transferredOffscreenCanvas.set(v8OffscreenCanvas, i);
     }
 }
 
 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();
     StateBase* state = doSerialize(value, nullptr);
     while (state)
         state = state->advance(*this);
 
     switch (m_status) {
     case Status::Success:
         transferData(transferables, exceptionState, serializedValue.get());
         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());
         break;
     default:
         NOTREACHED();
     }
 
-    m_blobDataHandles = nullptr;
-
     return serializedValue.release();
 }
 
 void ScriptValueSerializer::transferData(Transferables* transferables, ExceptionState& exceptionState, SerializedScriptValue* serializedValue)
 {
     serializedValue->setData(m_writer.takeWireString());
     DCHECK(serializedValue->data().impl()->hasOneRef());
     if (!transferables)
         return;
 
@@ skipping 15 matching lines @@
 }
 
 // static
 String ScriptValueSerializer::serializeNullValue()
 {
     SerializedScriptValueWriter valueWriter;
     valueWriter.writeNull();
     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);
 
     if (value.IsEmpty())
         return handleError(Status::InputError, "The empty property cannot be cloned.", next);
 
     uint32_t objectReference;
     if ((value->IsObject() || value->IsDate() || value->IsRegExp())
         && m_objectPool.tryGet(value.As<v8::Object>(), &objectReference)) {
         // Note that IsObject() also detects wrappers (eg, it will catch the things
@@ skipping 20 matching lines @@
     } else if (value->IsNumber()) {
         m_writer.writeNumber(value.As<v8::Number>()->Value());
     } else if (value->IsString()) {
         writeString(value);
     } else {
         return handleError(Status::DataCloneError, "A value could not be cloned.", next);
     }
     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());
 
 
     if (object->IsArrayBufferView()) {
         return writeAndGreyArrayBufferView(object, next);
     }
     if (object->IsArrayBuffer()) {
         return writeAndGreyArrayBuffer(object, next);
     }
@@ skipping 75 matching lines @@
     }
 
     // Since IsNativeError is expensive, this check should always be the last check.
     if (isHostObject(object) || object->IsCallable() || object->IsNativeError()) {
         return handleError(Status::DataCloneError, "An object could not be cloned.", next);
     }
 
     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)
         return false;
     if (!shouldCheckForCycles(m_depth))
         return true;
     v8::Local<v8::Value> composite = top->composite();
     for (StateBase* state = top->nextState(); state; state = state->nextState()) {
         if (state->composite() == composite)
             return false;
@@ skipping 22 matching lines @@
     v8::Local<v8::NumberObject> numberObject = value.As<v8::NumberObject>();
     m_writer.writeNumberObject(numberObject->ValueOf());
 }
 
 void ScriptValueSerializer::writeBooleanObject(v8::Local<v8::Value> value)
 {
     v8::Local<v8::BooleanObject> booleanObject = value.As<v8::BooleanObject>();
     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;
     m_blobDataHandles->set(blob->uuid(), blob->blobDataHandle());
     if (appendBlobInfo(blob->uuid(), blob->type(), blob->size(), &blobIndex))
         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)
         return nullptr;
     if (!compositorProxy->connected())
         return handleError(Status::DataCloneError, "A CompositorProxy object has been disconnected, and could therefore not be cloned.", next);
     m_writer.writeCompositorProxy(*compositorProxy);
     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;
     m_blobDataHandles->set(file->uuid(), file->blobDataHandle());
     if (appendFileInfo(file, &blobIndex)) {
         ASSERT(blobIndex >= 0);
         m_writer.writeFileIndex(blobIndex);
     } 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) {
         int blobIndex = -1;
         const File* file = fileList->item(i);
         if (file->isClosed())
             return handleError(Status::DataCloneError, "A File object has been closed, and could therefore not be cloned.", next);
         m_blobDataHandles->set(file->uuid(), file->blobDataHandle());
         if (appendFileInfo(file, &blobIndex)) {
             ASSERT(!i || blobIndex > 0);
             ASSERT(blobIndex >= 0);
             blobIndices.append(blobIndex);
         }
     }
     if (!blobIndices.isEmpty())
         m_writer.writeFileListIndex(blobIndices);
     else
         m_writer.writeFileList(*fileList);
-    return 0;
+    return nullptr;
 }
 
 void ScriptValueSerializer::writeImageData(v8::Local<v8::Value> value)
 {
     ImageData* imageData = V8ImageData::toImpl(value.As<v8::Object>());
     if (!imageData)
         return;
     DOMUint8ClampedArray* pixelArray = imageData->data();
     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)
         return nullptr;
     if (imageBitmap->isNeutered())
         return handleError(Status::DataCloneError, "An ImageBitmap is detached and could not be cloned.", next);
 
     uint32_t index;
     if (m_transferredImageBitmaps.tryGet(object, &index)) {
         m_writer.writeTransferredImageBitmap(index);
     } else {
         greyObject(object);
         std::unique_ptr<uint8_t[]> pixelData = imageBitmap->copyBitmapData(PremultiplyAlpha);
         m_writer.writeImageBitmap(imageBitmap->width(), imageBitmap->height(), static_cast<uint32_t>(imageBitmap->originClean()), pixelData.get(), imageBitmap->width() * imageBitmap->height() * 4);
     }
     return nullptr;
 }
 
 void ScriptValueSerializer::writeRegExp(v8::Local<v8::Value> value)
 {
     v8::Local<v8::RegExp> regExp = value.As<v8::RegExp>();
     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());
     if (underlyingBuffer.IsEmpty())
         return handleError(Status::DataCloneError, "An ArrayBuffer could not be cloned.", next);
     StateBase* stateOut = doSerializeArrayBuffer(underlyingBuffer, next);
     if (stateOut)
         return stateOut;
     m_writer.writeArrayBufferView(*arrayBufferView);
     // This should be safe: we serialize something that we know to be a wrapper (see
     // the toV8 call above), so the call to doSerializeArrayBuffer should neither
     // cause the system stack to overflow nor should it have potential to reach
     // this ArrayBufferView again.
     //
     // We do need to grey the underlying buffer before we grey its view, however;
     // ArrayBuffers may be shared, so they need to be given reference IDs, and an
     // ArrayBufferView cannot be constructed without a corresponding ArrayBuffer
     // (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)
         return nullptr;
     if (arrayBuffer->isNeutered())
         return handleError(Status::DataCloneError, "An ArrayBuffer is neutered and could not be cloned.", next);
 
     uint32_t index;
     if (m_transferredArrayBuffers.tryGet(object, &index)) {
         m_writer.writeTransferredArrayBuffer(index);
     } else {
         greyObject(object);
         m_writer.writeArrayBuffer(*arrayBuffer);
     }
     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)
         return nullptr;
     if (offscreenCanvas->isNeutered())
         return handleError(Status::DataCloneError, "An OffscreenCanvas is detached and could not be cloned.", next);
     if (offscreenCanvas->renderingContext())
         return handleError(Status::DataCloneError, "An OffscreenCanvas with a context could not be cloned.", next);
     m_writer.writeTransferredOffscreenCanvas(index, offscreenCanvas->width(), offscreenCanvas->height(), offscreenCanvas->getAssociatedCanvasId());
     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)
 {
     // Let K be the cost of serializing all property values that are there
     // Cost of serializing sparsely: 5*propertyCount + K (5 bytes per uint32_t key)
     // Cost of serializing densely: K + 1*(length - propertyCount) (1 byte for all properties that are not there)
     // so densely is better than sparsly whenever 6*propertyCount > length
     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))
         return checkException(next);
     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()));
     }
 
     m_writer.writeGenerateFreshSparseArray(length);
     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
     return push(new ObjectState(object, next));
 }
 
 // Marks object as having been visited by the serializer and assigns it a unique object reference ID.
 // An object may only be greyed once.
 void ScriptValueSerializer::greyObject(const v8::Local<v8::Object>& object)
 {
@@ skipping 1160 matching lines @@
         return false;
     uint32_t objectReference = m_openCompositeReferenceStack[m_openCompositeReferenceStack.size() - 1];
     m_openCompositeReferenceStack.shrink(m_openCompositeReferenceStack.size() - 1);
     if (objectReference >= m_objectPool.size())
         return false;
     *object = m_objectPool[objectReference];
     return true;
 }
 
 } // namespace blink