[WebIDL] PutForwards is not implemented to spec as illustrated by the WPT WebIDL...
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Jun 2017 18:57:26 +0000 (18:57 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Jun 2017 18:57:26 +0000 (18:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172956

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

Update results to show we now implement [PutForwards] correctly.

* web-platform-tests/WebIDL/ecmascript-binding/put-forwards-expected.txt:

Source/WebCore:

Implements step 3.5.9 of https://heycam.github.io/webidl/#dfn-attribute-setter.

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateAttributeSetterDefinition):
Implement [PutForwards] to spec, which involves using JSC get/set rather than calling
directly into the implementation.

* bindings/scripts/test/JS/JSTestCEReactions.cpp:
* bindings/scripts/test/JS/JSTestObj.cpp:
Update test results.

LayoutTests:

* http/tests/security/xss-DENIED-contentWindow-eval-expected.txt:
Update results to show that we now throw a type error, because the action now requires
an explicit get of the location object, which does not work in the context.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/security/xss-DENIED-contentWindow-eval-expected.txt
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/put-forwards-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp

index 791b5f9..de132de 100644 (file)
@@ -1,3 +1,14 @@
+2017-06-07  Sam Weinig  <sam@webkit.org>
+
+        [WebIDL] PutForwards is not implemented to spec as illustrated by the WPT WebIDL/ecmascript-binding/put-forwards.html
+        https://bugs.webkit.org/show_bug.cgi?id=172956
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/security/xss-DENIED-contentWindow-eval-expected.txt:
+        Update results to show that we now throw a type error, because the action now requires
+        an explicit get of the location object, which does not work in the context.
+
 2017-06-07  Antoine Quint  <graouts@apple.com>
 
         Rebaseline and enable media/modern-media-controls/controls-visibility-support
index 09e8b9f..fd73c3a 100644 (file)
@@ -1,3 +1,14 @@
+2017-06-05  Sam Weinig  <sam@webkit.org>
+
+        [WebIDL] PutForwards is not implemented to spec as illustrated by the WPT WebIDL/ecmascript-binding/put-forwards.html
+        https://bugs.webkit.org/show_bug.cgi?id=172956
+
+        Reviewed by Chris Dumez.
+
+        Update results to show we now implement [PutForwards] correctly.
+
+        * web-platform-tests/WebIDL/ecmascript-binding/put-forwards-expected.txt:
+
 2017-06-06  Darin Adler  <darin@apple.com>
 
         Update to slightly stricter rules for custom element names from more recent standard draft
index d56ad23..58639f4 100644 (file)
@@ -1,14 +1,8 @@
 
-FAIL Overriding getter of [PutForwards] attribute assert_true: Overridden getter should be called expected true got false
-FAIL Overriding setter of [PutForwards] target attribute assert_true: Overridden setter should be called expected true got false
-FAIL Overriding target of [PutForwards] attribute assert_equals: Original value intact expected "" but got "color: green;"
-FAIL Exception propagation from getter of [PutForwards] attribute assert_throws: function "() => {
-    element.style = "color: green";
-  }" did not throw
-FAIL Exception propagation from setter of [PutForwards] target attribute assert_throws: function "() => {
-    element.style = "color: green";
-  }" did not throw
-FAIL TypeError when getter of [PutForwards] attribute returns non-object assert_throws: function "() => {
-    element.style = "color: green";
-  }" did not throw
+PASS Overriding getter of [PutForwards] attribute 
+PASS Overriding setter of [PutForwards] target attribute 
+PASS Overriding target of [PutForwards] attribute 
+PASS Exception propagation from getter of [PutForwards] attribute 
+PASS Exception propagation from setter of [PutForwards] target attribute 
+PASS TypeError when getter of [PutForwards] attribute returns non-object 
 
