Title: [224287] trunk
Revision
224287
Author
cdu...@apple.com
Date
2017-11-01 12:39:12 -0700 (Wed, 01 Nov 2017)

Log Message

Regression(r219659): Can no longer log into ifttt.com using Google account
https://bugs.webkit.org/show_bug.cgi?id=179117

Reviewed by Geoffrey Garen.

LayoutTests/imported/w3c:

Rebaseline WPT tests.

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

Source/WebCore:

After r219659, it is no longer possible to log into ifttt.com using a Google
account:
- Signed into a Google account already
- Visit https://ifttt.com/login
- Click "Continue with Google"
- Select the signed in account

It turns out that this change to the HTML specification was not Web-compatible:
See https://bugzilla.mozilla.org/show_bug.cgi?id=1412741 & https://github.com/whatwg/html/issues/3183

This patch reverts r219659 for now until we agree on what behavior should get
specified.

No new tests, rebaselined existing tests.

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
(WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
(WebCore::JSDOMWindow::getOwnPropertyNames):
* bindings/js/JSLocationCustom.cpp:
(WebCore::getOwnPropertySlotCommon):
(WebCore::JSLocation::getOwnPropertyNames):

LayoutTests:

Update / rebaseline existing test.

* http/tests/security/cross-origin-descriptors-expected.txt:
* http/tests/security/cross-origin-descriptors.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (224286 => 224287)


--- trunk/LayoutTests/ChangeLog	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/LayoutTests/ChangeLog	2017-11-01 19:39:12 UTC (rev 224287)
@@ -1,3 +1,15 @@
+2017-11-01  Chris Dumez  <cdu...@apple.com>
+
+        Regression(r219659): Can no longer log into ifttt.com using Google account
+        https://bugs.webkit.org/show_bug.cgi?id=179117
+
+        Reviewed by Geoffrey Garen.
+
+        Update / rebaseline existing test.
+
+        * http/tests/security/cross-origin-descriptors-expected.txt:
+        * http/tests/security/cross-origin-descriptors.html:
+
 2017-11-01  Frederic Wang  <fw...@igalia.com>
 
         Make iOS Find UI reveal matches in scrollable elements

Modified: trunk/LayoutTests/http/tests/security/cross-origin-descriptors-expected.txt (224286 => 224287)


--- trunk/LayoutTests/http/tests/security/cross-origin-descriptors-expected.txt	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/LayoutTests/http/tests/security/cross-origin-descriptors-expected.txt	2017-11-01 19:39:12 UTC (rev 224287)
@@ -6,78 +6,78 @@
 * Location.href
 PASS descriptor.get is undefined.
 PASS descriptor.set is an instance of Function
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Location.replace
 PASS descriptor.value is an instance of Function
 PASS descriptor.writable is false
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 
 * Window.window
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is undefined.
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.self
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is undefined.
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.location
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is an instance of Function
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.close
 PASS descriptor.value is an instance of Function
 PASS descriptor.writable is false
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.closed
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is undefined.
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.focus
 PASS descriptor.value is an instance of Function
 PASS descriptor.writable is false
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.blur
 PASS descriptor.value is an instance of Function
 PASS descriptor.writable is false
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.frames
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is undefined.
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.length
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is undefined.
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.top
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is undefined.
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.opener
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is undefined.
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.parent
 PASS descriptor.get is an instance of Function
 PASS descriptor.set is undefined.
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 * Window.postMessage
 PASS descriptor.value is an instance of Function
 PASS descriptor.writable is false
-PASS descriptor.enumerable is true
+PASS descriptor.enumerable is false
 PASS descriptor.configurable is true
 
 PASS successfullyParsed is true

Modified: trunk/LayoutTests/http/tests/security/cross-origin-descriptors.html (224286 => 224287)


--- trunk/LayoutTests/http/tests/security/cross-origin-descriptors.html	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/LayoutTests/http/tests/security/cross-origin-descriptors.html	2017-11-01 19:39:12 UTC (rev 224287)
@@ -27,7 +27,7 @@
         shouldBeType("descriptor.value", "Function");
         shouldBeFalse("descriptor.writable");
     }
