Title: [170015] trunk/Source/WebCore
Revision
170015
Author
[email protected]
Date
2014-06-16 10:41:40 -0700 (Mon, 16 Jun 2014)

Log Message

"nullable" sequence support is incomplete (i.e. sequence<NativeType>?) https://bugs.webkit.org/show_bug.cgi?id=131240

eeviewed by Darin Adler.
Patch by Antonio Gomes <[email protected]>

Nullable sequences are not fully supported in WebKit's
code generator machinery. Although the generated code
does add "is nullable" check to the evaluation condition
(i.e. if (arg.isNull() || ...)), when the JSValue
that holds a "null" JSObject is actually to be "converted"
to a native Vector<T>, it fails.

The reason for the failure is in JSDOMBindings::toNativeArray.
This method verifies that JSValue does not hold a "non-null"
object, and it bails out.
Analogly, the "ref ptr" variant of this method (toRefPtrNativeArray)
does support nullables.

Patch fixes it be checking for a "null" JSValue check before hand.

Tests: Binding tests updated.

* bindings/js/JSDOMBinding.h:
(WebCore::toNativeArray):
* bindings/scripts/CodeGeneratorJS.pm:
(JSValueToNative):
* Modules/websocket/WebSocket.idl:
Removed one overload ctor now that
we can use nullable sequences.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (170014 => 170015)


--- trunk/Source/WebCore/ChangeLog	2014-06-16 17:23:49 UTC (rev 170014)
+++ trunk/Source/WebCore/ChangeLog	2014-06-16 17:41:40 UTC (rev 170015)
@@ -1,3 +1,35 @@
+2014-04-05  Antonio Gomes  <[email protected]>
+
+        [Bindings] "nullable" sequence support is incomplete (i.e. sequence<NativeType>?)
+        https://bugs.webkit.org/show_bug.cgi?id=131240
+
+        Reviewed by Darin Adler.
+
+        Nullable sequences are not fully supported in WebKit's
+        code generator machinery. Although the generated code
+        does add "is nullable" check to the evaluation condition
+        (i.e. if (arg.isNull() || ...)), when the JSValue
+        that holds a "null" JSObject is actually to be "converted"
+        to a native Vector<T>, it fails.
+
+        The reason for the failure is in JSDOMBindings::toNativeArray.
+        This method verifies that JSValue does not hold a "non-null"
+        object, and it bails out.
+        Analogly, the "ref ptr" variant of this method (toRefPtrNativeArray)
+        does support nullables.
+
+        Patch fixes it be checking for a "null" JSValue check before hand.
+
+        Tests: Binding tests updated.
+
+        * bindings/js/JSDOMBinding.h:
+        (WebCore::toNativeArray):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (JSValueToNative):
+        * Modules/websocket/WebSocket.idl:
+        Removed one overload ctor now that
+        we can use nullable sequences.
+
 2014-06-16  Jer Noble  <[email protected]>
 
         [MSE][Mac] Occasional image corruption after seeking

Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.idl (170014 => 170015)


--- trunk/Source/WebCore/Modules/websockets/WebSocket.idl	2014-06-16 17:23:49 UTC (rev 170014)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.idl	2014-06-16 17:41:40 UTC (rev 170015)
@@ -34,8 +34,7 @@
     EnabledAtRuntime,
     Conditional=WEB_SOCKETS,
     ActiveDOMObject,
-    Constructor(DOMString url),
-    Constructor(DOMString url, sequence<DOMString> protocols),
+    Constructor(DOMString url, [Default=Undefined] optional sequence<DOMString>? protocols),
     Constructor(DOMString url, DOMString protocol),
     ConstructorRaisesException,
     ConstructorCallWith=ScriptExecutionContext,

Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.h (170014 => 170015)


