JSDOMWindow should not claim HasImpureGetOwnPropertySlot

https://bugs.webkit.org/show_bug.cgi?id=132918 Reviewed by Geoffrey Garen. Source/JavaScriptCore: * jit/Repatch.cpp: (JSC::tryRepatchIn): We forgot to check for watchpoints when repatching "in". Source/WebCore: Tests: js/cached-window-properties.html js/cached-window-prototype-properties.html We now correctly handle the impurity of JSDOMWindow's custom getOwnPropertySlot without needing the blanket HasImpureGetOwnPropertySlot. We do this through the use of watchpoints and by explicitly forbidding any caching beyond a certain point using PropertySlot::disableCaching. Getting rid of this flag will allow us to cache many properties/methods on both the JSDOMWindow and its prototype, which are very commonly used across the web. * bindings/js/JSDOMWindowCustom.cpp: (WebCore::JSDOMWindow::getOwnPropertySlot): * bindings/scripts/CodeGeneratorJS.pm: (HasComplexGetOwnProperty): (InterfaceRequiresAttributesOnInstance): (InstanceOverridesGetOwnPropertySlot): (GenerateHeader): LayoutTests: We now correctly handle the impurity of JSDOMWindow's custom getOwnPropertySlot without needing the blanket HasImpureGetOwnPropertySlot. We do this through the use of watchpoints and by explicitly forbidding any caching beyond a certain point using PropertySlot::disableCaching. Getting rid of this flag will allow us to cache many properties/methods on both the JSDOMWindow and its prototype, which are very commonly used across the web. These tests trigger inline caching of window and window prototype properties. * js/cached-window-properties-expected.txt: Added. * js/cached-window-properties.html: Added. * js/cached-window-prototype-properties-expected.txt: Added. * js/cached-window-prototype-properties.html: Added. Canonical link: https://commits.webkit.org/151021@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@168914 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 15, 2014 · c1ae3e2ac01f3b42c76c650763019ca824cde863 · c1ae3e2
1 parent e1d0049
commit c1ae3e2ac01f3b42c76c650763019ca824cde863
Showing 10 changed files with 140 additions and 12 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,23 @@
+2014-05-15  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        JSDOMWindow should not claim HasImpureGetOwnPropertySlot
+        https://bugs.webkit.org/show_bug.cgi?id=132918
+
+        Reviewed by Geoffrey Garen.
+
+        We now correctly handle the impurity of JSDOMWindow's custom getOwnPropertySlot without needing the 
+        blanket HasImpureGetOwnPropertySlot. We do this through the use of watchpoints and by explicitly forbidding
+        any caching beyond a certain point using PropertySlot::disableCaching. Getting rid of this flag will allow 
+        us to cache many properties/methods on both the JSDOMWindow and its prototype, which are very commonly used 
+        across the web.
+
+        These tests trigger inline caching of window and window prototype properties.
+
+        * js/cached-window-properties-expected.txt: Added.
+        * js/cached-window-properties.html: Added.
+        * js/cached-window-prototype-properties-expected.txt: Added.
+        * js/cached-window-prototype-properties.html: Added.
+
 2014-05-15  Alexey Proskuryakov  <ap@apple.com>
 
         Automatically zip document bundles used via File API

diff --git a/LayoutTests/js/cached-window-properties-expected.txt b/LayoutTests/js/cached-window-properties-expected.txt
@@ -0,0 +1,4 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/cached-window-properties.html b/LayoutTests/js/cached-window-properties.html
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+var foo = function(o) {
+    return o.screenX;
+};
+
+var x = window.screenX;
+var niters = 100000;
+var sum = 0;
+for (var i = 0; i < niters; ++i) {
+    sum += foo(window);
+}
+if (sum !== x * niters)
+    throw new Error("Incorrect sum");
+
+window.screenX = 42;
+
+sum = 0;
+for (var i = 0; i < niters; ++i) {
+    sum += foo(window);
+}
+if (sum !== 42 * niters)
+    throw new Error("Incorrect sum");
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/cached-window-prototype-properties-expected.txt b/LayoutTests/js/cached-window-prototype-properties-expected.txt
@@ -0,0 +1,4 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/cached-window-prototype-properties.html b/LayoutTests/js/cached-window-prototype-properties.html
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+var foo = function(o) {
+    return o.setTimeout;
+};
+
+var realSetTimeout = window.setTimeout;
+var niters = 100000;
+for (var i = 0; i < niters; ++i) {
+    if (foo(window) !== realSetTimeout)
+        throw new Error("Incorrect setTimeout");
+}
+
+var fakeSetTimeout = function() {
+    return;
+};
+
+window.setTimeout = fakeSetTimeout;
+
+for (var i = 0; i < niters; ++i) {
+    if (foo(window) !== fakeSetTimeout)
+        throw new Error("Incorrect setTimeout");
+}
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,13 @@
+2014-05-15  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        JSDOMWindow should not claim HasImpureGetOwnPropertySlot
+        https://bugs.webkit.org/show_bug.cgi?id=132918
+
+        Reviewed by Geoffrey Garen.
+
+        * jit/Repatch.cpp:
+        (JSC::tryRepatchIn): We forgot to check for watchpoints when repatching "in".
+
 2014-05-15  Alex Christensen  <achristensen@webkit.org>
 
         Add pointer lock to features without enabling it.