-    shouldBeTrue("descriptor.enumerable");
+    shouldBeFalse("descriptor.enumerable");
     shouldBeTrue("descriptor.configurable");
 }
 

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (224286 => 224287)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2017-11-01 19:39:12 UTC (rev 224287)
@@ -1,3 +1,15 @@
+2017-11-01  Chris Dumez  <cdu...@apple.com>
+
+        Regression(r219659): Can no longer log into ifttt.com using Google account
+        https://bugs.webkit.org/show_bug.cgi?id=179117
+
+        Reviewed by Geoffrey Garen.
+
+        Rebaseline WPT tests.
+
+        * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:
+        * web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt:
+
 2017-10-31  Dean Jackson  <d...@apple.com>
 
         transferFromImageBitmap should update canvas dimensions

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt (224286 => 224287)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt	2017-11-01 19:39:12 UTC (rev 224287)
@@ -6,11 +6,11 @@
 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| 
-PASS [[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly 
+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 [[Delete]] Should throw on cross-origin objects 
 PASS [[DefineOwnProperty]] Should throw for cross-origin objects 
-PASS Can only enumerate safelisted properties 
-PASS [[OwnPropertyKeys]] should return all properties from 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 [[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 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt (224286 => 224287)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt	2017-11-01 19:39:12 UTC (rev 224287)
@@ -1,6 +1,6 @@
 
 PASS Indexed properties of the window object (non-strict mode) 
-PASS Ensure indexed properties have the correct configuration 
+FAIL Ensure indexed properties have the correct configuration assert_true: expected true got false
 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 

Modified: trunk/LayoutTests/js/dom/getOwnPropertyDescriptor-expected.txt (224286 => 224287)


--- trunk/LayoutTests/js/dom/getOwnPropertyDescriptor-expected.txt	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/LayoutTests/js/dom/getOwnPropertyDescriptor-expected.txt	2017-11-01 19:39:12 UTC (rev 224287)
@@ -128,7 +128,7 @@
 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 true
+PASS Object.getOwnPropertyDescriptor(global, 0).enumerable is false
 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

Modified: trunk/LayoutTests/js/resources/getOwnPropertyDescriptor.js (224286 => 224287)


--- trunk/LayoutTests/js/resources/getOwnPropertyDescriptor.js	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/LayoutTests/js/resources/getOwnPropertyDescriptor.js	2017-11-01 19:39:12 UTC (rev 224287)
@@ -44,7 +44,7 @@
 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: true, configurable: true, value:"global[0]"});
+descriptorShouldBe("global", "0", {writable: true, enumerable: false, 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");

Modified: trunk/Source/WebCore/ChangeLog (224286 => 224287)


--- trunk/Source/WebCore/ChangeLog	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/Source/WebCore/ChangeLog	2017-11-01 19:39:12 UTC (rev 224287)
@@ -1,3 +1,33 @@
+2017-11-01  Chris Dumez  <cdu...@apple.com>
+
+        Regression(r219659): Can no longer log into ifttt.com using Google account
+        https://bugs.webkit.org/show_bug.cgi?id=179117
+
+        Reviewed by Geoffrey Garen.
+
+        After r219659, it is no longer possible to log into ifttt.com using a Google
+        account:
+        - Signed into a Google account already
+        - Visit https://ifttt.com/login
+        - Click "Continue with Google"
+        - Select the signed in account
+
+        It turns out that this change to the HTML specification was not Web-compatible:
+        See https://bugzilla.mozilla.org/show_bug.cgi?id=1412741 & https://github.com/whatwg/html/issues/3183
+
+        This patch reverts r219659 for now until we agree on what behavior should get
+        specified.
+
+        No new tests, rebaselined existing tests.
+
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
+        (WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
+        (WebCore::JSDOMWindow::getOwnPropertyNames):
+        * bindings/js/JSLocationCustom.cpp:
+        (WebCore::getOwnPropertySlotCommon):
+        (WebCore::JSLocation::getOwnPropertyNames):
+
 2017-10-31  Dean Jackson  <d...@apple.com>
 
         transferFromImageBitmap should update canvas dimensions

Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (224286 => 224287)


--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2017-11-01 19:39:12 UTC (rev 224287)
@@ -91,11 +91,11 @@
     // access below; we should try to find a way to merge the two.
     if (!frame) {
         if (propertyName == builtinNames.closedPublicName()) {
-            slot.setCustom(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete, jsDOMWindowClosed);
+            slot.setCustom(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, jsDOMWindowClosed);
             return true;
         }
         if (propertyName == builtinNames.closePublicName()) {
-            slot.setCustom(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
+            slot.setCustom(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
             return true;
         }
 
@@ -114,19 +114,19 @@
     // These are the functions we allow access to cross-origin (DoNotCheckSecurity in IDL).
     // Always provide the original function, on a fresh uncached function object.
     if (propertyName == builtinNames.blurPublicName()) {
-        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionBlur, 0>);
+        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionBlur, 0>);
         return true;
     }
     if (propertyName == builtinNames.closePublicName()) {
-        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
+        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionClose, 0>);
         return true;
     }
     if (propertyName == builtinNames.focusPublicName()) {
-        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionFocus, 0>);
+        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionFocus, 0>);
         return true;
     }
     if (propertyName == builtinNames.postMessagePublicName()) {
-        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionPostMessage, 2>);
+        slot.setCustom(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsDOMWindowInstanceFunctionPostMessage, 2>);
         return true;
     }
 
@@ -146,7 +146,7 @@
             || propertyName == builtinNames.topPublicName()) {
             bool shouldExposeSetter = propertyName == builtinNames.locationPublicName();
             CustomGetterSetter* customGetterSetter = CustomGetterSetter::create(vm, entry->propertyGetter(), shouldExposeSetter ? entry->propertyPutter() : nullptr);
-            slot.setCustomGetterSetter(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), customGetterSetter);
+            slot.setCustomGetterSetter(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DontEnum), customGetterSetter);
             return true;
         }
 
