Changeset 196690 in webkit


Ignore:
Timestamp:
Feb 17, 2016 12:38:27 AM (8 years ago)
Author:
Chris Dumez
Message:

Window should have its 'constructor' property on the prototype
​https://bugs.webkit.org/show_bug.cgi?id=154037
<rdar://problem/24689078>

Reviewed by Gavin Barraclough.

LayoutTests/imported/w3c:

Rebaseline W3C test now that one more check is passing.

  • web-platform-tests/html/dom/interfaces-expected.txt:

Source/WebCore:

Window should have its 'constructor' property on the prototype as per
the Web IDL specification:
​http://heycam.github.io/webidl/#interface-prototype-object

Firefox and Chrome already match the specification.

No new tests, covered by:

  • fast/dom/Window/window-constructor-settable.html
  • fast/dom/Window/window-constructor.html
  • http/tests/security/cross-origin-window-property-access.html
  • imported/w3c/web-platform-tests/html/dom/interfaces.html
  • bindings/scripts/CodeGeneratorJS.pm:

(ConstructorShouldBeOnInstance): Deleted.
Drop this routine as all constructors are now on the prototype.

(InstancePropertyCount):
Do not account for constructor properties as these can only be
on the prototype now.

(PrototypePropertyCount):
Increment the property count by 1 if the interface has a constructor
property (e.g. [NoInterfaceObject] interfaces do not have one).

(GeneratePropertiesHashTable):
Stop calling ConstructorShouldBeOnInstance() as it no longer exists.
Always generated the "constructor" property if:

  1. We are generating the prototype hash table.

and

  1. The interface needs a constructor (i.e. not marked as [NoInterfaceObject]).

(GenerateImplementation):

  • Drop code handling the case where ConstructorShouldBeOnInstance() returns true as constructors are not always on the prototype and the ConstructorShouldBeOnInstance() routine has been dropped.
  • Drop code handling [CustomProxyToJSObject]. Now that the constructor is always on the prototype, we never need to cast thisValue to a JSDOMWindow (by calling toJSDOMWindow). In the Window case, thisValue is now casted to a JSDOMWindowPrototype*, similarly to other interfaces so we don't need a special casting function anymore.
  • Stop generating security checks. This only impacts Window as it is the only interface marked as [CheckSecurity]. The cross-origin checking code as it was would not work when "constructor" is on the prototype because thisValue is a JSDOMWindowPrototype, not a JSDOMWindow and we have no way of getting the wrapped window. Also, the security check is no longer needed because:
    1. Accessing crossOriginWindow.constructor will not work now that constructor is on the prototype because JSDOMWindow::getOwnPropertySlot() already prevents access to the prototype in the cross-origin case.
    2. "constructor" is a value property, not a getter/setter. Therefore, it is no possible to use the getter/setter from a same origin window instance and call it on a cross origin window.

LayoutTests:

  • http/tests/security/cross-origin-window-property-access-expected.txt:
  • http/tests/security/cross-origin-window-property-access.html:

Add checks to make sure it still is not possible to access
window.constructor cross-origin.

  • js/getOwnPropertyDescriptor-window-attributes-expected.txt:
  • js/getOwnPropertyDescriptor-window-attributes.html:

