Make binding DOM constructor use toJSNewlyCreated instead of toJS
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 May 2016 07:14:45 +0000 (07:14 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 May 2016 07:14:45 +0000 (07:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157832

Reviewed by Chris Dumez.

Using toJSNewlyCreated in generated constructors instead of toJS.
Enabling generation of toJS and toJSNewlyCreated for constructable DOM objects.
This ensures that toJSNewlyCreated call in constructor will compile properly.

Updating some custom code to implement toJSNewlyCreated.

Covered by existing tests.

* bindings/js/JSImageDataCustom.cpp:
(WebCore::toJSNewlyCreated):
(WebCore::toJS):
* bindings/js/JSTextTrackCueCustom.cpp:
(WebCore::toJSNewlyCreated):
(WebCore::toJS):
* bindings/scripts/CodeGeneratorJS.pm:
(ShouldGenerateToJSDeclaration):
(GenerateConstructorDefinition):
* bindings/scripts/test/JS/JSTestInterface.cpp:
(WebCore::JSTestInterfaceConstructor::construct):
* bindings/scripts/test/JS/JSTestNamedConstructor.cpp:
(WebCore::JSTestNamedConstructorNamedConstructor::construct):
* bindings/scripts/test/JS/JSTestNode.cpp:
(WebCore::JSTestNodeConstructor::construct):
(WebCore::toJSNewlyCreated):
(WebCore::toJS):
* bindings/scripts/test/JS/JSTestNode.h:
(WebCore::toJS):
(WebCore::toJSNewlyCreated):
* bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::JSTestObjConstructor::construct):
* bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp:
(WebCore::constructJSTestOverloadedConstructors1):
(WebCore::constructJSTestOverloadedConstructors2):
(WebCore::constructJSTestOverloadedConstructors3):
(WebCore::constructJSTestOverloadedConstructors4):
(WebCore::constructJSTestOverloadedConstructors5):
* bindings/scripts/test/JS/JSTestTypedefs.cpp:
(WebCore::JSTestTypedefsConstructor::construct):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201128 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSImageDataCustom.cpp
Source/WebCore/bindings/js/JSTextTrackCueCustom.cpp
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestInterface.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestNamedConstructor.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestNode.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestNode.h
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp

index 44c9aad..9d896d5 100644 (file)
@@ -1,3 +1,49 @@
+2016-05-19  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        Make binding DOM constructor use toJSNewlyCreated instead of toJS
+        https://bugs.webkit.org/show_bug.cgi?id=157832
+
+        Reviewed by Chris Dumez.
+
+        Using toJSNewlyCreated in generated constructors instead of toJS.
+        Enabling generation of toJS and toJSNewlyCreated for constructable DOM objects.
+        This ensures that toJSNewlyCreated call in constructor will compile properly.
+
+        Updating some custom code to implement toJSNewlyCreated.
+
+        Covered by existing tests.
+
+        * bindings/js/JSImageDataCustom.cpp:
+        (WebCore::toJSNewlyCreated):
+        (WebCore::toJS):
+        * bindings/js/JSTextTrackCueCustom.cpp:
+        (WebCore::toJSNewlyCreated):
+        (WebCore::toJS):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (ShouldGenerateToJSDeclaration):
+        (GenerateConstructorDefinition):
+        * bindings/scripts/test/JS/JSTestInterface.cpp:
+        (WebCore::JSTestInterfaceConstructor::construct):
+        * bindings/scripts/test/JS/JSTestNamedConstructor.cpp:
+        (WebCore::JSTestNamedConstructorNamedConstructor::construct):
+        * bindings/scripts/test/JS/JSTestNode.cpp:
+        (WebCore::JSTestNodeConstructor::construct):
+        (WebCore::toJSNewlyCreated):
+        (WebCore::toJS):
+        * bindings/scripts/test/JS/JSTestNode.h:
+        (WebCore::toJS):
+        (WebCore::toJSNewlyCreated):
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        (WebCore::JSTestObjConstructor::construct):
+        * bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp:
+        (WebCore::constructJSTestOverloadedConstructors1):
+        (WebCore::constructJSTestOverloadedConstructors2):
+        (WebCore::constructJSTestOverloadedConstructors3):
+        (WebCore::constructJSTestOverloadedConstructors4):
+        (WebCore::constructJSTestOverloadedConstructors5):
+        * bindings/scripts/test/JS/JSTestTypedefs.cpp:
+        (WebCore::JSTestTypedefsConstructor::construct):
+
 2016-05-18  Zalan Bujtas  <zalan@apple.com>
 
         Make LayoutUnit::operator bool() explicit.
index 7d5725d..a9e73b7 100644 (file)
@@ -35,19 +35,24 @@ using namespace JSC;
 
 namespace WebCore {
 
-JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, ImageData& imageData)
+JSValue toJSNewlyCreated(ExecState* state, JSDOMGlobalObject* globalObject, Ref<ImageData>&& imageData)
 {
-    if (auto* wrapper = getCachedWrapper(globalObject->world(), imageData))
-        return wrapper;
-    
-    auto* wrapper = CREATE_DOM_WRAPPER(globalObject, ImageData, imageData);
-    Identifier dataName = Identifier::fromString(exec, "data");
-    wrapper->putDirect(exec->vm(), dataName, toJS(exec, globalObject, imageData.data()), DontDelete | ReadOnly);
+    auto* data = imageData->data();
+    auto* wrapper = CREATE_DOM_WRAPPER(globalObject, ImageData, WTFMove(imageData));
+    Identifier dataName = Identifier::fromString(state, "data");
+    wrapper->putDirect(state->vm(), dataName, toJS(state, globalObject, data), DontDelete | ReadOnly);
     // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
     // https://bugs.webkit.org/show_bug.cgi?id=142595
-    exec->heap()->deprecatedReportExtraMemory(imageData.data()->length());
+    state->heap()->deprecatedReportExtraMemory(data->length());
     
     return wrapper;
 }
 
+JSValue toJS(ExecState* state, JSDOMGlobalObject* globalObject, ImageData& imageData)
+{
+    if (auto* wrapper = getCachedWrapper(globalObject->world(), imageData))
+        return wrapper;
+    return toJSNewlyCreated(state, globalObject, Ref<ImageData>(imageData));
+}
+
 }