index 9baa65f..aa80249 100644 (file)
@@ -1,3 +1,21 @@
+2017-06-07  Sam Weinig  <sam@webkit.org>
+
+        [WebIDL] PutForwards is not implemented to spec as illustrated by the WPT WebIDL/ecmascript-binding/put-forwards.html
+        https://bugs.webkit.org/show_bug.cgi?id=172956
+
+        Reviewed by Chris Dumez.
+
+        Implements step 3.5.9 of https://heycam.github.io/webidl/#dfn-attribute-setter.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateAttributeSetterDefinition):
+        Implement [PutForwards] to spec, which involves using JSC get/set rather than calling
+        directly into the implementation.
+
+        * bindings/scripts/test/JS/JSTestCEReactions.cpp:
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        Update test results.
+
 2017-06-07  Jer Noble  <jer.noble@apple.com>
 
         Clean-up: RenderElement.h includes headers it doesn't use
index ef09056..68f6256 100644 (file)
@@ -4573,35 +4573,36 @@ sub GenerateAttributeSetterDefinition
         } else {
             push(@implContent, "    return thisObject.putDirect(state.vm(), Identifier::fromString(&state, \"$name\"), value);\n");
         }
+    } elsif ($attribute->extendedAttributes->{PutForwards}) {
+        assert("[PutForwards] is not compatible with static attributes") if $attribute->isStatic;
+        
+        # 3.5.9.1. Let Q be ? Get(O, id).
+        my $id = $attribute->name;
+        push(@implContent, "    auto id = Identifier::fromString(&state.vm(), reinterpret_cast<const LChar*>(\"${id}\"), strlen(\"${id}\"));\n");
+        push(@implContent, "    auto valueToForwardTo = thisObject.get(&state, id);\n");
+        push(@implContent, "    RETURN_IF_EXCEPTION(throwScope, false);\n");
+        
+        # 3.5.9.2. If Type(Q) is not Object, then throw a TypeError.
+        push(@implContent, "    if (UNLIKELY(!valueToForwardTo.isObject())) {\n");
+        push(@implContent, "        throwTypeError(&state, throwScope);\n");
+        push(@implContent, "        return false;\n");
+        push(@implContent, "    }\n");
+        
+        # 3.5.9.3. Let forwardId be the identifier argument of the [PutForwards] extended attribute.
+        my $forwardId = $attribute->extendedAttributes->{PutForwards};
+        push(@implContent, "    auto forwardId = Identifier::fromString(&state.vm(), reinterpret_cast<const LChar*>(\"${forwardId}\"), strlen(\"${forwardId}\"));\n");
+        
+        # 3.5.9.4. Perform ? Set(Q, forwardId, V).
+        # FIXME: What should the second value to the PutPropertySlot be?
+        # (https://github.com/heycam/webidl/issues/368)
+        push(@implContent, "    PutPropertySlot slot(valueToForwardTo, false);\n");
+        push(@implContent, "    asObject(valueToForwardTo)->methodTable(state.vm())->put(asObject(valueToForwardTo), &state, forwardId, value, slot);\n");
+        push(@implContent, "    RETURN_IF_EXCEPTION(throwScope, false);\n");
+        
+        push(@implContent, "    return true;\n");
     } else {
-        if (!$attribute->isStatic) {
-            my $putForwards = $attribute->extendedAttributes->{PutForwards};
-            if ($putForwards) {
-                my $implGetterFunctionName = $codeGenerator->WK_lcfirst($attribute->extendedAttributes->{ImplementedAs} || $name);
-                my $forwardedAttribute = $codeGenerator->GetAttributeFromInterface($interface, $type->name, $putForwards);
-
-                if ($forwardedAttribute->extendedAttributes->{CEReactions}) {
-                    push(@implContent, "    CustomElementReactionStack customElementReactionStack;\n");
-                    AddToImplIncludes("CustomElementReactionQueue.h", $conditional);
-                }
-
-                if ($type->isNullable) {
-                    push(@implContent, "    RefPtr<" . $type->name . "> forwardedImpl = thisObject.wrapped().${implGetterFunctionName}();\n");
-                    push(@implContent, "    if (!forwardedImpl)\n");
-                    push(@implContent, "        return false;\n");
-                    push(@implContent, "    auto& impl = *forwardedImpl;\n");
-                } else {
-                    # Attribute is not nullable, the implementation is expected to return a reference.
-                    push(@implContent, "    Ref<" . $type->name . "> forwardedImpl = thisObject.wrapped().${implGetterFunctionName}();\n");
-                    push(@implContent, "    auto& impl = forwardedImpl.get();\n");
-                }
-                $attribute = $forwardedAttribute;
-                $type = $attribute->type;
-            } else {
-                push(@implContent, "    auto& impl = thisObject.wrapped();\n");
-            }
-        }
-
+        push(@implContent, "    auto& impl = thisObject.wrapped();\n") if !$attribute->isStatic;
+       
         if ($codeGenerator->IsEnumType($type)) {
             # As per section 3.5.6 of https://heycam.github.io/webidl/#dfn-attribute-setter, enumerations do not use
             # the standard conversion, but rather silently fail on invalid enumeration values.
index fb767d1..f3027d2 100644 (file)
@@ -262,12 +262,17 @@ static inline bool setJSTestCEReactionsStringifierAttributeSetter(ExecState& sta
 {
     UNUSED_PARAM(state);
     UNUSED_PARAM(throwScope);
-    CustomElementReactionStack customElementReactionStack;
-    Ref<TestCEReactionsStringifier> forwardedImpl = thisObject.wrapped().stringifierAttribute();
-    auto& impl = forwardedImpl.get();
-    auto nativeValue = convert<IDLDOMString>(state, value);
+    auto id = Identifier::fromString(&state.vm(), reinterpret_cast<const LChar*>("stringifierAttribute"), strlen("stringifierAttribute"));
+    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*>("value"), strlen("value"));
+    PutPropertySlot slot(valueToForwardTo, false);
+    asObject(valueToForwardTo)->methodTable(state.vm())->put(asObject(valueToForwardTo), &state, forwardId, value, slot);
     RETURN_IF_EXCEPTION(throwScope, false);
-    impl.setValue(WTFMove(nativeValue));
     return true;
 }
 
index ea0e5e1..c6b3fdc 100644 (file)
@@ -4760,11 +4760,17 @@ static inline bool setJSTestObjPutForwardsAttributeSetter(ExecState& state, JSTe
 {
     UNUSED_PARAM(state);
     UNUSED_PARAM(throwScope);
-    Ref<TestNode> forwardedImpl = thisObject.wrapped().putForwardsAttribute();
-    auto& impl = forwardedImpl.get();
-    auto nativeValue = convert<IDLDOMString>(state, value);
+    auto id = Identifier::fromString(&state.vm(), reinterpret_cast<const LChar*>("putForwardsAttribute"), strlen("putForwardsAttribute"));
+    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*>("name"), strlen("name"));
+    PutPropertySlot slot(valueToForwardTo, false);
+    asObject(valueToForwardTo)->methodTable(state.vm())->put(asObject(valueToForwardTo), &state, forwardId, value, slot);
     RETURN_IF_EXCEPTION(throwScope, false);
-    impl.setName(WTFMove(nativeValue));
     return true;
 }
 
@@ -4791,13 +4797,17 @@ static inline bool setJSTestObjPutForwardsNullableAttributeSetter(ExecState& sta
 {
     UNUSED_PARAM(state);
     UNUSED_PARAM(throwScope);
-    RefPtr<TestNode> forwardedImpl = thisObject.wrapped().putForwardsNullableAttribute();
-    if (!forwardedImpl)
+    auto id = Identifier::fromString(&state.vm(), reinterpret_cast<const LChar*>("putForwardsNullableAttribute"), strlen("putForwardsNullableAttribute"));
+    auto valueToForwardTo = thisObject.get(&state, id);
+    RETURN_IF_EXCEPTION(throwScope, false);
+    if (UNLIKELY(!valueToForwardTo.isObject())) {
+        throwTypeError(&state, throwScope);
         return false;
-    auto& impl = *forwardedImpl;
-    auto nativeValue = convert<IDLDOMString>(state, value);
+    }
+    auto forwardId = Identifier::fromString(&state.vm(), reinterpret_cast<const LChar*>("name"), strlen("name"));
+    PutPropertySlot slot(valueToForwardTo, false);
+    asObject(valueToForwardTo)->methodTable(state.vm())->put(asObject(valueToForwardTo), &state, forwardId, value, slot);
     RETURN_IF_EXCEPTION(throwScope, false);
-    impl.setName(WTFMove(nativeValue));
     return true;
 }