Update test now that window has it's "constructor" attribute
on the prototype.

Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r196688 r196690  
     12016-02-17  Chris Dumez  <cdumez@apple.com>
     2
     3        Window should have its 'constructor' property on the prototype
     4        https://bugs.webkit.org/show_bug.cgi?id=154037
     5        <rdar://problem/24689078>
     6
     7        Reviewed by Gavin Barraclough.
     8
     9        * http/tests/security/cross-origin-window-property-access-expected.txt:
     10        * http/tests/security/cross-origin-window-property-access.html:
     11        Add checks to make sure it still is not possible to access
     12        window.constructor cross-origin.
     13
     14        * js/getOwnPropertyDescriptor-window-attributes-expected.txt:
     15        * js/getOwnPropertyDescriptor-window-attributes.html:
     16        Update test now that window has it's "constructor" attribute
     17        on the prototype.
     18
    1192016-02-16  Carlos Garcia Campos  <cgarcia@igalia.com>
    220

  • trunk/LayoutTests/http/tests/security/cross-origin-window-property-access-expected.txt

    r196220 r196690  
    44CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
    55CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
     6CONSOLE MESSAGE: line 15: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
     7CONSOLE MESSAGE: line 15: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
    68Tests that using another window's property getter does not bypass cross-origin checks.
    79
    … …  
    1517PASS Object.getOwnPropertyDescriptor(window, "navigator").get.call(crossOriginWindow) returned undefined.
    1618PASS Object.getOwnPropertyDescriptor(window, "screenX").get.call(crossOriginWindow) returned undefined.
     19PASS Object.getOwnPropertyDescriptor(window.__proto__, "constructor").get.call(crossOriginWindow) threw exception TypeError: undefined is not an object (evaluating 'Object.getOwnPropertyDescriptor(window.__proto__, "constructor").get.call').
     20PASS Object.getOwnPropertyDescriptor(window.__proto__, "constructor").get.call(crossOriginWindow.__proto__) threw exception TypeError: undefined is not an object (evaluating 'Object.getOwnPropertyDescriptor(window.__proto__, "constructor").get.call').
     21PASS crossOriginWindow.constructor returned undefined.
     22PASS Object.getOwnPropertyDescriptor(crossOriginWindow.__proto__, "constructor").value threw exception TypeError: undefined is not an object (evaluating 'Object.getOwnPropertyDescriptor(crossOriginWindow.__proto__, "constructor")').
    1723PASS Object.getOwnPropertyDescriptor(window, "location").get.call(crossOriginWindow) === crossOriginWindow.location is true
    1824PASS successfullyParsed is true

  • trunk/LayoutTests/http/tests/security/cross-origin-window-property-access.html

    r196220 r196690  
    3333    shouldThrowOrReturnUndefined('Object.getOwnPropertyDescriptor(window, "navigator").get.call(crossOriginWindow)');
    3434    shouldThrowOrReturnUndefined('Object.getOwnPropertyDescriptor(window, "screenX").get.call(crossOriginWindow)');
     35    shouldThrowOrReturnUndefined('Object.getOwnPropertyDescriptor(window.__proto__, "constructor").get.call(crossOriginWindow)');
     36    shouldThrowOrReturnUndefined('Object.getOwnPropertyDescriptor(window.__proto__, "constructor").get.call(crossOriginWindow.__proto__)');
     37    shouldThrowOrReturnUndefined('crossOriginWindow.constructor');
     38    shouldThrowOrReturnUndefined('Object.getOwnPropertyDescriptor(crossOriginWindow.__proto__, "constructor").value');
    3539    shouldBeTrue('Object.getOwnPropertyDescriptor(window, "location").get.call(crossOriginWindow) === crossOriginWindow.location');
    3640    finishJSTest();

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r196673 r196690  
     12016-02-17  Chris Dumez  <cdumez@apple.com>
     2
     3        Window should have its 'constructor' property on the prototype
     4        https://bugs.webkit.org/show_bug.cgi?id=154037
     5        <rdar://problem/24689078>
     6
     7        Reviewed by Gavin Barraclough.
     8
     9        Rebaseline W3C test now that one more check is passing.
     10
     11        * web-platform-tests/html/dom/interfaces-expected.txt:
     12
    1132016-02-16  Chris Dumez  <cdumez@apple.com>
    214

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/dom/interfaces-expected.txt

    r196676 r196690  
    38123812PASS Window interface object name
    38133813FAIL Window interface: existence and properties of interface prototype object assert_equals: Class name for prototype of Window.prototype is not "WindowProperties" expected "[object WindowProperties]" but got "[object EventTargetPrototype]"
    3814 FAIL Window interface: existence and properties of interface prototype object's "constructor" property assert_own_property: Window.prototype does not have own property "constructor" expected property "constructor" missing
     3814PASS Window interface: existence and properties of interface prototype object's "constructor" property
    38153815PASS Window interface: attribute self
    38163816PASS Window interface: attribute name

  • trunk/LayoutTests/js/getOwnPropertyDescriptor-window-attributes-expected.txt

    r196374 r196690  
    5555PASS descriptor.value is window.Node
    5656
    57 * window.constructor
     57* window.__proto__.constructor
    5858PASS descriptor.enumerable is false
    5959PASS descriptor.writable is true

  • trunk/LayoutTests/js/getOwnPropertyDescriptor-window-attributes.html

    r196374 r196690  
    6363
    6464debug("");
    65 // FIXME: 'constructor' should be on the prototype.
    66 debug("* window.constructor");
    67 descriptor = Object.getOwnPropertyDescriptor(window, "constructor");
     65debug("* window.__proto__.constructor");
     66descriptor = Object.getOwnPropertyDescriptor(window.__proto__, "constructor");
    6867shouldBeFalse("descriptor.enumerable");
    6968shouldBeTrue("descriptor.writable");

  • trunk/Source/WebCore/ChangeLog

    r196688 r196690  
     12016-02-17  Chris Dumez  <cdumez@apple.com>
     2
     3        Window should have its 'constructor' property on the prototype
     4        https://bugs.webkit.org/show_bug.cgi?id=154037
     5        <rdar://problem/24689078>
     6
     7        Reviewed by Gavin Barraclough.
     8
     9        Window should have its 'constructor' property on the prototype as per
     10        the Web IDL specification:
     11        http://heycam.github.io/webidl/#interface-prototype-object
     12
     13        Firefox and Chrome already match the specification.
     14
     15        No new tests, covered by:
     16        - fast/dom/Window/window-constructor-settable.html
     17        - fast/dom/Window/window-constructor.html
     18        - http/tests/security/cross-origin-window-property-access.html
     19        - imported/w3c/web-platform-tests/html/dom/interfaces.html
     20
     21        * bindings/scripts/CodeGeneratorJS.pm:
     22        (ConstructorShouldBeOnInstance): Deleted.
     23        Drop this routine as all constructors are now on the prototype.
     24
     25        (InstancePropertyCount):
     26        Do not account for constructor properties as these can only be
     27        on the prototype now.
     28
     29        (PrototypePropertyCount):
     30        Increment the property count by 1 if the interface has a constructor
     31        property (e.g. [NoInterfaceObject] interfaces do not have one).
     32
     33        (GeneratePropertiesHashTable):
     34        Stop calling ConstructorShouldBeOnInstance() as it no longer exists.
     35        Always generated the "constructor" property if:
     36        1. We are generating the prototype hash table.
     37        and
     38        2. The interface needs a constructor (i.e. not marked as
     39           [NoInterfaceObject]).
     40
     41        (GenerateImplementation):
     42        - Drop code handling the case where ConstructorShouldBeOnInstance()
     43          returns true as constructors are not always on the prototype and
     44          the ConstructorShouldBeOnInstance() routine has been dropped.
     45        - Drop code handling [CustomProxyToJSObject]. Now that the constructor
     46          is always on the prototype, we never need to cast thisValue to a
     47          JSDOMWindow (by calling toJSDOMWindow). In the Window case, thisValue
     48          is now casted to a JSDOMWindowPrototype*, similarly to other interfaces
     49          so we don't need a special casting function anymore.
     50        - Stop generating security checks. This only impacts Window as it is the
     51          only interface marked as [CheckSecurity]. The cross-origin checking code
     52          as it was would not work when "constructor" is on the prototype because
     53          thisValue is a JSDOMWindowPrototype, not a JSDOMWindow and we have no
     54          way of getting the wrapped window. Also, the security check is no longer
     55          needed because:
     56          1. Accessing crossOriginWindow.constructor will not work now that
     57             constructor is on the prototype because
     58             JSDOMWindow::getOwnPropertySlot() already prevents access to the
     59             prototype in the cross-origin case.
     60          2. "constructor" is a value property, not a getter/setter. Therefore,
     61             it is no possible to use the getter/setter from a same origin window
     62             instance and call it on a cross origin window.
     63
    1642016-02-16  Carlos Garcia Campos  <cgarcia@igalia.com>
    265

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r196673 r196690  
    682682}
    683683
    684 sub ConstructorShouldBeOnInstance
    685 {
    686     my $interface = shift;
    687 
    688     # FIXME: constructor should always be on the prototype:
    689     # http://www.w3.org/TR/WebIDL/#interface-prototype-object
    690     return 1 if $interface->extendedAttributes->{"CheckSecurity"};
    691     return 0;
    692 }
    693 
    694684sub AttributeShouldBeOnInstanceForCompatibility
    695685{
    … …  
    791781    }
    792782    $count += InstanceFunctionCount($interface);
    793     $count++ if ConstructorShouldBeOnInstance($interface);
    794783    return $count;
    795784}
    … …  
    803792    }
    804793    $count += PrototypeFunctionCount($interface);
    805     $count++ if !ConstructorShouldBeOnInstance($interface);
     794    $count++ if NeedsConstructorProperty($interface);
    806795    return $count;
    807796}
    … …  
    13891378    my $propertyCount = $isInstance ? InstancePropertyCount($interface) : PrototypePropertyCount($interface);
    13901379
    1391     if (ConstructorShouldBeOnInstance($interface) == $isInstance) {
    1392 
    1393         if (NeedsConstructorProperty($interface)) {
    1394             die if !$propertyCount;
    1395             push(@$hashKeys, "constructor");
    1396             my $getter = "js" . $interfaceName . "Constructor";
    1397             push(@$hashValue1, $getter);
    1398 
    1399             my $setter = "setJS" . $interfaceName . "Constructor";
    1400             push(@$hashValue2, $setter);
    1401             push(@$hashSpecials, "DontEnum");
    1402         }
     1380    if (!$isInstance && NeedsConstructorProperty($interface)) {
     1381        die if !$propertyCount;
     1382        push(@$hashKeys, "constructor");
     1383        my $getter = "js" . $interfaceName . "Constructor";
     1384        push(@$hashValue1, $getter);
     1385
     1386        my $setter = "setJS" . $interfaceName . "Constructor";
     1387        push(@$hashValue2, $setter);
     1388        push(@$hashSpecials, "DontEnum");
    14031389    }
    14041390
    … …  
    25242510            my $constructorFunctionName = "js" . $interfaceName . "Constructor";
    25252511
    2526             if ($interface->extendedAttributes->{"CustomProxyToJSObject"}) {
    2527                 push(@implContent, "EncodedJSValue ${constructorFunctionName}(ExecState* state, EncodedJSValue thisValue, PropertyName)\n");
    2528                 push(@implContent, "{\n");
    2529                 push(@implContent, "    ${className}* domObject = to${className}(JSValue::decode(thisValue));\n");
    2530             } elsif (ConstructorShouldBeOnInstance($interface)) {
    2531                 push(@implContent, "EncodedJSValue ${constructorFunctionName}(ExecState* state, EncodedJSValue thisValue, PropertyName)\n");
    2532                 push(@implContent, "{\n");
    2533                 push(@implContent, "    ${className}* domObject = " . GetCastingHelperForThisObject($interface) . "(JSValue::decode(thisValue));\n");
    2534             } else {
    2535                 push(@implContent, "EncodedJSValue ${constructorFunctionName}(ExecState* state, EncodedJSValue thisValue, PropertyName)\n");
    2536                 push(@implContent, "{\n");
    2537                 push(@implContent, "    ${className}Prototype* domObject = jsDynamicCast<${className}Prototype*>(JSValue::decode(thisValue));\n");
    2538             }
     2512            push(@implContent, "EncodedJSValue ${constructorFunctionName}(ExecState* state, EncodedJSValue thisValue, PropertyName)\n");
     2513            push(@implContent, "{\n");
     2514            push(@implContent, "    ${className}Prototype* domObject = jsDynamicCast<${className}Prototype*>(JSValue::decode(thisValue));\n");
    25392515            push(@implContent, "    if (!domObject)\n");
    25402516            push(@implContent, "        return throwVMTypeError(state);\n");
    2541 
    2542             if ($interface->extendedAttributes->{"CheckSecurity"}) {
    2543                 die if !ConstructorShouldBeOnInstance($interface);
    2544                 push(@implContent, "    if (!BindingSecurity::shouldAllowAccessToDOMWindow(state, domObject->wrapped()))\n");
    2545                 push(@implContent, "        return JSValue::encode(jsUndefined());\n");
    2546             }
    25472517
    25482518            if (!$interface->extendedAttributes->{"NoInterfaceObject"}) {
    … …  
    25622532        push(@implContent, "{\n");
    25632533        push(@implContent, "    JSValue value = JSValue::decode(encodedValue);\n");
    2564         if ($interface->extendedAttributes->{"CustomProxyToJSObject"}) {
    2565             push(@implContent, "    ${className}* domObject = to${className}(JSValue::decode(thisValue));\n");
    2566         } elsif (ConstructorShouldBeOnInstance($interface)) {
    2567             push(@implContent, "    ${className}* domObject = " . GetCastingHelperForThisObject($interface) . "(JSValue::decode(thisValue));\n");
    2568         } else {
    2569             push(@implContent, "    ${className}Prototype* domObject = jsDynamicCast<${className}Prototype*>(JSValue::decode(thisValue));\n");
    2570         }
     2534        push(@implContent, "    ${className}Prototype* domObject = jsDynamicCast<${className}Prototype*>(JSValue::decode(thisValue));\n");
    25712535        push(@implContent, "    if (UNLIKELY(!domObject)) {\n");
    25722536        push(@implContent, "        throwVMTypeError(state);\n");
    25732537        push(@implContent, "        return;\n");
    25742538        push(@implContent, "    }\n");
    2575         if ($interface->extendedAttributes->{"CheckSecurity"}) {
    2576             if ($interfaceName eq "DOMWindow") {
    2577                 push(@implContent, "    if (!BindingSecurity::shouldAllowAccessToDOMWindow(state, domObject->wrapped()))\n");
    2578             } else {
    2579                 push(@implContent, "    if (!shouldAllowAccessToFrame(state, domObject->wrapped().frame()))\n");
    2580             }
    2581             push(@implContent, "        return;\n");
    2582         }
    25832539
    25842540        push(@implContent, "    // Shadowing a built-in constructor\n");

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.cpp

    r196563 r196690  
    7474/* Hash table */
    7575
    76 static const struct CompactHashIndex JSTestActiveDOMObjectTableIndex[4] = {
    77     { 1, -1 },
     76static const struct CompactHashIndex JSTestActiveDOMObjectTableIndex[2] = {
    7877    { 0, -1 },
    7978    { -1, -1 },
    80     { -1, -1 },
    8179};
    8280
    … …  
    8482static const HashTableValue JSTestActiveDOMObjectTableValues[] =
    8583{
    86     { "constructor", DontEnum, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestActiveDOMObjectConstructor), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestActiveDOMObjectConstructor) } },
    8784    { "excitingAttr", ReadOnly | CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestActiveDOMObjectExcitingAttr), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(0) } },
    8885};
    8986
    90 static const HashTable JSTestActiveDOMObjectTable = { 2, 3, true, JSTestActiveDOMObjectTableValues, JSTestActiveDOMObjectTableIndex };
     87static const HashTable JSTestActiveDOMObjectTable = { 1, 1, true, JSTestActiveDOMObjectTableValues, JSTestActiveDOMObjectTableIndex };
    9188template<> JSValue JSTestActiveDOMObjectConstructor::prototypeForStructure(JSC::VM& vm, const JSDOMGlobalObject& globalObject)
    9289{
    … …  
    108105static const HashTableValue JSTestActiveDOMObjectPrototypeTableValues[] =
    109106{
     107    { "constructor", DontEnum, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestActiveDOMObjectConstructor), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestActiveDOMObjectConstructor) } },
    110108    { "excitingFunction", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestActiveDOMObjectPrototypeFunctionExcitingFunction), (intptr_t) (1) } },
    111109    { "postMessage", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestActiveDOMObjectPrototypeFunctionPostMessage), (intptr_t) (1) } },
    … …  
    171169EncodedJSValue jsTestActiveDOMObjectConstructor(ExecState* state, EncodedJSValue thisValue, PropertyName)
    172170{
    173     JSTestActiveDOMObject* domObject = jsDynamicCast<JSTestActiveDOMObject*>(JSValue::decode(thisValue));
     171    JSTestActiveDOMObjectPrototype* domObject = jsDynamicCast<JSTestActiveDOMObjectPrototype*>(JSValue::decode(thisValue));
    174172    if (!domObject)
    175173        return throwVMTypeError(state);
    176     if (!BindingSecurity::shouldAllowAccessToDOMWindow(state, domObject->wrapped()))
    177         return JSValue::encode(jsUndefined());
    178174    return JSValue::encode(JSTestActiveDOMObject::getConstructor(state->vm(), domObject->globalObject()));
    179175}
    … …  
    182178{
    183179    JSValue value = JSValue::decode(encodedValue);
    184     JSTestActiveDOMObject* domObject = jsDynamicCast<JSTestActiveDOMObject*>(JSValue::decode(thisValue));
     180    JSTestActiveDOMObjectPrototype* domObject = jsDynamicCast<JSTestActiveDOMObjectPrototype*>(JSValue::decode(thisValue));
    185181    if (UNLIKELY(!domObject)) {
    186182        throwVMTypeError(state);
    187183        return;
    188184    }
    189     if (!shouldAllowAccessToFrame(state, domObject->wrapped().frame()))
    190         return;
    191185    // Shadowing a built-in constructor
    192186    domObject->putDirect(state->vm(), state->propertyNames().constructor, value);
Note: See TracChangeset for help on using the changeset viewer.