--- trunk/Source/WebCore/bindings/js/JSDOMBinding.h	2014-06-16 17:23:49 UTC (rev 170014)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.h	2014-06-16 17:41:40 UTC (rev 170015)
@@ -513,6 +513,10 @@
 
 template<typename T> Vector<T> toNativeArray(JSC::ExecState* exec, JSC::JSValue value)
 {
+    JSC::JSObject* object = value.getObject();
+    if (!object)
+        return Vector<T>();
+
     unsigned length = 0;
     if (isJSArray(value)) {
         JSC::JSArray* array = asArray(value);
@@ -520,7 +524,6 @@
     } else
         toJSSequence(exec, value, length);
 
-    JSC::JSObject* object = value.getObject();
     Vector<T> result;
     result.reserveInitialCapacity(length);
     typedef NativeValueTraits<T> TraitsType;

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp (170014 => 170015)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2014-06-16 17:23:49 UTC (rev 170014)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2014-06-16 17:41:40 UTC (rev 170015)
@@ -315,7 +315,7 @@
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
-    { 125, -1 },
+    { 127, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
@@ -333,7 +333,7 @@
     { -1, -1 },
     { -1, -1 },
     { 5, -1 },
-    { 133, -1 },
+    { 135, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
@@ -418,11 +418,11 @@
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
-    { 126, -1 },
+    { 128, -1 },
+    { 121, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
-    { -1, -1 },
     { 71, -1 },
     { -1, -1 },
     { -1, -1 },
@@ -500,10 +500,10 @@
     { -1, -1 },
     { 8, 512 },
     { -1, -1 },
-    { 129, -1 },
+    { 131, -1 },
     { -1, -1 },
     { 58, 528 },
-    { 134, -1 },
+    { 136, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
@@ -512,7 +512,7 @@
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
-    { 130, -1 },
+    { 132, -1 },
     { -1, -1 },
     { 102, -1 },
     { -1, -1 },
@@ -558,8 +558,8 @@
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
-    { 127, -1 },
-    { 122, -1 },
+    { 129, -1 },
+    { 124, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
@@ -586,13 +586,13 @@
     { 43, -1 },
     { -1, -1 },
     { -1, -1 },
+    { 120, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
-    { -1, -1 },
     { 12, -1 },
     { -1, -1 },
     { 59, -1 },
@@ -609,7 +609,7 @@
     { 28, -1 },
     { -1, -1 },
     { -1, -1 },
-    { 123, -1 },
+    { 125, -1 },
     { -1, -1 },
     { 35, -1 },
     { -1, -1 },
@@ -714,7 +714,7 @@
     { 52, -1 },
     { -1, -1 },
     { 73, -1 },
-    { 131, -1 },
+    { 133, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
@@ -753,11 +753,11 @@
     { -1, -1 },
     { 7, 525 },
     { -1, -1 },
-    { 124, -1 },
+    { 126, -1 },
     { 105, -1 },
     { 74, 520 },
     { -1, -1 },
-    { 121, -1 },
+    { 123, -1 },
     { 82, -1 },
     { -1, -1 },
     { -1, -1 },
@@ -780,9 +780,9 @@
     { 103, -1 },
     { 114, -1 },
     { 115, -1 },
-    { 120, -1 },
-    { 128, -1 },
-    { 132, -1 },
+    { 122, -1 },
+    { 130, -1 },
+    { 134, -1 },
 };
 
 
@@ -936,6 +936,8 @@
     { "methodWithUnsignedLongSequence", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithUnsignedLongSequence), (intptr_t) (1) },
     { "stringArrayFunction", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionStringArrayFunction), (intptr_t) (1) },
     { "domStringListFunction", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionDomStringListFunction), (intptr_t) (1) },
+    { "methodWithAndWithoutNullableSequence", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence), (intptr_t) (2) },
+    { "methodWithAndWithoutNullableSequence2", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence2), (intptr_t) (2) },
     { "getSVGDocument", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionGetSVGDocument), (intptr_t) (0) },
     { "convert1", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionConvert1), (intptr_t) (1) },
     { "convert2", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionConvert2), (intptr_t) (1) },
@@ -953,7 +955,7 @@
     { "any", JSC::Function, NoIntrinsic, (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionAny), (intptr_t) (2) },
 };
 
-static const HashTable JSTestObjPrototypeTable = { 135, 511, true, JSTestObjPrototypeTableValues, 0, JSTestObjPrototypeTableIndex };
+static const HashTable JSTestObjPrototypeTable = { 137, 511, true, JSTestObjPrototypeTableValues, 0, JSTestObjPrototypeTableIndex };
 const ClassInfo JSTestObjPrototype::s_info = { "TestObjectPrototype", &Base::s_info, &JSTestObjPrototypeTable, 0, CREATE_METHOD_TABLE(JSTestObjPrototype) };
 
 JSObject* JSTestObjPrototype::self(VM& vm, JSGlobalObject* globalObject)