index e6143aa..f138da1 100644 (file)
@@ -59,24 +59,29 @@ bool JSTextTrackCueOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> h
     return visitor.containsOpaqueRoot(root(textTrackCue.track()));
 }
 
-JSValue toJS(ExecState*, JSDOMGlobalObject* globalObject, TextTrackCue& cue)
+JSValue toJSNewlyCreated(ExecState*, JSDOMGlobalObject* globalObject, Ref<TextTrackCue>&& cue)
 {
-    if (auto* wrapper = getCachedWrapper(globalObject->world(), cue))
-        return wrapper;
-
     // This switch will make more sense once we support DataCue
-    switch (cue.cueType()) {
+    switch (cue->cueType()) {
     case TextTrackCue::Data:
-        return CREATE_DOM_WRAPPER(globalObject, DataCue, cue);
+        return CREATE_DOM_WRAPPER(globalObject, DataCue, WTFMove(cue));
     case TextTrackCue::WebVTT:
     case TextTrackCue::Generic:
-        return CREATE_DOM_WRAPPER(globalObject, VTTCue, cue);
+        return CREATE_DOM_WRAPPER(globalObject, VTTCue, WTFMove(cue));
     default:
         ASSERT_NOT_REACHED();
         return jsNull();
     }
 }
 
+JSValue toJS(ExecState* state, JSDOMGlobalObject* globalObject, TextTrackCue& cue)
+{
+    if (auto* wrapper = getCachedWrapper(globalObject->world(), cue))
+        return wrapper;
+
+    return toJSNewlyCreated(state, globalObject, Ref<TextTrackCue>(cue));
+}
+
 void JSTextTrackCue::visitAdditionalChildren(SlotVisitor& visitor)
 {
     if (TextTrack* textTrack = wrapped().track())
index f7178d1..12bfbd3 100644 (file)
@@ -474,6 +474,7 @@ sub ShouldGenerateToJSDeclaration
     return 0 if $interface->extendedAttributes->{"CustomProxyToJSObject"};
     return 1 if (!$hasParent or $interface->extendedAttributes->{"JSGenerateToJSObject"} or $interface->extendedAttributes->{"CustomToJSObject"});
     return 1 if $interface->parent && $interface->parent eq "EventTarget";
+    return 1 if $interface->extendedAttributes->{"Constructor"} or $interface->extendedAttributes->{"NamedConstructor"};
     return 0;
 }
 
@@ -5154,7 +5155,7 @@ END
                  push(@$outputArray, "        return JSValue::encode(jsUndefined());\n");
             }
 
-            push(@$outputArray, "    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));\n");
+            push(@$outputArray, "    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));\n");
             push(@$outputArray, "}\n\n");
         }
     }
index 03f9f39..33a76ce 100644 (file)
@@ -240,7 +240,7 @@ template<> EncodedJSValue JSC_HOST_CALL JSTestInterfaceConstructor::construct(Ex
         setDOMException(state, ec);
         return JSValue::encode(JSValue());
     }
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 template<> JSValue JSTestInterfaceConstructor::prototypeForStructure(JSC::VM& vm, const JSDOMGlobalObject& globalObject)
index 898ef29..2300236 100644 (file)
@@ -100,7 +100,7 @@ template<> EncodedJSValue JSC_HOST_CALL JSTestNamedConstructorNamedConstructor::
         setDOMException(state, ec);
         return JSValue::encode(JSValue());
     }
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 template<> JSValue JSTestNamedConstructorNamedConstructor::prototypeForStructure(JSC::VM& vm, const JSDOMGlobalObject& globalObject)
index 3d0268e..cc8ebe5 100644 (file)
@@ -80,7 +80,7 @@ template<> EncodedJSValue JSC_HOST_CALL JSTestNodeConstructor::construct(ExecSta
 {
     auto* castedThis = jsCast<JSTestNodeConstructor*>(state->callee());
     auto object = TestNode::create();
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 template<> JSValue JSTestNodeConstructor::prototypeForStructure(JSC::VM& vm, const JSDOMGlobalObject& globalObject)
@@ -234,5 +234,45 @@ void JSTestNode::visitChildren(JSCell* cell, SlotVisitor& visitor)
     thisObject->wrapped().visitJSEventListeners(visitor);
 }
 
+#if ENABLE(BINDING_INTEGRITY)
+#if PLATFORM(WIN)
+#pragma warning(disable: 4483)
+extern "C" { extern void (*const __identifier("??_7TestNode@WebCore@@6B@")[])(); }
+#else
+extern "C" { extern void* _ZTVN7WebCore8TestNodeE[]; }
+#endif
+#endif
+
+JSC::JSValue toJSNewlyCreated(JSC::ExecState*, JSDOMGlobalObject* globalObject, Ref<TestNode>&& impl)
+{
+    return createNewWrapper<JSTestNode>(globalObject, WTFMove(impl));
+}
+
+JSC::JSValue toJS(JSC::ExecState*, JSDOMGlobalObject* globalObject, TestNode& impl)
+{
+    if (JSValue result = getExistingWrapper<JSTestNode>(globalObject, impl))
+        return result;
+
+#if ENABLE(BINDING_INTEGRITY)
+    void* actualVTablePointer = *(reinterpret_cast<void**>(&impl));
+#if PLATFORM(WIN)
+    void* expectedVTablePointer = reinterpret_cast<void*>(__identifier("??_7TestNode@WebCore@@6B@"));
+#else
+    void* expectedVTablePointer = &_ZTVN7WebCore8TestNodeE[2];
+#if COMPILER(CLANG)
+    // If this fails TestNode does not have a vtable, so you need to add the
+    // ImplementationLacksVTable attribute to the interface definition
+    static_assert(__is_polymorphic(TestNode), "TestNode is not polymorphic");
+#endif
+#endif
+    // If you hit this assertion you either have a use after free bug, or
+    // TestNode has subclasses. If TestNode has subclasses that get passed
+    // to toJS() we currently require TestNode you to opt out of binding hardening
+    // by adding the SkipVTableValidation attribute to the interface IDL definition
+    RELEASE_ASSERT(actualVTablePointer == expectedVTablePointer);
+#endif
+    return createNewWrapper<JSTestNode, TestNode>(globalObject, impl);
+}
+
 
 }
index 5e6b107..4268842 100644 (file)
@@ -65,6 +65,10 @@ protected:
 
 };
 
