Attachment 346652 Details for Bug 187851 – Fixed release builds

[patch] Fixed release builds

bug-187851-20180806134312.patch (text/plain), 38.16 KB, created by Ryosuke Niwa on 2018-08-06 13:43:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-08-06 13:43:13 PDT

Size: 38.16 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 234615)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,57 @@
>+2018-08-04  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Add CEReactions=NotNeeded for reactions only needed for customized builtins
>+        https://bugs.webkit.org/show_bug.cgi?id=187851
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Because WebKit doesn't and will not support customized builtin elements, there are many DOM APIs marked with
>+        [CEReactions] which don't actually need CustomElementReactionStack.
>+
>+        To clarify and document this difference, this patch introduces WebKit extention: [CEReactions=NotNeeded].
>+        When this IDL attribute is specified, we generate CustomElementReactionDisallowedScope in the bindings code
>+        to assert that there are no custom elements reactions being enqueued within the DOM API.
>+
>+        We suppress this assertion in CustomElementReactionStack since a DOM API with [CEReactions=NotNeeded] can
>+        synchronously fire an event and otherwise execute arbirary scripts, which in turn could invoke a DOM API
>+        with [CEReactions].
>+
>+        This patch deployes this change to HTMLIFrameElement since "src" IDL attribute triggers this second scenario.
>+
>+        Test: fast/custom-elements/custom-element-reaction-within-disallowed-scope.html
>+
>+        * bindings/scripts/CodeGeneratorJS.pm:
>+        (GeneratePut):
>+        (GeneratePutByIndex):
>+        (GenerateDefineOwnProperty):
>+        (GenerateDeletePropertyCommon):
>+        (GenerateAttributeSetterBodyDefinition):
>+        (GenerateCustomElementReactionsStackIfNeeed): Added. Generate CustomElementReactionStack for [CEReactions]
>+        and CustomElementReactionDisallowedScope for [CEReactions=NotNeeded].
>+        * bindings/scripts/test/JS/JSTestCEReactions.cpp:
>+        * bindings/scripts/test/TestCEReactions.idl: Added test cases for [CEReactions=NotNeeded].
>+        * bindings/scripts/test/TestCEReactionsStringifier.idl: Ditto.
>+        * dom/CustomElementReactionQueue.cpp:
>+        (WebCore::CustomElementReactionQueue::enqueueElementUpgrade): Added an assertion to catch cases where
>+        a DOM API with [CEReactions=NotNeeded] enqueues a custom element reaction; i.e. cases where [CEReactions]
>+        should have been used.
>+        (WebCore::CustomElementReactionQueue::enqueueElementUpgradeIfDefined): Ditto.
>+        (WebCore::CustomElementReactionQueue::enqueueConnectedCallbackIfNeeded): Ditto.
>+        (WebCore::CustomElementReactionQueue::enqueueDisconnectedCallbackIfNeeded): Ditto.
>+        (WebCore::CustomElementReactionQueue::enqueueAdoptedCallbackIfNeeded): Ditto.
>+        (WebCore::CustomElementReactionQueue::enqueueAttributeChangedCallbackIfNeeded): Ditto.
>+        (WebCore::CustomElementReactionQueue::enqueuePostUpgradeReactions): Ditto.
>+        * dom/CustomElementReactionQueue.h:
>+        (WebCore::CustomElementReactionDisallowedScope): Added. Enables the assertion in enqueue* functions above.
>+        (WebCore::CustomElementReactionDisallowedScope::CustomElementReactionDisallowedScope): Added.
>+        (WebCore::CustomElementReactionDisallowedScope::~CustomElementReactionDisallowedScope): Added.
>+        (WebCore::CustomElementReactionDisallowedScope::isReactionAllowed): Added.
>+        (WebCore::CustomElementReactionDisallowedScope::AllowedScope): Added.
>+        (WebCore::CustomElementReactionDisallowedScope::AllowedScope::AllowedScope): Added.
>+        (WebCore::CustomElementReactionDisallowedScope::AllowedScope::~AllowedScope): Added.
>+        (WebCore::CustomElementReactionStack): Suppress the assertion. See above for why this is needed.
>+        * html/HTMLIFrameElement.idl:
>+
> 2018-08-06  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r234569.
>Index: Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
>===================================================================
>--- Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	(revision 234615)
>+++ Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	(working copy)
>@@ -930,10 +930,14 @@ sub GeneratePut
>     push(@$outputArray, "{\n");
>     push(@$outputArray, "    auto* thisObject = jsCast<${className}*>(cell);\n");
>     push(@$outputArray, "    ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n\n");
>-    
>-    if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
>-        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(*state);\n\n");
>-        AddToImplIncludes("CustomElementReactionQueue.h");
>+
>+    assert("CEReactions is not supported on having both named setters and indexed setters") if $namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}
>+        && $indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions};
>+    if ($namedSetterOperation) {
>+        GenerateCustomElementReactionsStackIfNeeed($outputArray, $namedSetterOperation, "*state");
>+    }
>+    if ($indexedSetterOperation) {
>+        GenerateCustomElementReactionsStackIfNeeed($outputArray, $indexedSetterOperation, "*state");
>     }
>     
>     if ($indexedSetterOperation) {
>@@ -1002,10 +1006,14 @@ sub GeneratePutByIndex
>     push(@$outputArray, "{\n");
>     push(@$outputArray, "    auto* thisObject = jsCast<${className}*>(cell);\n");
>     push(@$outputArray, "    ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n\n");
>-    
>-    if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
>-        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(*state);\n\n");
>-        AddToImplIncludes("CustomElementReactionQueue.h");
>+
>+    assert("CEReactions is not supported on having both named setters and indexed setters") if $namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}
>+        && $indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions};
>+    if ($namedSetterOperation) {
>+        GenerateCustomElementReactionsStackIfNeeed($outputArray, $namedSetterOperation, "*state");
>+    }
>+    if ($indexedSetterOperation) {
>+        GenerateCustomElementReactionsStackIfNeeed($outputArray, $indexedSetterOperation, "*state");
>     }
>     
>     if ($indexedSetterOperation ) {
>@@ -1098,10 +1106,14 @@ sub GenerateDefineOwnProperty
>     push(@$outputArray, "{\n");
>     push(@$outputArray, "    auto* thisObject = jsCast<${className}*>(object);\n");
>     push(@$outputArray, "    ASSERT_GC_OBJECT_INHERITS(thisObject, info());\n\n");
>-    
>-    if (($namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}) || ($indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions})) {
>-        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(*state);\n\n");
>-        AddToImplIncludes("CustomElementReactionQueue.h");
>+
>+    assert("CEReactions is not supported on having both named setters and indexed setters") if $namedSetterOperation && $namedSetterOperation->extendedAttributes->{CEReactions}
>+        && $indexedSetterOperation && $indexedSetterOperation->extendedAttributes->{CEReactions};
>+    if ($namedSetterOperation) {
>+        GenerateCustomElementReactionsStackIfNeeed($outputArray, $namedSetterOperation, "*state");
>+    }
>+    if ($indexedSetterOperation) {
>+        GenerateCustomElementReactionsStackIfNeeed($outputArray, $indexedSetterOperation, "*state");
>     }
>     
>     # 1. If O supports indexed properties and P is an array index property name, then:
>@@ -1223,10 +1235,7 @@ sub GenerateDeletePropertyCommon
>     my $overrideBuiltin = $codeGenerator->InheritsExtendedAttribute($interface, "OverrideBuiltins") ? "OverrideBuiltins::Yes" : "OverrideBuiltins::No";
>     push(@$outputArray, "    if (isVisibleNamedProperty<${overrideBuiltin}>(*state, thisObject, propertyName)) {\n");
> 
>-    if ($operation->extendedAttributes->{CEReactions}) {
>-        push(@$outputArray, "        CustomElementReactionStack customElementReactionStack(*state);\n");
>-        AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
>-    }
>+    GenerateCustomElementReactionsStackIfNeeed($outputArray, $operation, "*state");
> 
>     # 2.1. If O does not implement an interface with a named property deleter, then return false.
>     # 2.2. Let operation be the operation used to declare the named property deleter.
>@@ -4842,12 +4851,9 @@ sub GenerateAttributeSetterBodyDefinitio
>     push(@$outputArray, "static inline bool ${attributeSetterBodyName}(" . join(", ", @signatureArguments) . ")\n");
>     push(@$outputArray, "{\n");
>     push(@$outputArray, "    UNUSED_PARAM(throwScope);\n");
>-    
>-    if ($attribute->extendedAttributes->{CEReactions}) {
>-        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(state);\n");
>-        AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
>-    }
>-    
>+
>+    GenerateCustomElementReactionsStackIfNeeed($outputArray, $attribute, "state");
>+
>     if ($interface->extendedAttributes->{CheckSecurity} && !$attribute->extendedAttributes->{DoNotCheckSecurity} && !$attribute->extendedAttributes->{DoNotCheckSecurityOnSetter}) {
>         AddToImplIncludes("JSDOMBindingSecurity.h", $conditional);
>         if ($interface->type->name eq "DOMWindow") {
>@@ -5068,10 +5074,7 @@ sub GenerateOperationBodyDefinition
>     push(@$outputArray, "    UNUSED_PARAM(throwScope);\n");
> 
>     if (!$generatingOverloadDispatcher) {
>-        if ($operation->extendedAttributes->{CEReactions}) {
>-            AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
>-            push(@$outputArray, "    CustomElementReactionStack customElementReactionStack(*state);\n");
>-        }
>+        GenerateCustomElementReactionsStackIfNeeed($outputArray, $operation, "*state");
> 
>         if ($interface->extendedAttributes->{CheckSecurity} and !$operation->extendedAttributes->{DoNotCheckSecurity}) {
>             assert("Security checks are not supported for static operations.") if $operation->isStatic;
>@@ -7410,4 +7413,21 @@ sub GenerateCallTracer()
>     }
> }
> 
>+sub GenerateCustomElementReactionsStackIfNeeed
>+{
>+    my ($outputArray, $context, $stateVariable) = @_;
>+
>+    my $CEReactions = $context->extendedAttributes->{CEReactions};
>+
>+    return if !$CEReactions;
>+
>+    AddToImplIncludes("CustomElementReactionQueue.h");
>+
>+    if ($CEReactions eq "NotNeeded") {
>+        push(@$outputArray, "    CustomElementReactionDisallowedScope customElementReactionDisallowedScope;\n");
>+    } else {
>+        push(@$outputArray, "    CustomElementReactionStack customElementReactionStack($stateVariable);\n");
>+    }
>+}
>+
> 1;
>Index: Source/WebCore/bindings/scripts/test/TestCEReactions.idl
>===================================================================
>--- Source/WebCore/bindings/scripts/test/TestCEReactions.idl	(revision 234615)
>+++ Source/WebCore/bindings/scripts/test/TestCEReactions.idl	(working copy)
>@@ -32,5 +32,11 @@ interface TestCEReactions {
>     [CEReactions, Reflect] attribute DOMString reflectAttributeWithCEReactions;
> 
>     [PutForwards=value] attribute TestCEReactionsStringifier stringifierAttribute;
>+
>+    [CEReactions=NotNeeded] void methodWithCEReactionsNotNeeded();
>+    [CEReactions=NotNeeded] attribute DOMString attributeWithCEReactionsNotNeeded;
>+    [CEReactions=NotNeeded, Reflect] attribute DOMString reflectAttributeWithCEReactionsNotNeeded;
>+
>+    [PutForwards=valueWithoutReactions] attribute TestCEReactionsStringifier stringifierAttributeNotNeeded;
> };
> 
>Index: Source/WebCore/bindings/scripts/test/TestCEReactionsStringifier.idl
>===================================================================
>--- Source/WebCore/bindings/scripts/test/TestCEReactionsStringifier.idl	(revision 234615)
>+++ Source/WebCore/bindings/scripts/test/TestCEReactionsStringifier.idl	(working copy)
>@@ -28,4 +28,5 @@
> 
> interface TestCEReactionsStringifier {
>     [CEReactions] stringifier attribute DOMString value;
>+    [CEReactions=NotNeeded] stringifier attribute DOMString valueWithoutReactions;
> };
>\ No newline at end of file
>Index: Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp
>===================================================================
>--- Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp	(revision 234615)
>+++ Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp	(working copy)
>@@ -45,6 +45,7 @@ using namespace JSC;
> // Functions
> 
> JSC::EncodedJSValue JSC_HOST_CALL jsTestCEReactionsPrototypeFunctionMethodWithCEReactions(JSC::ExecState*);
>+JSC::EncodedJSValue JSC_HOST_CALL jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsNotNeeded(JSC::ExecState*);
> 
> // Attributes
> 
>@@ -56,6 +57,12 @@ JSC::EncodedJSValue jsTestCEReactionsRef
> bool setJSTestCEReactionsReflectAttributeWithCEReactions(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
> JSC::EncodedJSValue jsTestCEReactionsStringifierAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
> bool setJSTestCEReactionsStringifierAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
>+JSC::EncodedJSValue jsTestCEReactionsAttributeWithCEReactionsNotNeeded(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
>+bool setJSTestCEReactionsAttributeWithCEReactionsNotNeeded(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
>+JSC::EncodedJSValue jsTestCEReactionsReflectAttributeWithCEReactionsNotNeeded(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
>+bool setJSTestCEReactionsReflectAttributeWithCEReactionsNotNeeded(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
>+JSC::EncodedJSValue jsTestCEReactionsStringifierAttributeNotNeeded(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
>+bool setJSTestCEReactionsStringifierAttributeNotNeeded(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
> 
> class JSTestCEReactionsPrototype : public JSC::JSNonFinalObject {
> public:
>@@ -107,7 +114,11 @@ static const HashTableValue JSTestCEReac
>     { "attributeWithCEReactions", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsAttributeWithCEReactions), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsAttributeWithCEReactions) } },
>     { "reflectAttributeWithCEReactions", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsReflectAttributeWithCEReactions), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsReflectAttributeWithCEReactions) } },
>     { "stringifierAttribute", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsStringifierAttribute), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsStringifierAttribute) } },
>+    { "attributeWithCEReactionsNotNeeded", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsAttributeWithCEReactionsNotNeeded), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsAttributeWithCEReactionsNotNeeded) } },
>+    { "reflectAttributeWithCEReactionsNotNeeded", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsReflectAttributeWithCEReactionsNotNeeded), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsReflectAttributeWithCEReactionsNotNeeded) } },
>+    { "stringifierAttributeNotNeeded", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsStringifierAttributeNotNeeded), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsStringifierAttributeNotNeeded) } },
>     { "methodWithCEReactions", static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { (intptr_t)static_cast<RawNativeFunction>(jsTestCEReactionsPrototypeFunctionMethodWithCEReactions), (intptr_t) (0) } },
>+    { "methodWithCEReactionsNotNeeded", static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { (intptr_t)static_cast<RawNativeFunction>(jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsNotNeeded), (intptr_t) (0) } },
> };
> 
> const ClassInfo JSTestCEReactionsPrototype::s_info = { "TestCEReactionsPrototype", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSTestCEReactionsPrototype) };
>@@ -286,6 +297,106 @@ bool setJSTestCEReactionsStringifierAttr
>     return IDLAttribute<JSTestCEReactions>::set<setJSTestCEReactionsStringifierAttributeSetter>(*state, thisValue, encodedValue, "stringifierAttribute");
> }
> 
>+static inline JSValue jsTestCEReactionsAttributeWithCEReactionsNotNeededGetter(ExecState& state, JSTestCEReactions& thisObject, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    UNUSED_PARAM(state);
>+    auto& impl = thisObject.wrapped();
>+    JSValue result = toJS<IDLDOMString>(state, throwScope, impl.attributeWithCEReactionsNotNeeded());
>+    return result;
>+}
>+
>+EncodedJSValue jsTestCEReactionsAttributeWithCEReactionsNotNeeded(ExecState* state, EncodedJSValue thisValue, PropertyName)
>+{
>+    return IDLAttribute<JSTestCEReactions>::get<jsTestCEReactionsAttributeWithCEReactionsNotNeededGetter, CastedThisErrorBehavior::Assert>(*state, thisValue, "attributeWithCEReactionsNotNeeded");
>+}
>+
>+static inline bool setJSTestCEReactionsAttributeWithCEReactionsNotNeededSetter(ExecState& state, JSTestCEReactions& thisObject, JSValue value, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    CustomElementReactionDisallowedScope customElementReactionDisallowedScope;
>+    auto& impl = thisObject.wrapped();
>+    auto nativeValue = convert<IDLDOMString>(state, value);
>+    RETURN_IF_EXCEPTION(throwScope, false);
>+    AttributeSetter::call(state, throwScope, [&] {
>+        return impl.setAttributeWithCEReactionsNotNeeded(WTFMove(nativeValue));
>+    });
>+    return true;
>+}
>+
>+bool setJSTestCEReactionsAttributeWithCEReactionsNotNeeded(ExecState* state, EncodedJSValue thisValue, EncodedJSValue encodedValue)
>+{
>+    return IDLAttribute<JSTestCEReactions>::set<setJSTestCEReactionsAttributeWithCEReactionsNotNeededSetter>(*state, thisValue, encodedValue, "attributeWithCEReactionsNotNeeded");
>+}
>+
>+static inline JSValue jsTestCEReactionsReflectAttributeWithCEReactionsNotNeededGetter(ExecState& state, JSTestCEReactions& thisObject, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    UNUSED_PARAM(state);
>+    auto& impl = thisObject.wrapped();
>+    JSValue result = toJS<IDLDOMString>(state, throwScope, impl.attributeWithoutSynchronization(WebCore::HTMLNames::reflectattributewithcereactionsnotneededAttr));
>+    return result;
>+}
>+
>+EncodedJSValue jsTestCEReactionsReflectAttributeWithCEReactionsNotNeeded(ExecState* state, EncodedJSValue thisValue, PropertyName)
>+{
>+    return IDLAttribute<JSTestCEReactions>::get<jsTestCEReactionsReflectAttributeWithCEReactionsNotNeededGetter, CastedThisErrorBehavior::Assert>(*state, thisValue, "reflectAttributeWithCEReactionsNotNeeded");
>+}
>+
>+static inline bool setJSTestCEReactionsReflectAttributeWithCEReactionsNotNeededSetter(ExecState& state, JSTestCEReactions& thisObject, JSValue value, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    CustomElementReactionDisallowedScope customElementReactionDisallowedScope;
>+    auto& impl = thisObject.wrapped();
>+    auto nativeValue = convert<IDLDOMString>(state, value);
>+    RETURN_IF_EXCEPTION(throwScope, false);
>+    AttributeSetter::call(state, throwScope, [&] {
>+        return impl.setAttributeWithoutSynchronization(WebCore::HTMLNames::reflectattributewithcereactionsnotneededAttr, WTFMove(nativeValue));
>+    });
>+    return true;
>+}
>+
>+bool setJSTestCEReactionsReflectAttributeWithCEReactionsNotNeeded(ExecState* state, EncodedJSValue thisValue, EncodedJSValue encodedValue)
>+{
>+    return IDLAttribute<JSTestCEReactions>::set<setJSTestCEReactionsReflectAttributeWithCEReactionsNotNeededSetter>(*state, thisValue, encodedValue, "reflectAttributeWithCEReactionsNotNeeded");
>+}
>+
>+static inline JSValue jsTestCEReactionsStringifierAttributeNotNeededGetter(ExecState& state, JSTestCEReactions& thisObject, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    UNUSED_PARAM(state);
>+    auto& impl = thisObject.wrapped();
>+    JSValue result = toJS<IDLInterface<TestCEReactionsStringifier>>(state, *thisObject.globalObject(), throwScope, impl.stringifierAttributeNotNeeded());
>+    return result;
>+}
>+
>+EncodedJSValue jsTestCEReactionsStringifierAttributeNotNeeded(ExecState* state, EncodedJSValue thisValue, PropertyName)
>+{
>+    return IDLAttribute<JSTestCEReactions>::get<jsTestCEReactionsStringifierAttributeNotNeededGetter, CastedThisErrorBehavior::Assert>(*state, thisValue, "stringifierAttributeNotNeeded");
>+}
>+
>+static inline bool setJSTestCEReactionsStringifierAttributeNotNeededSetter(ExecState& state, JSTestCEReactions& thisObject, JSValue value, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    auto id = Identifier::fromString(&state.vm(), reinterpret_cast<const LChar*>("stringifierAttributeNotNeeded"), strlen("stringifierAttributeNotNeeded"));
>+    auto valueToForwardTo = thisObject.get(&state, id);
>+    RETURN_IF_EXCEPTION(throwScope, false);
>+    if (UNLIKELY(!valueToForwardTo.isObject())) {
>+        throwTypeError(&state, throwScope);
>+        return false;
>+    }
>+    auto forwardId = Identifier::fromString(&state.vm(), reinterpret_cast<const LChar*>("valueWithoutReactions"), strlen("valueWithoutReactions"));
>+    PutPropertySlot slot(valueToForwardTo, false);
>+    asObject(valueToForwardTo)->methodTable(state.vm())->put(asObject(valueToForwardTo), &state, forwardId, value, slot);
>+    RETURN_IF_EXCEPTION(throwScope, false);
>+    return true;
>+}
>+
>+bool setJSTestCEReactionsStringifierAttributeNotNeeded(ExecState* state, EncodedJSValue thisValue, EncodedJSValue encodedValue)
>+{
>+    return IDLAttribute<JSTestCEReactions>::set<setJSTestCEReactionsStringifierAttributeNotNeededSetter>(*state, thisValue, encodedValue, "stringifierAttributeNotNeeded");
>+}
>+
> static inline JSC::EncodedJSValue jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsBody(JSC::ExecState* state, typename IDLOperation<JSTestCEReactions>::ClassParameter castedThis, JSC::ThrowScope& throwScope)
> {
>     UNUSED_PARAM(state);
>@@ -301,6 +412,21 @@ EncodedJSValue JSC_HOST_CALL jsTestCERea
>     return IDLOperation<JSTestCEReactions>::call<jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsBody>(*state, "methodWithCEReactions");
> }
> 
>+static inline JSC::EncodedJSValue jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsNotNeededBody(JSC::ExecState* state, typename IDLOperation<JSTestCEReactions>::ClassParameter castedThis, JSC::ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(state);
>+    UNUSED_PARAM(throwScope);
>+    CustomElementReactionDisallowedScope customElementReactionDisallowedScope;
>+    auto& impl = castedThis->wrapped();
>+    impl.methodWithCEReactionsNotNeeded();
>+    return JSValue::encode(jsUndefined());
>+}
>+
>+EncodedJSValue JSC_HOST_CALL jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsNotNeeded(ExecState* state)
>+{
>+    return IDLOperation<JSTestCEReactions>::call<jsTestCEReactionsPrototypeFunctionMethodWithCEReactionsNotNeededBody>(*state, "methodWithCEReactionsNotNeeded");
>+}
>+
> bool JSTestCEReactionsOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor)
> {
>     UNUSED_PARAM(handle);
>Index: Source/WebCore/bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp
>===================================================================
>--- Source/WebCore/bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp	(revision 234615)
>+++ Source/WebCore/bindings/scripts/test/JS/JSTestCEReactionsStringifier.cpp	(working copy)
>@@ -48,6 +48,8 @@ JSC::EncodedJSValue jsTestCEReactionsStr
> bool setJSTestCEReactionsStringifierConstructor(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
> JSC::EncodedJSValue jsTestCEReactionsStringifierValue(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
> bool setJSTestCEReactionsStringifierValue(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
>+JSC::EncodedJSValue jsTestCEReactionsStringifierValueWithoutReactions(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
>+bool setJSTestCEReactionsStringifierValueWithoutReactions(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
> 
> class JSTestCEReactionsStringifierPrototype : public JSC::JSNonFinalObject {
> public:
>@@ -97,6 +99,7 @@ static const HashTableValue JSTestCEReac
> {
>     { "constructor", static_cast<unsigned>(JSC::PropertyAttribute::DontEnum), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsStringifierConstructor), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsStringifierConstructor) } },
>     { "value", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsStringifierValue), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsStringifierValue) } },
>+    { "valueWithoutReactions", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestCEReactionsStringifierValueWithoutReactions), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestCEReactionsStringifierValueWithoutReactions) } },
>     { "toString", static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { (intptr_t)static_cast<RawNativeFunction>(jsTestCEReactionsStringifierPrototypeFunctionToString), (intptr_t) (0) } },
> };
> 
>@@ -208,6 +211,38 @@ bool setJSTestCEReactionsStringifierValu
>     return IDLAttribute<JSTestCEReactionsStringifier>::set<setJSTestCEReactionsStringifierValueSetter>(*state, thisValue, encodedValue, "value");
> }
> 
>+static inline JSValue jsTestCEReactionsStringifierValueWithoutReactionsGetter(ExecState& state, JSTestCEReactionsStringifier& thisObject, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    UNUSED_PARAM(state);
>+    auto& impl = thisObject.wrapped();
>+    JSValue result = toJS<IDLDOMString>(state, throwScope, impl.valueWithoutReactions());
>+    return result;
>+}
>+
>+EncodedJSValue jsTestCEReactionsStringifierValueWithoutReactions(ExecState* state, EncodedJSValue thisValue, PropertyName)
>+{
>+    return IDLAttribute<JSTestCEReactionsStringifier>::get<jsTestCEReactionsStringifierValueWithoutReactionsGetter, CastedThisErrorBehavior::Assert>(*state, thisValue, "valueWithoutReactions");
>+}
>+
>+static inline bool setJSTestCEReactionsStringifierValueWithoutReactionsSetter(ExecState& state, JSTestCEReactionsStringifier& thisObject, JSValue value, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    CustomElementReactionDisallowedScope customElementReactionDisallowedScope;
>+    auto& impl = thisObject.wrapped();
>+    auto nativeValue = convert<IDLDOMString>(state, value);
>+    RETURN_IF_EXCEPTION(throwScope, false);
>+    AttributeSetter::call(state, throwScope, [&] {
>+        return impl.setValueWithoutReactions(WTFMove(nativeValue));
>+    });
>+    return true;
>+}
>+
>+bool setJSTestCEReactionsStringifierValueWithoutReactions(ExecState* state, EncodedJSValue thisValue, EncodedJSValue encodedValue)
>+{
>+    return IDLAttribute<JSTestCEReactionsStringifier>::set<setJSTestCEReactionsStringifierValueWithoutReactionsSetter>(*state, thisValue, encodedValue, "valueWithoutReactions");
>+}
>+
> static inline JSC::EncodedJSValue jsTestCEReactionsStringifierPrototypeFunctionToStringBody(JSC::ExecState* state, typename IDLOperation<JSTestCEReactionsStringifier>::ClassParameter castedThis, JSC::ThrowScope& throwScope)
> {
>     UNUSED_PARAM(state);
>Index: Source/WebCore/dom/CustomElementReactionQueue.cpp
>===================================================================
>--- Source/WebCore/dom/CustomElementReactionQueue.cpp	(revision 234615)
>+++ Source/WebCore/dom/CustomElementReactionQueue.cpp	(working copy)
>@@ -119,6 +119,7 @@ void CustomElementReactionQueue::clear()
> 
> void CustomElementReactionQueue::enqueueElementUpgrade(Element& element, bool alreadyScheduledToUpgrade)
> {
>+    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
>     auto& queue = ensureCurrentQueue(element);
>     if (alreadyScheduledToUpgrade) {
>         ASSERT(queue.m_items.size() == 1);
>@@ -129,6 +130,7 @@ void CustomElementReactionQueue::enqueue
> 
> void CustomElementReactionQueue::enqueueElementUpgradeIfDefined(Element& element)
> {
>+    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
>     ASSERT(element.isCustomElementUpgradeCandidate());
>     auto* window = element.document().domWindow();
>     if (!window)
>@@ -147,6 +149,7 @@ void CustomElementReactionQueue::enqueue
> 
> void CustomElementReactionQueue::enqueueConnectedCallbackIfNeeded(Element& element)
> {
>+    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
>     ASSERT(element.isDefinedCustomElement());
>     ASSERT(element.document().refCount() > 0);
>     auto& queue = ensureCurrentQueue(element);
>@@ -156,6 +159,7 @@ void CustomElementReactionQueue::enqueue
> 
> void CustomElementReactionQueue::enqueueDisconnectedCallbackIfNeeded(Element& element)
> {
>+    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
>     ASSERT(element.isDefinedCustomElement());
>     if (element.document().refCount() <= 0)
>         return; // Don't enqueue disconnectedCallback if the entire document is getting destructed.
>@@ -166,6 +170,7 @@ void CustomElementReactionQueue::enqueue
> 
> void CustomElementReactionQueue::enqueueAdoptedCallbackIfNeeded(Element& element, Document& oldDocument, Document& newDocument)
> {
>+    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
>     ASSERT(element.isDefinedCustomElement());
>     ASSERT(element.document().refCount() > 0);
>     auto& queue = ensureCurrentQueue(element);
>@@ -175,6 +180,7 @@ void CustomElementReactionQueue::enqueue
> 
> void CustomElementReactionQueue::enqueueAttributeChangedCallbackIfNeeded(Element& element, const QualifiedName& attributeName, const AtomicString& oldValue, const AtomicString& newValue)
> {
>+    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
>     ASSERT(element.isDefinedCustomElement());
>     ASSERT(element.document().refCount() > 0);
>     auto& queue = ensureCurrentQueue(element);
>@@ -184,6 +190,7 @@ void CustomElementReactionQueue::enqueue
> 
> void CustomElementReactionQueue::enqueuePostUpgradeReactions(Element& element)
> {
>+    ASSERT(CustomElementReactionDisallowedScope::isReactionAllowed());
>     ASSERT(element.isCustomElementUpgradeCandidate());
>     if (!element.hasAttributes() && !element.isConnected())
>         return;
>@@ -280,6 +287,10 @@ CustomElementReactionQueue& CustomElemen
>     return *element.reactionQueue();
> }
> 
>+#if !ASSERT_DISABLED
>+unsigned CustomElementReactionDisallowedScope::s_customElementReactionDisallowedCount = 0;
>+#endif
>+
> CustomElementReactionStack* CustomElementReactionStack::s_currentProcessingStack = nullptr;
> 
> void CustomElementReactionStack::processQueue(JSC::ExecState* state)
>Index: Source/WebCore/dom/CustomElementReactionQueue.h
>===================================================================
>--- Source/WebCore/dom/CustomElementReactionQueue.h	(revision 234615)
>+++ Source/WebCore/dom/CustomElementReactionQueue.h	(working copy)
>@@ -84,7 +84,55 @@ private:
>     Vector<CustomElementReactionQueueItem> m_items;
> };
> 
>-class CustomElementReactionStack {
>+class CustomElementReactionDisallowedScope {
>+public:
>+    CustomElementReactionDisallowedScope()
>+    {
>+#if !ASSERT_DISABLED
>+        s_customElementReactionDisallowedCount++;
>+#endif
>+    }
>+
>+    ~CustomElementReactionDisallowedScope()
>+    {
>+#if !ASSERT_DISABLED
>+        ASSERT(s_customElementReactionDisallowedCount);
>+        s_customElementReactionDisallowedCount--;
>+#endif
>+    }
>+
>+#if !ASSERT_DISABLED
>+    static bool isReactionAllowed() { return !s_customElementReactionDisallowedCount; }
>+#endif
>+
>+    class AllowedScope {
>+#if !ASSERT_DISABLED
>+    public:
>+        AllowedScope()
>+            : m_originalCount(s_customElementReactionDisallowedCount)
>+        {
>+            s_customElementReactionDisallowedCount = 0;
>+        }
>+
>+        ~AllowedScope()
>+        {
>+            s_customElementReactionDisallowedCount = m_originalCount;
>+        }
>+
>+    private:
>+        unsigned m_originalCount;
>+#endif
>+    };
>+
>+private:
>+#if !ASSERT_DISABLED
>+    WEBCORE_EXPORT static unsigned s_customElementReactionDisallowedCount;
>+
>+    friend class AllowedScope;
>+#endif
>+};
>+
>+class CustomElementReactionStack : public CustomElementReactionDisallowedScope::AllowedScope {
> public:
>     ALWAYS_INLINE CustomElementReactionStack(JSC::ExecState* state)
>         : m_previousProcessingStack(s_currentProcessingStack)
>Index: Source/WebCore/html/HTMLIFrameElement.idl
>===================================================================
>--- Source/WebCore/html/HTMLIFrameElement.idl	(revision 234615)
>+++ Source/WebCore/html/HTMLIFrameElement.idl	(working copy)
>@@ -19,22 +19,22 @@
>  */
> 
> interface HTMLIFrameElement : HTMLElement {
>-    [Reflect] attribute DOMString align;
>-    [Reflect] attribute DOMString frameBorder;
>-    [Reflect] attribute DOMString height;
>-    [Reflect, URL] attribute USVString longDesc;
>-    [Reflect] attribute [TreatNullAs=EmptyString] DOMString marginHeight;
>-    [Reflect] attribute [TreatNullAs=EmptyString] DOMString marginWidth;
>-    [Reflect] attribute DOMString name;
>+    [Reflect, CEReactions=NotNeeded] attribute DOMString align;
>+    [Reflect, CEReactions=NotNeeded] attribute DOMString frameBorder;
>+    [Reflect, CEReactions=NotNeeded] attribute DOMString height;
>+    [Reflect, URL, CEReactions=NotNeeded] attribute USVString longDesc;
>+    [Reflect, CEReactions=NotNeeded] attribute [TreatNullAs=EmptyString] DOMString marginHeight;
>+    [Reflect, CEReactions=NotNeeded] attribute [TreatNullAs=EmptyString] DOMString marginWidth;
>+    [Reflect, CEReactions=NotNeeded] attribute DOMString name;
> 
>     [PutForwards=value] readonly attribute DOMTokenList sandbox;
>-    [Reflect] attribute boolean allowFullscreen;
>-    [CEReactions, Reflect] attribute DOMString allow;
>+    [Reflect, CEReactions=NotNeeded] attribute boolean allowFullscreen;
>+    [Reflect, CEReactions=NotNeeded] attribute DOMString allow;
> 
>-    [Reflect] attribute DOMString scrolling;
>-    [Reflect, URL] attribute USVString src;
>-    [Reflect] attribute DOMString srcdoc;
>-    [Reflect] attribute DOMString width;
>+    [Reflect, CEReactions=NotNeeded] attribute DOMString scrolling;
>+    [Reflect, URL, CEReactions=NotNeeded] attribute USVString src;
>+    [Reflect, CEReactions=NotNeeded] attribute DOMString srcdoc;
>+    [Reflect, CEReactions=NotNeeded] attribute DOMString width;
> 
>     [CheckSecurityForNode] readonly attribute Document contentDocument;
> 
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 234615)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2018-08-04  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Add CEReactions=NotNeeded for reactions only needed for customized builtins
>+        https://bugs.webkit.org/show_bug.cgi?id=187851
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a regression test for enqueuing a custom element reaction in a DOM API marked as [CEReaction]
>+        inside another DOM API with [CEReaction=NotNeeded]. WebKit should not hit a debug assertion added
>+        by this patch.
>+
>+        * fast/custom-elements/custom-element-reaction-within-disallowed-scope-expected.txt: Added.
>+        * fast/custom-elements/custom-element-reaction-within-disallowed-scope.html: Added.
>+
> 2018-08-03  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed test gardening for mac-wk1.
>Index: LayoutTests/fast/custom-elements/custom-element-reaction-within-disallowed-scope-expected.txt
>===================================================================
>--- LayoutTests/fast/custom-elements/custom-element-reaction-within-disallowed-scope-expected.txt	(nonexistent)
>+++ LayoutTests/fast/custom-elements/custom-element-reaction-within-disallowed-scope-expected.txt	(working copy)
>@@ -0,0 +1,4 @@
>+This tests enqueuing a custom element reaction inside an API with CEReaction=NotNeeded. WebKit should not hit a debug assertion.
>+
>+PASS - WebKit did not hit an assertion
>+
>Index: LayoutTests/fast/custom-elements/custom-element-reaction-within-disallowed-scope.html
>===================================================================
>--- LayoutTests/fast/custom-elements/custom-element-reaction-within-disallowed-scope.html	(nonexistent)
>+++ LayoutTests/fast/custom-elements/custom-element-reaction-within-disallowed-scope.html	(working copy)
>@@ -0,0 +1,45 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script>
>+
>+if (window.testRunner) {
>+    testRunner.waitUntilDone();
>+    testRunner.dumpAsText();
>+}
>+
>+function startTest() {
>+    iframe = document.createElement('iframe');
>+    document.body.appendChild(iframe);
>+    iframe.onload = continueTest;
>+    iframe.src = 'custom-element-reaction-within-disallowed-scope.html';
>+}
>+
>+function continueTest() {
>+    iframe.src = 'custom-element-reaction-within-disallowed-scope.html#target';
>+    document.getElementById('result').textContent = didChange
>+        ? 'PASS - WebKit did not hit an assertion'
>+        : 'FAIL - The code to trigger a custom element reaction never ran';
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+}
>+
>+if (top == self) {
>+    document.write('<p>This tests enqueuing a custom element reaction inside an API with CEReaction=NotNeeded. WebKit should not hit a debug assertion.</p>');
>+    window.onload = startTest;
>+} else {
>+    document.write('<p><a id="target" href="#">child target</a></p>');
>+
>+    class SomeElement extends HTMLElement { };
>+    customElements.define('some-element', SomeElement);
>+
>+    document.getElementById('target').addEventListener('focus', () => {
>+        document.body.appendChild(new SomeElement);
>+        top.didChange = true;
>+    });
>+}
>+
>+</script>
>+<pre id="result"></pre>
>+</body>
>+</html>

Flags:

cdumez: review+
cdumez: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 187851: 345514 | 345531 | 345538 | 345564 | 345656 | 345839 | 346584 | 346585 | 346652