diff --git a/Source/JavaScriptCore/jit/Repatch.cpp b/Source/JavaScriptCore/jit/Repatch.cpp
@@ -1458,6 +1458,9 @@ static bool tryRepatchIn(
         if (structure->typeInfo().newImpurePropertyFiresWatchpoints())
             vm->registerWatchpointForImpureProperty(ident, stubInfo.addWatchpoint(codeBlock));
 
+        if (slot.watchpointSet())
+            slot.watchpointSet()->add(stubInfo.addWatchpoint(codeBlock));
+
         Structure* currStructure = structure;
         WriteBarrier<Structure>* it = chain->head();
         for (unsigned i = 0; i < count; ++i, ++it) {

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2014-05-15  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        JSDOMWindow should not claim HasImpureGetOwnPropertySlot
+        https://bugs.webkit.org/show_bug.cgi?id=132918
+
+        Reviewed by Geoffrey Garen.
+
+        Tests: js/cached-window-properties.html
+               js/cached-window-prototype-properties.html
+
+        We now correctly handle the impurity of JSDOMWindow's custom getOwnPropertySlot without needing the 
+        blanket HasImpureGetOwnPropertySlot. We do this through the use of watchpoints and by explicitly forbidding
+        any caching beyond a certain point using PropertySlot::disableCaching. Getting rid of this flag will allow 
+        us to cache many properties/methods on both the JSDOMWindow and its prototype, which are very commonly used 
+        across the web.
+
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::JSDOMWindow::getOwnPropertySlot):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (HasComplexGetOwnProperty):
+        (InterfaceRequiresAttributesOnInstance):
+        (InstanceOverridesGetOwnPropertySlot):
+        (GenerateHeader):
+
 2014-05-15  Alexey Proskuryakov  <ap@apple.com>
 
         NetworkProcess crashes at ResourceHandle::continueDidReceiveResponse

diff --git a/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp b/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
@@ -192,7 +192,7 @@ bool JSDOMWindow::getOwnPropertySlot(JSObject* object, ExecState* exec, Property
 
     entry = JSDOMWindow::info()->propHashTable(exec)->entry(exec, propertyName);
     if (entry) {
-        slot.setCustom(thisObject, allowsAccess ? entry->attributes() : ReadOnly | DontDelete | DontEnum, entry->propertyGetter());
+        slot.setCacheableCustom(thisObject, allowsAccess ? entry->attributes() : ReadOnly | DontDelete | DontEnum, entry->propertyGetter());
         return true;
     }
 

diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
@@ -589,11 +589,11 @@ sub HasComplexGetOwnProperty
     my $indexedGetterFunction = GetIndexedGetterFunction($interface);
 
     my $hasImpureNamedGetter = $namedGetterFunction
-        || $interface->extendedAttributes->{"CustomNamedGetter"}
-        || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"};
+        || $interface->extendedAttributes->{"CustomNamedGetter"};
 
     my $hasComplexGetter = $indexedGetterFunction
         || $interface->extendedAttributes->{"JSCustomGetOwnPropertySlotAndDescriptor"}
+        || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"}
         || $hasImpureNamedGetter;
 
     return 1 if $interface->extendedAttributes->{"CheckSecurity"};
@@ -616,9 +616,9 @@ sub InterfaceRequiresAttributesOnInstance
     # FIXME: We should rearrange how custom named getters and getOwnPropertySlot
     # overrides are handled so that we get the correct semantics and lookup ordering
     my $hasImpureNamedGetter = $namedGetterFunction
-    || $interface->extendedAttributes->{"CustomNamedGetter"}
-    || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"};
-    return 1 if $hasImpureNamedGetter;
+        || $interface->extendedAttributes->{"CustomNamedGetter"};
+    return 1 if $hasImpureNamedGetter
+        || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"};
 
     # FIXME: These two should be fixed by removing the custom override of message, etc
     return 1 if $interfaceName =~ "Exception";
@@ -727,11 +727,11 @@ sub InstanceOverridesGetOwnPropertySlot
     my $indexedGetterFunction = GetIndexedGetterFunction($interface);
 
     my $hasImpureNamedGetter = $namedGetterFunction
-        || $interface->extendedAttributes->{"CustomNamedGetter"}
-        || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"};
+        || $interface->extendedAttributes->{"CustomNamedGetter"};
 
     my $hasComplexGetter = $indexedGetterFunction
         || $interface->extendedAttributes->{"JSCustomGetOwnPropertySlotAndDescriptor"}
+        || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"}
         || $hasImpureNamedGetter;
 
     return $numInstanceAttributes > 0 || !$interface->extendedAttributes->{"NoInterfaceObject"} || $hasComplexGetter;
@@ -891,14 +891,13 @@ sub GenerateHeader
     my $namedGetterFunction = GetNamedGetterFunction($interface);
     my $indexedGetterFunction = GetIndexedGetterFunction($interface);
 
-    my $hasImpureNamedGetter =
-        $namedGetterFunction
-        || $interface->extendedAttributes->{"CustomNamedGetter"}
-        || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"};
+    my $hasImpureNamedGetter = $namedGetterFunction
+        || $interface->extendedAttributes->{"CustomNamedGetter"};
 
     my $hasComplexGetter =
         $indexedGetterFunction
         || $interface->extendedAttributes->{"JSCustomGetOwnPropertySlotAndDescriptor"}
+        || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"}
         || $hasImpureNamedGetter;
 
     my $hasGetter = InstanceOverridesGetOwnPropertySlot($interface);