@@ -163,7 +163,7 @@
     // properties that are in Moz but not IE. Since we have some of these, we have to do it
     // the Moz way.
     if (auto* scopedChild = frame->tree().scopedChild(propertyNameToAtomicString(propertyName))) {
-        slot.setValue(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete, toJS(exec, scopedChild->document()->domWindow()));
+        slot.setValue(thisObject, JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum, toJS(exec, scopedChild->document()->domWindow()));
         return true;
     }
 
@@ -233,7 +233,7 @@
     // (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), toJS(state, frame->tree().scopedChild(index)->document()->domWindow()));
+        slot.setValue(thisObject, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::DontEnum), toJS(state, frame->tree().scopedChild(index)->document()->domWindow()));
         return true;
     }
 
@@ -351,10 +351,12 @@
 {
     JSDOMWindow* thisObject = jsCast<JSDOMWindow*>(object);
 
-    addScopedChildrenIndexes(*exec, thisObject->wrapped(), propertyNames);
+    if (mode.includeDontEnumProperties())
+        addScopedChildrenIndexes(*exec, thisObject->wrapped(), propertyNames);
 
     if (!BindingSecurity::shouldAllowAccessToDOMWindow(exec, thisObject->wrapped(), DoNotReportSecurityError)) {
-        addCrossOriginWindowOwnPropertyNames(*exec, propertyNames);
+        if (mode.includeDontEnumProperties())
+            addCrossOriginWindowOwnPropertyNames(*exec, propertyNames);
         return;
     }
     Base::getOwnPropertyNames(thisObject, exec, propertyNames, mode);

Modified: trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp (224286 => 224287)


--- trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp	2017-11-01 19:10:20 UTC (rev 224286)
+++ trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp	2017-11-01 19:39:12 UTC (rev 224287)
@@ -62,7 +62,7 @@
 
     // We only allow access to Location.replace() cross origin.
     if (propertyName == vm.propertyNames->replace) {
-        slot.setCustom(&thisObject, static_cast<unsigned>(PropertyAttribute::ReadOnly), nonCachingStaticFunctionGetter<jsLocationInstanceFunctionReplace, 1>);
+        slot.setCustom(&thisObject, static_cast<unsigned>(PropertyAttribute::ReadOnly | PropertyAttribute::DontEnum), nonCachingStaticFunctionGetter<jsLocationInstanceFunctionReplace, 1>);
         return true;
     }
 
@@ -71,7 +71,7 @@
     if (slot.internalMethodType() == PropertySlot::InternalMethodType::GetOwnProperty && propertyName == vm.propertyNames->href) {
         auto* entry = JSLocation::info()->staticPropHashTable->entry(propertyName);
         CustomGetterSetter* customGetterSetter = CustomGetterSetter::create(vm, nullptr, entry->propertyPutter());
-        slot.setCustomGetterSetter(&thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), customGetterSetter);
+        slot.setCustomGetterSetter(&thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | PropertyAttribute::DontEnum), customGetterSetter);
         return true;
     }
 
@@ -188,7 +188,8 @@
 {
     JSLocation* thisObject = jsCast<JSLocation*>(object);
     if (!BindingSecurity::shouldAllowAccessToFrame(exec, thisObject->wrapped().frame(), DoNotReportSecurityError)) {
-        addCrossOriginLocationOwnPropertyNames(*exec, propertyNames);
+        if (mode.includeDontEnumProperties())
+            addCrossOriginLocationOwnPropertyNames(*exec, propertyNames);
         return;
     }
     Base::getOwnPropertyNames(thisObject, exec, propertyNames, mode);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to