Index properties on cross origin Window objects should be enumerable
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Nov 2017 00:54:32 +0000 (00:54 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Nov 2017 00:54:32 +0000 (00:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=179289

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Re-sync WPT test after:
- https://github.com/w3c/web-platform-tests/pull/8045

Rebaseline a couple of WPT tests now that more checks are passing.

* web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:
* web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html:
* web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt:

Source/WebCore:

Index properties on cross origin Window objects should be enumerable:
- https://github.com/whatwg/html/pull/3186
- https://github.com/w3c/web-platform-tests/pull/8045

All exposed properties used to be enumerable but we had to revert this in
r224287 because it was not Web-compatible. The HTML specification has now
been updated so that only index properties are enumerable cross origin.

No new tests, rebaselined existing tests.

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
(WebCore::JSDOMWindow::getOwnPropertyNames):

LayoutTests:

Update / rebaseline existing test to match new expected behavior.

* js/dom/getOwnPropertyDescriptor-expected.txt:
* js/resources/getOwnPropertyDescriptor.js:

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

LayoutTests/ChangeLog
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt
LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt
LayoutTests/js/dom/getOwnPropertyDescriptor-expected.txt
LayoutTests/js/resources/getOwnPropertyDescriptor.js
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

index 91ef6b6..1d83034 100644 (file)
@@ -1,3 +1,15 @@
+2017-11-04  Chris Dumez  <cdumez@apple.com>
+
+        Index properties on cross origin Window objects should be enumerable
+        https://bugs.webkit.org/show_bug.cgi?id=179289
+
+        Reviewed by Darin Adler.
+
+        Update / rebaseline existing test to match new expected behavior.
+
+        * js/dom/getOwnPropertyDescriptor-expected.txt:
+        * js/resources/getOwnPropertyDescriptor.js:
+
 2017-11-04  Aishwarya Nirmal  <anirmal@apple.com>
 
         [Touch Bar Web API] Add support for menuitem tag
index d5fea49..ef33db4 100644 (file)
@@ -1,3 +1,19 @@
+2017-11-04  Chris Dumez  <cdumez@apple.com>
+
+        Index properties on cross origin Window objects should be enumerable
+        https://bugs.webkit.org/show_bug.cgi?id=179289
+
+        Reviewed by Darin Adler.
+
+        Re-sync WPT test after:
+        - https://github.com/w3c/web-platform-tests/pull/8045
+
+        Rebaseline a couple of WPT tests now that more checks are passing.
+
+        * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:
+        * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html:
+        * web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt:
+
 2017-11-03  Youenn Fablet  <youenn@apple.com>
 
         Implement ServiceWorkerContainer.getRegistration
index c989445..12b521b 100644 (file)
@@ -6,11 +6,11 @@ PASS [[SetPrototypeOf]] should return false
 PASS [[IsExtensible]] should return true for cross-origin objects 
 PASS [[PreventExtensions]] should throw for cross-origin objects 
 PASS [[GetOwnProperty]] - Properties on cross-origin objects should be reported |own| 
-FAIL [[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly assert_equals: property descriptor for 0 should be enumerable expected true but got false
+PASS [[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly 
 PASS [[Delete]] Should throw on cross-origin objects 
 PASS [[DefineOwnProperty]] Should throw for cross-origin objects 
-FAIL Can only enumerate safelisted properties assert_equals: Enumerate all safelisted cross-origin Window properties expected 15 but got 0
-FAIL [[OwnPropertyKeys]] should return all properties from cross-origin objects assert_array_equals: Object.keys() gives the right answer for cross-origin Window lengths differ, expected 15 got 0
+PASS Can only enumerate safelisted enumerable properties 
+PASS [[OwnPropertyKeys]] should return all properties from cross-origin objects 
 PASS [[OwnPropertyKeys]] should return the right symbol-named properties for cross-origin objects 
 PASS [[OwnPropertyKeys]] should place the symbols after the property names after the subframe indices 
 PASS A and B jointly observe the same identity for cross-origin Window and Location 
index 06d96b1..2820b75 100644 (file)
@@ -184,17 +184,20 @@ addTest(function() {
 }, "[[GetOwnProperty]] - Properties on cross-origin objects should be reported |own|");
 
 function checkPropertyDescriptor(desc, propName, expectWritable) {
-  var isSymbol = (typeof(propName) == "symbol");
+  const isSymbol = typeof(propName) === "symbol";
+  const isArrayIndexPropertyName = !isSymbol && !isNaN(parseInt(propName, 10));
   propName = String(propName);
   assert_true(isObject(desc), "property descriptor for " + propName + " should exist");
   assert_equals(desc.configurable, true, "property descriptor for " + propName + " should be configurable");
-  if (isSymbol) {
-    assert_equals(desc.enumerable, false, "symbol-property descriptor for " + propName + " should not be enumerable");
-    assert_true("value" in desc,
-                "property descriptor for " + propName + " should be a value descriptor");
-    assert_equals(desc.value, undefined,
+  if (!isArrayIndexPropertyName) {
+    assert_equals(desc.enumerable, false, "property descriptor for " + propName + " should not be enumerable");
+    if(isSymbol) {
+      assert_true("value" in desc,
+                  "property descriptor for " + propName + " should be a value descriptor");
+      assert_equals(desc.value, undefined,
                   "symbol-named cross-origin visible prop " + propName +
                   " should come back as undefined");
+    }
   } else {
     assert_equals(desc.enumerable, true, "property descriptor for " + propName + " should be enumerable");
   }
@@ -265,16 +268,15 @@ addTest(function() {
   let i = 0;
   for (var prop in C) {
     i++;
-    assert_true(whitelistedWindowPropNames.includes(prop), prop + " is not safelisted for a cross-origin Window");
+    assert_true(whitelistedWindowIndices.includes(prop), prop + " is not safelisted for a cross-origin Window");
   }
-  assert_equals(i, whitelistedWindowPropNames.length, "Enumerate all safelisted cross-origin Window properties");
+  assert_equals(i, whitelistedWindowIndices.length, "Enumerate all enumerable safelisted cross-origin Window properties");
   i = 0;
   for (var prop in C.location) {
     i++;
-    assert_true(whitelistedLocationPropNames.includes(prop), prop + " is not safelisted for a cross-origin Location");
   }
-  assert_equals(i, whitelistedLocationPropNames.length, "Enumerate all safelisted cross-origin Location properties");
-}, "Can only enumerate safelisted properties");
+  assert_equals(i, 0, "There's nothing to enumerate for cross-origin Location properties");
+}, "Can only enumerate safelisted enumerable properties");
 
 /*
  * [[OwnPropertyKeys]]
@@ -285,13 +287,12 @@ addTest(function() {
                       whitelistedWindowPropNames,
                       "Object.getOwnPropertyNames() gives the right answer for cross-origin Window");
   assert_array_equals(Object.keys(C).sort(),
-                      whitelistedWindowPropNames,
+                      whitelistedWindowIndices,
                       "Object.keys() gives the right answer for cross-origin Window");
   assert_array_equals(Object.getOwnPropertyNames(C.location).sort(),
                       whitelistedLocationPropNames,
                       "Object.getOwnPropertyNames() gives the right answer for cross-origin Location");
-  assert_array_equals(Object.keys(C.location).sort(),
-                      whitelistedLocationPropNames,
+  assert_equals(Object.keys(C.location).length, 0,
                       "Object.keys() gives the right answer for cross-origin Location");
 }, "[[OwnPropertyKeys]] should return all properties from cross-origin objects");
 
index fca7728..8a8c16f 100644 (file)
@@ -1,6 +1,6 @@
 
 PASS Indexed properties of the window object (non-strict mode) 
-FAIL Ensure indexed properties have the correct configuration assert_true: expected true got false
+PASS Ensure indexed properties have the correct configuration 
 FAIL Indexed properties of the window object (non-strict mode) 1 assert_throws: function "() => Object.defineProperty(window, 0, { value: "bar" })" did not throw
 FAIL Indexed properties of the window object (non-strict mode) 2 assert_throws: function "() => Object.defineProperty(window, 1, { value: "bar" })" did not throw
 PASS Indexed properties of the window object (non-strict mode) 3 
index e8ed7d6..8ec0119 100644 (file)
@@ -128,7 +128,7 @@ PASS Object.getOwnPropertyDescriptor(global, 'XMLHttpRequest').configurable is t
 PASS Object.getOwnPropertyDescriptor(global, 0).value is global[0]
 PASS Object.getOwnPropertyDescriptor(global, 0).hasOwnProperty('get') is false
 PASS Object.getOwnPropertyDescriptor(global, 0).hasOwnProperty('set') is false
-PASS Object.getOwnPropertyDescriptor(global, 0).enumerable is false
+PASS Object.getOwnPropertyDescriptor(global, 0).enumerable is true
 PASS Object.getOwnPropertyDescriptor(global, 0).configurable is true
 PASS Object.getOwnPropertyDescriptor(document.getElementsByTagName('div'), 0).value is document.getElementsByTagName('div')[0]
 PASS Object.getOwnPropertyDescriptor(document.getElementsByTagName('div'), 0).hasOwnProperty('get') is false
index 6e35f3b..2ed30ac 100644 (file)
@@ -44,7 +44,7 @@ descriptorShouldBe("global", "'Infinity'", {writable: false, enumerable: false,
 var globalWindowGetter = Object.getOwnPropertyDescriptor(global, 'window').get;
 descriptorShouldBe("global", "'window'", {get: 'globalWindowGetter', set: undefined, enumerable: true, configurable: false});
 descriptorShouldBe("global", "'XMLHttpRequest'", {writable: true, enumerable: false, configurable: true, value:"XMLHttpRequest"});
-descriptorShouldBe("global", "0", {writable: true, enumerable: false, configurable: true, value:"global[0]"});
+descriptorShouldBe("global", "0", {writable: true, enumerable: true, configurable: true, value:"global[0]"});
 descriptorShouldBe("document.getElementsByTagName('div')", "0", {writable: false, enumerable: true, configurable: true, value:"document.getElementsByTagName('div')[0]"});
 descriptorShouldBe("document.getElementsByClassName('pass')", "0", {writable: false, enumerable: true, configurable: true, value:"document.getElementsByClassName('pass')[0]"});
 var canvas = document.createElement("canvas");
index 6e387f4..54e5e48 100644 (file)
@@ -1,3 +1,24 @@
+2017-11-04  Chris Dumez  <cdumez@apple.com>
+
+        Index properties on cross origin Window objects should be enumerable
+        https://bugs.webkit.org/show_bug.cgi?id=179289
+
+        Reviewed by Darin Adler.
+
+        Index properties on cross origin Window objects should be enumerable:
+        - https://github.com/whatwg/html/pull/3186
+        - https://github.com/w3c/web-platform-tests/pull/8045
+
+        All exposed properties used to be enumerable but we had to revert this in
+        r224287 because it was not Web-compatible. The HTML specification has now
+        been updated so that only index properties are enumerable cross origin.
+
+        No new tests, rebaselined existing tests.
+
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
+        (WebCore::JSDOMWindow::getOwnPropertyNames):
+
 2017-11-04  Simon Fraser  <simon.fraser@apple.com>
 
         Add a GraphicsContextImpl and use it for DispayList::Recorder
index c6a670d..63f0839 100644 (file)
@@ -233,7 +233,7 @@ bool JSDOMWindow::getOwnPropertySlotByIndex(JSObject* object, ExecState* state,
     // (1) First, indexed properties.
     // These are also allowed cross-orgin, so come before the access check.
     if (frame && index < frame->tree().scopedChildCount()) {
-        slot.setValue(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), toJS(state, frame->tree().scopedChild(index)->document()->domWindow()));
+        slot.setValue(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), toJS(state, frame->tree().scopedChild(index)->document()->domWindow()));
         return true;
     }
 
@@ -351,8 +351,7 @@ void JSDOMWindow::getOwnPropertyNames(JSObject* object, ExecState* exec, Propert
 {
     JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
 
-    if (mode.includeDontEnumProperties())
-        addScopedChildrenIndexes(*exec, thisObject->wrapped(), propertyNames);
+    addScopedChildrenIndexes(*exec, thisObject->wrapped(), propertyNames);
 
     if (!BindingSecurity::shouldAllowAccessToDOMWindow(exec, thisObject->wrapped(), DoNotReportSecurityError)) {
         if (mode.includeDontEnumProperties())