@@ -4363,6 +4365,46 @@
     return JSValue::encode(result);
 }
 
+EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence(ExecState* exec)
+{
+    JSValue thisValue = exec->thisValue();
+    JSTestObj* castedThis = jsDynamicCast<JSTestObj*>(thisValue);
+    if (UNLIKELY(!castedThis))
+        return throwThisTypeError(*exec, "TestObj", "methodWithAndWithoutNullableSequence");
+    ASSERT_GC_OBJECT_INHERITS(castedThis, JSTestObj::info());
+    TestObj& impl = castedThis->impl();
+    if (exec->argumentCount() < 2)
+        return throwVMError(exec, createNotEnoughArgumentsError(exec));
+    Vector<unsigned> arrayArg(toNativeArray<unsigned>(exec, exec->argument(0)));
+    if (UNLIKELY(exec->hadException()))
+        return JSValue::encode(jsUndefined());
+    Vector<unsigned> nullableArrayArg(toNativeArray<unsigned>(exec, exec->argument(1)));
+    if (UNLIKELY(exec->hadException()))
+        return JSValue::encode(jsUndefined());
+    impl.methodWithAndWithoutNullableSequence(arrayArg, nullableArrayArg);
+    return JSValue::encode(jsUndefined());
+}
+
+EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence2(ExecState* exec)
+{
+    JSValue thisValue = exec->thisValue();
+    JSTestObj* castedThis = jsDynamicCast<JSTestObj*>(thisValue);
+    if (UNLIKELY(!castedThis))
+        return throwThisTypeError(*exec, "TestObj", "methodWithAndWithoutNullableSequence2");
+    ASSERT_GC_OBJECT_INHERITS(castedThis, JSTestObj::info());
+    TestObj& impl = castedThis->impl();
+    if (exec->argumentCount() < 2)
+        return throwVMError(exec, createNotEnoughArgumentsError(exec));
+    Vector<unsigned> arrayArg(toNativeArray<unsigned>(exec, exec->argument(0)));
+    if (UNLIKELY(exec->hadException()))
+        return JSValue::encode(jsUndefined());
+    Vector<unsigned> nullableArrayArg(toNativeArray<unsigned>(exec, exec->argument(1)));
+    if (UNLIKELY(exec->hadException()))
+        return JSValue::encode(jsUndefined());
+    impl.methodWithAndWithoutNullableSequence2(arrayArg, nullableArrayArg);
+    return JSValue::encode(jsUndefined());
+}
+
 EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionGetSVGDocument(ExecState* exec)
 {
     JSValue thisValue = exec->thisValue();

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.h (170014 => 170015)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.h	2014-06-16 17:23:49 UTC (rev 170014)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.h	2014-06-16 17:41:40 UTC (rev 170015)
@@ -221,6 +221,8 @@
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionMethodWithUnsignedLongSequence(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionStringArrayFunction(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionDomStringListFunction(JSC::ExecState*);
+JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence(JSC::ExecState*);
+JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionMethodWithAndWithoutNullableSequence2(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionGetSVGDocument(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionConvert1(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionConvert2(JSC::ExecState*);

Modified: trunk/Source/WebCore/bindings/scripts/test/TestObj.idl (170014 => 170015)


--- trunk/Source/WebCore/bindings/scripts/test/TestObj.idl	2014-06-16 17:23:49 UTC (rev 170014)
+++ trunk/Source/WebCore/bindings/scripts/test/TestObj.idl	2014-06-16 17:41:40 UTC (rev 170015)
@@ -207,6 +207,9 @@
     void methodWithUnsignedLongSequence(sequence<unsigned long> unsignedLongSequence);
     [RaisesException] DOMString[] stringArrayFunction(DOMString[] values);
     [RaisesException] DOMStringList domStringListFunction(DOMStringList values);
+
+    void methodWithAndWithoutNullableSequence(sequence<unsigned long> arrayArg, sequence<unsigned long>? nullableArrayArg);
+    void methodWithAndWithoutNullableSequence2(unsigned long[] arrayArg, unsigned long[]? nullableArrayArg);
 #endif
 
     [CheckSecurityForNode] readonly attribute Document contentDocument;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to