+WEBCORE_TESTSUPPORT_EXPORT JSC::JSValue toJS(JSC::ExecState*, JSDOMGlobalObject*, TestNode&);
+inline JSC::JSValue toJS(JSC::ExecState* state, JSDOMGlobalObject* globalObject, TestNode* impl) { return impl ? toJS(state, globalObject, *impl) : JSC::jsNull(); }
+JSC::JSValue toJSNewlyCreated(JSC::ExecState*, JSDOMGlobalObject*, Ref<TestNode>&&);
+inline JSC::JSValue toJSNewlyCreated(JSC::ExecState* state, JSDOMGlobalObject* globalObject, RefPtr<TestNode>&& impl) { return impl ? toJSNewlyCreated(state, globalObject, impl.releaseNonNull()) : JSC::jsNull(); }
 
 
 } // namespace WebCore
index 261fa70..159a0c1 100644 (file)
@@ -1024,7 +1024,7 @@ template<> EncodedJSValue JSC_HOST_CALL JSTestObjConstructor::construct(ExecStat
         return throwArgumentMustBeFunctionError(*state, 1, "testCallbackFunction", "TestObj", nullptr);
     RefPtr<TestCallbackFunction> testCallbackFunction = JSTestCallbackFunction::create(asObject(state->uncheckedArgument(1)), castedThis->globalObject());
     auto object = TestObj::create(*testCallback, *testCallbackFunction);
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 template<> JSValue JSTestObjConstructor::prototypeForStructure(JSC::VM& vm, const JSDOMGlobalObject& globalObject)
index 483ca12..d43d792 100644 (file)
@@ -76,7 +76,7 @@ static inline EncodedJSValue constructJSTestOverloadedConstructors1(ExecState* s
     if (UNLIKELY(!arrayBuffer))
         return throwArgumentTypeError(*state, 0, "arrayBuffer", "TestOverloadedConstructors", nullptr, "ArrayBuffer");
     auto object = TestOverloadedConstructors::create(*arrayBuffer);
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 static inline EncodedJSValue constructJSTestOverloadedConstructors2(ExecState* state)
@@ -90,7 +90,7 @@ static inline EncodedJSValue constructJSTestOverloadedConstructors2(ExecState* s
     if (UNLIKELY(!arrayBufferView))
         return throwArgumentTypeError(*state, 0, "arrayBufferView", "TestOverloadedConstructors", nullptr, "ArrayBufferView");
     auto object = TestOverloadedConstructors::create(*arrayBufferView);
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 static inline EncodedJSValue constructJSTestOverloadedConstructors3(ExecState* state)
@@ -102,7 +102,7 @@ static inline EncodedJSValue constructJSTestOverloadedConstructors3(ExecState* s
     if (UNLIKELY(!blob))
         return throwArgumentTypeError(*state, 0, "blob", "TestOverloadedConstructors", nullptr, "Blob");
     auto object = TestOverloadedConstructors::create(*blob);
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 static inline EncodedJSValue constructJSTestOverloadedConstructors4(ExecState* state)
@@ -114,7 +114,7 @@ static inline EncodedJSValue constructJSTestOverloadedConstructors4(ExecState* s
     if (UNLIKELY(state->hadException()))
         return JSValue::encode(jsUndefined());
     auto object = TestOverloadedConstructors::create(string);
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 static inline EncodedJSValue constructJSTestOverloadedConstructors5(ExecState* state)
@@ -124,7 +124,7 @@ static inline EncodedJSValue constructJSTestOverloadedConstructors5(ExecState* s
     if (UNLIKELY(state->hadException()))
         return JSValue::encode(jsUndefined());
     auto object = TestOverloadedConstructors::create(longArgs);
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 template<> EncodedJSValue JSC_HOST_CALL JSTestOverloadedConstructorsConstructor::construct(ExecState* state)
index 3bf1341..fc35e51 100644 (file)
@@ -136,7 +136,7 @@ template<> EncodedJSValue JSC_HOST_CALL JSTestTypedefsConstructor::construct(Exe
         return throwArgumentMustBeFunctionError(*state, 1, "testCallback", "TestTypedefs", nullptr);
     RefPtr<TestCallback> testCallback = JSTestCallback::create(asObject(state->uncheckedArgument(1)), castedThis->globalObject());
     auto object = TestTypedefs::create(hello, *testCallback);
-    return JSValue::encode(asObject(toJS(state, castedThis->globalObject(), WTFMove(object))));
+    return JSValue::encode(asObject(toJSNewlyCreated(state, castedThis->globalObject(), WTFMove(object))));
 }
 
 template<> JSValue JSTestTypedefsConstructor::prototypeForStructure(JSC::VM& vm, const JSDOMGlobalObject& globalObject)