ScriptValueSerializer should throw, not crash, when handling unknown types

JavaScript Symbols are a new (in ES2015) primitive type. This caused
ScriptValueSerializer to go off the rails when it had exhausted its known
list of primitives and then assumed anything else might be an object.

Now the code checks for Object-ness before assuming it. Also refactored
the if/else-if cascade to group all Object handling together, and prune
dead branches.

BUG=522227

Review URL: https://codereview.chromium.org/1297223004

git-svn-id: svn://svn.chromium.org/blink/trunk@200874 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/LayoutTests/fast/js/structured-clone.html

@@ -189,7 +189,7 @@ promise_test(function() {
     var map = new Map([['key', function(){}]]);
     return structuredClone(map).then(function(clone) {
         assert_unreached('Should have thrown an exception');
-    }).catch(function(ex) {
+    }, function(ex) {
         assert_true(ex instanceof DOMException, 'Should throw a DOMException');
         assert_equals(ex.code, DOMException.DATA_CLONE_ERR, 'Should be a DataCloneError');
     });
@@ -199,10 +199,19 @@ promise_test(function() {
     var set = new Set([function(){}]);
     return structuredClone(set).then(function(clone) {
         assert_unreached('Should have thrown an exception');
-    }).catch(function(ex) {
+    }, function(ex) {
         assert_true(ex instanceof DOMException, 'Should throw a DOMException');
         assert_equals(ex.code, DOMException.DATA_CLONE_ERR, 'Should be a DataCloneError');
     });
 }, 'Cloning Sets should fail if they contain non-cloneable things');
 
+promise_test(function() {
+    return structuredClone(Symbol('foo')).then(function(clone) {
+        assert_unreached('Should have thrown an exception');
+    }, function(ex) {
+        assert_true(ex instanceof DOMException, 'Should throw a DOMException');
+        assert_equals(ex.code, DOMException.DATA_CLONE_ERR, 'Should be a DataCloneError');
+    });
+}, 'Cloning Symbols should fail');
+
 </script>

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

@@ -712,7 +712,6 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerialize(v8::Local<v
 
 ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeValue(v8::Local<v8::Value> value, ScriptValueSerializer::StateBase* next)
 {
-    uint32_t arrayBufferIndex;
     if (value.IsEmpty())
         return handleError(InputError, "The empty property name cannot be cloned.", next);
     if (value->IsUndefined()) {
@@ -729,25 +728,26 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeValue(v8::Lo
         m_writer.writeUint32(value.As<v8::Uint32>()->Value());
     } else if (value->IsNumber()) {
         m_writer.writeNumber(value.As<v8::Number>()->Value());
-    } else if (V8ArrayBufferView::hasInstance(value, isolate())) {
-        return writeAndGreyArrayBufferView(value.As<v8::Object>(), next);
     } else if (value->IsString()) {
         writeString(value);
-    } else if (V8MessagePort::hasInstance(value, isolate())) {
-        uint32_t messagePortIndex;
-        if (m_transferredMessagePorts.tryGet(value.As<v8::Object>(), &messagePortIndex)) {
+    } else if (value->IsObject()) {
+        v8::Local<v8::Object> jsObject = value.As<v8::Object>();
+
+        uint32_t arrayBufferIndex;
+        if (V8ArrayBufferView::hasInstance(value, isolate())) {
+            return writeAndGreyArrayBufferView(jsObject, next);
+        } else if (V8MessagePort::hasInstance(value, isolate())) {
+            uint32_t messagePortIndex;
+            if (!m_transferredMessagePorts.tryGet(jsObject, &messagePortIndex))
+                return handleError(DataCloneError, "A MessagePort could not be cloned.", next);
             m_writer.writeTransferredMessagePort(messagePortIndex);
-        } else {
-            return handleError(DataCloneError, "A MessagePort could not be cloned.", next);
+            return nullptr;
+        } else if (V8ArrayBuffer::hasInstance(value, isolate()) && m_transferredArrayBuffers.tryGet(jsObject, &arrayBufferIndex)) {
+            return writeTransferredArrayBuffer(value, arrayBufferIndex, next);
+        } else if (V8SharedArrayBuffer::hasInstance(value, isolate()) && m_transferredArrayBuffers.tryGet(jsObject, &arrayBufferIndex)) {
+            return writeTransferredSharedArrayBuffer(value, arrayBufferIndex, next);
         }
-    } else if (V8ArrayBuffer::hasInstance(value, isolate()) && m_transferredArrayBuffers.tryGet(value.As<v8::Object>(), &arrayBufferIndex)) {
-        return writeTransferredArrayBuffer(value, arrayBufferIndex, next);
-    } else if (V8SharedArrayBuffer::hasInstance(value, isolate()) && m_transferredArrayBuffers.tryGet(value.As<v8::Object>(), &arrayBufferIndex)) {
-        return writeTransferredSharedArrayBuffer(value, arrayBufferIndex, next);
-    } else {
-        v8::Local<v8::Object> jsObject = value.As<v8::Object>();
-        if (jsObject.IsEmpty())
-            return handleError(DataCloneError, "An object could not be cloned.", next);
+
         greyObject(jsObject);
         if (value->IsDate()) {
             m_writer.writeDate(value.As<v8::Date>()->ValueOf());
@@ -777,15 +777,15 @@ ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeValue(v8::Lo
             return writeArrayBuffer(value, next);
         } else if (V8CompositorProxy::hasInstance(value, isolate())) {
             return writeCompositorProxy(value, next);
-        } else if (value->IsObject()) {
-            if (isHostObject(jsObject) || jsObject->IsCallable() || value->IsNativeError())
-                return handleError(DataCloneError, "An object could not be cloned.", next);
-            return startObjectState(jsObject, next);
+        } else if (isHostObject(jsObject) || jsObject->IsCallable() || value->IsNativeError()) {
+            return handleError(DataCloneError, "An object could not be cloned.", next);
         } else {
-            return handleError(DataCloneError, "A value could not be cloned.", next);
+            return startObjectState(jsObject, next);
         }
+    } else {
+        return handleError(DataCloneError, "A value could not be cloned.", next);
     }
-    return 0;
+    return nullptr;
 }
 
 ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerializeArrayBuffer(v8::Local<v8::Value> arrayBuffer, ScriptValueSerializer::StateBase* next)