Attributes on the Window instance should be configurable unless [Unforgeable]
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Feb 2016 19:47:10 +0000 (19:47 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Feb 2016 19:47:10 +0000 (19:47 +0000)
commitc72daf480c3269cf5de9aa6822ca8d605339a218
treedc5581a84e257032350dd0de27ec0cb2dfccffa6
parente172aae207ddb4edf956d46c9712696f18f64601
Attributes on the Window instance should be configurable unless [Unforgeable]
https://bugs.webkit.org/show_bug.cgi?id=153920
<rdar://problem/24563211>

Reviewed by Darin Adler.

Source/JavaScriptCore:

Marking the Window instance attributes as configurable but cause
getOwnPropertyDescriptor() to report them as configurable, as
expected. However, trying to delete them would actually lead to
unexpected behavior because:
- We did not reify custom accessor properties (most of the Window
  properties are custom accessors) upon deletion.
- For non-reified static properties marked as configurable,
  JSObject::deleteProperty() would attempt to call the property
  setter with undefined. As a result, calling delete window.name
  would cause window.name to become the string "undefined" instead
  of the undefined value.

* runtime/JSObject.cpp:
(JSC::getClassPropertyNames):
Now that we reify ALL properties, we only need to check the property table
if we have not reified. As a result, I dropped the 'didReify' parameter for
this function and instead only call this function if we have not yet reified.

(JSC::JSObject::putInlineSlow):
Only call putEntry() if we have not reified: Drop the
'|| !(entry->attributes() & BuiltinOrFunctionOrAccessor)'
check as such properties now get reified as well.

(JSC::JSObject::deleteProperty):
- Call reifyAllStaticProperties() instead of reifyStaticFunctionsForDelete()
  so that we now reify all properties upon deletion, including the custom
  accessors. reifyStaticFunctionsForDelete() is now removed and the same
  reification function is now used by: deletion, getOwnPropertyDescriptor()
  and eager reification of the prototype objects in the bindings.
- Drop code that falls back to calling the static property setter with
  undefined if we cannot find the property in the property storage. As
  we now reify ALL properties, the code removing the property from the
  property storage should succeed, provided that the property actually
  exists.

(JSC::JSObject::getOwnNonIndexPropertyNames):
Only call getClassPropertyNames() if we have not reified. We should no longer
check the static property table after reifying now that we reify all
properties.

(JSC::JSObject::reifyAllStaticProperties):
Merge with reifyStaticFunctionsForDelete(). The only behavior change is the
flattening to an uncacheable dictionary, like reifyStaticFunctionsForDelete()
used to do.

* runtime/JSObject.h:

Source/WebCore:

Attributes on the Window instance should be configurable unless [Unforgeable]:
1. 'constructor' property:
   - http://www.w3.org/TR/WebIDL/#interface-prototype-object
2. Constructor properties (e.g. window.Node):
   - http://www.w3.org/TR/WebIDL/#es-interfaces
3. IDL attributes:
   - http://heycam.github.io/webidl/#es-attributes (configurable unless
     [Unforgeable], e.g. window.location)

Firefox complies with the WebIDL specification but WebKit does not for 1. and 3.

Test: fast/dom/Window/window-properties-configurable.html

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::getOwnPropertySlot):
For known Window properties (i.e. properties in the static property table),
if we have reified and this is same-origin access, then call
Base::getOwnPropertySlot() to get the property from the local property
storage. If we have not reified yet, or this is cross-origin access, query
the static property table. This is to match the behavior of Firefox and
Chrome which seem to keep returning the original properties upon cross
origin access, even if those were deleted or redefined.

(WebCore::JSDOMWindow::put):
The previous code used to call the static property setter for properties in
the static table. However, this does not do the right thing if properties
were reified. For example, deleting window.name and then trying to set it
again would not work. Therefore, update this code to only do this if the
properties have not been reified, similarly to what is done in
JSObject::putInlineSlow().

* bindings/scripts/CodeGeneratorJS.pm:
(ConstructorShouldBeOnInstance):
Add a FIXME comment indicating that window.constructor should be on
the prototype as per the Web IDL specification.

(GenerateAttributesHashTable):
- Mark 'constructor' property as configurable for Window, as per the
  specification and consistently with other 'constructor' properties:
  http://www.w3.org/TR/WebIDL/#interface-prototype-object
- Mark properties as configurable even though they are on the instance.
  Window has its properties on the instance as per the specification:
  1. http://heycam.github.io/webidl/#es-attributes
  2. http://heycam.github.io/webidl/#PrimaryGlobal (window is [PrimaryGlobal]
  However, these properties should be configurable as long as they are
  not marked as [Unforgeable], as per 1.

* bindings/scripts/test/JS/JSTestActiveDOMObject.cpp:
* bindings/scripts/test/JS/JSTestException.cpp:
* bindings/scripts/test/JS/JSTestObj.cpp:
Rebaseline bindings tests.

LayoutTests:

* fast/dom/Window/window-properties-configurable-expected.txt: Added.
* fast/dom/Window/window-properties-configurable.html: Added.
Add a test to check that Window properties are reported as configurable
unless the [Unforgeable] ones and that deleting them actually works.

* fast/dom/global-constructors.html:
Update test so it no longer expects window.Node to be shadowable. As per
the specification, the "Node" property is on the window instance, not its
prototype. Therefore, it should cannot be shadowed and setting it to
something actually overwites the previous value, given that the property
is writable as per:
- http://heycam.github.io/webidl/#es-interfaces
I have verified that the new behavior is consistent with Firefox.

* http/tests/security/cross-origin-reified-window-property-access-expected.txt: Added.
* http/tests/security/cross-origin-reified-window-property-access.html: Added.
* http/tests/security/resources/reify-window.html: Added.
Add a test case to cover cross-origin access of Window properties after
reification.

* js/getOwnPropertyDescriptor-unforgeable-attributes-expected.txt:
* js/getOwnPropertyDescriptor-unforgeable-attributes.html:
Drop window.self from the list of unforgeable attributes. This attribute
is not unforgeable in our implementation or in the specification:
- https://html.spec.whatwg.org/multipage/browsers.html#the-window-object

* js/getOwnPropertyDescriptor-window-attributes-expected.txt:
* js/getOwnPropertyDescriptor-window-attributes.html:
- Add coverage for window.self which is a regular Window property.
- Add coverage for window.Node which is a constructor property
- Add coverage for window.constructor. It should really be on the prototype
  as per the specification but this at least checks that the property is
  configurable, as per the specification.
- Rebaseline the test as more checks are passing now that Window properties
  are marked as configurable.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196374 268f45cc-cd09-0410-ab3c-d52691b4dbfc
22 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/Window/window-properties-configurable-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Window/window-properties-configurable.html [new file with mode: 0644]
LayoutTests/fast/dom/global-constructors.html
LayoutTests/http/tests/security/cross-origin-reified-window-property-access-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/cross-origin-reified-window-property-access.html [new file with mode: 0644]
LayoutTests/http/tests/security/resources/reify-window.html [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/html/dom/interfaces-expected.txt
LayoutTests/js/getOwnPropertyDescriptor-unforgeable-attributes-expected.txt
LayoutTests/js/getOwnPropertyDescriptor-unforgeable-attributes.html
LayoutTests/js/getOwnPropertyDescriptor-window-attributes-expected.txt
LayoutTests/js/getOwnPropertyDescriptor-window-attributes.html
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/Lookup.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestException.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp