JSDOMWindow should not claim HasImpureGetOwnPropertySlot
authormhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 May 2014 23:03:06 +0000 (23:03 +0000)
committermhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 May 2014 23:03:06 +0000 (23:03 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/js/cached-window-properties-expected.txt [new file with mode: 0644]
LayoutTests/js/cached-window-properties.html [new file with mode: 0644]
LayoutTests/js/cached-window-prototype-properties-expected.txt [new file with mode: 0644]
LayoutTests/js/cached-window-prototype-properties.html [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jit/Repatch.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

index 659ef72..22f6580 100644 (file)
@@ -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
new file mode 100644 (file)
index 0000000..43e5eee
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..c42e837
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..43e5eee
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..a2137f4
--- /dev/null
@@ -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>
index e16d37a..a37afd1 100644 (file)
@@ -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.
index c77ce5a..5f0cbf2 100644 (file)
@@ -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) {
index d3e6437..997c685 100644 (file)
@@ -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
index e311861..5b20a31 100644 (file)
@@ -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;
     }
 
index 035f7db..e0b5753 100644 (file)
@@ -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);