replaceable own properties seem to ignore replacement after property caching
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 May 2016 19:51:26 +0000 (19:51 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 May 2016 19:51:26 +0000 (19:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158091

Reviewed by Darin Adler.

PerformanceTests:

* MallocBench/MallocBench.xcodeproj/project.pbxproj:
* MallocBench/MallocBench/Benchmark.cpp:
* MallocBench/MallocBench/Interpreter.cpp:
(Interpreter::doMallocOp):
* MallocBench/MallocBench/Interpreter.h:
* MallocBench/MallocBench/fastMallocLog.63316.ops: Added.
* MallocBench/MallocBench/jetstream.cpp: Added.
(benchmark_jetstream):
* MallocBench/MallocBench/jetstream.h: Added.

Source/JavaScriptCore:

* runtime/Lookup.h:
(JSC::replaceStaticPropertySlot): New helper function for replacing a
static property with a direct property. We need to do an attribute changed
transition because client code might have cached our static property.

Source/WebCore:

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation): Use our new replacement helper if we're replacing
an own static property with an own direct property. Because we advertise
that our own static properties are cacheable, we need to do a structure
transition to indicate when they change. (Only own properties need this
special treatment because JSC considers it normal to shadow a prototype
property with an own property.)

LayoutTests:

* js/cached-window-properties.html: Augmneted this test to enter cacheable
dictionary mode in order to demonstrate a bug that is not visible otherwise.

Factored out a helper test function.

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

LayoutTests/ChangeLog
LayoutTests/js/cached-window-properties.html
PerformanceTests/ChangeLog
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/Lookup.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

index 3b31fa5ac5af4597b6970f0dee7dcd481408d2ef..2a2aedefda2890e330a51c3176b186d4dfbc6ed3 100644 (file)
@@ -1,3 +1,15 @@
+2016-05-25  Geoffrey Garen  <ggaren@apple.com>
+
+        replaceable own properties seem to ignore replacement after property caching
+        https://bugs.webkit.org/show_bug.cgi?id=158091
+
+        Reviewed by Darin Adler.
+
+        * js/cached-window-properties.html: Augmneted this test to enter cacheable
+        dictionary mode in order to demonstrate a bug that is not visible otherwise.
+
+        Factored out a helper test function.
+
 2016-05-26  Pranjal Jumde  <pjumde@apple.com>
 
         Sites served over insecure connections should not be allowed to use geolocation.
index c42e837ebfa77bc39b92b7c10f1add7f8e52c8fe..3f0294bffa7d6946b9197269df80e1ea7f8c9c1c 100644 (file)
@@ -1,31 +1,37 @@
-<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<!DOCTYPE HTML>
 <html>
 <head>
 <script src="../resources/js-test-pre.js"></script>
 </head>
 <body>
 <script>
+
+// Force the window into cacheable dictionary mode.
+for (var i = 0; i < 100; ++i)
+       window["p" + i] = i;
+
 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");
+var test = function(id, x)
+{
+       var niters = 100000;
 
-window.screenX = 42;
+       var sum = 0;
+       for (var i = 0; i < niters; ++i)
+           sum += foo(window);
 
-sum = 0;
-for (var i = 0; i < niters; ++i) {
-    sum += foo(window);
+       if (sum !== x * niters)
+           throw new Error("Incorrect sum for " + id);
 }
-if (sum !== 42 * niters)
-    throw new Error("Incorrect sum");
+
+var x = window.screenX;
+test("x", x);
+
+var newX = window.screenX + 1;
+window.screenX = newX;
+test("newX", newX);
 </script>
 <script src="../resources/js-test-post.js"></script>
 </body>
index 14cd2dc658d2987a5aed809383346ba6e579336d..0bd70db6abbecec0eb8907d56c9fe891111ab48a 100644 (file)
@@ -1,3 +1,20 @@
+2016-05-25  Geoffrey Garen  <ggaren@apple.com>
+
+        replaceable own properties seem to ignore replacement after property caching
+        https://bugs.webkit.org/show_bug.cgi?id=158091
+
+        Reviewed by Darin Adler.
+
+        * MallocBench/MallocBench.xcodeproj/project.pbxproj:
+        * MallocBench/MallocBench/Benchmark.cpp:
+        * MallocBench/MallocBench/Interpreter.cpp:
+        (Interpreter::doMallocOp):
+        * MallocBench/MallocBench/Interpreter.h:
+        * MallocBench/MallocBench/fastMallocLog.63316.ops: Added.
+        * MallocBench/MallocBench/jetstream.cpp: Added.
+        (benchmark_jetstream):
+        * MallocBench/MallocBench/jetstream.h: Added.
+
 2016-05-25  Keith Miller  <keith_miller@apple.com>
 
         Unreviewed, add JSBench to the skipped list for now since it doesn't
index 5ebad68315cc5a535a965fe34633aebfc15883fc..0d754596d701346739cab2ad17bb6717da8e7095 100644 (file)
@@ -1,3 +1,15 @@
+2016-05-25  Geoffrey Garen  <ggaren@apple.com>
+
+        replaceable own properties seem to ignore replacement after property caching
+        https://bugs.webkit.org/show_bug.cgi?id=158091
+
+        Reviewed by Darin Adler.
+
+        * runtime/Lookup.h:
+        (JSC::replaceStaticPropertySlot): New helper function for replacing a
+        static property with a direct property. We need to do an attribute changed
+        transition because client code might have cached our static property.
+
 2016-05-25  Benjamin Poulain  <benjamin@webkit.org>
 
         [JSC] RegExp with deeply nested subexpressions overflow the stack in Yarr
index 7b54f1918b031cd7a011bde3c950f807d356e4bc..83c7344d859e28d49cd0eb70527667833985b7cb 100644 (file)
@@ -288,6 +288,17 @@ inline bool getStaticValueSlot(ExecState* exec, const HashTable& table, ThisImp*
     return true;
 }
 
+inline bool replaceStaticPropertySlot(VM& vm, JSObject* thisObject, PropertyName propertyName, JSValue value)
+{
+    if (!thisObject->putDirect(vm, propertyName, value))
+        return false;
+
+    if (!thisObject->staticFunctionsReified())
+        thisObject->JSObject::setStructure(vm, Structure::attributeChangeTransition(vm, thisObject->structure(), propertyName, 0));
+
+    return true;
+}
+
 // 'base' means the object holding the property (possibly in the prototype chain of the object put was called on).
 // 'thisValue' is the object that put is being applied to (in the case of a proxy, the proxy target).
 // 'slot.thisValue()' is the object the put was originally performed on (in the case of a proxy, the proxy itself).
index 8b811f1dc05826de1c73f0178b544e36b3f73062..bec2eadb990f1edc096eac0b42202b4c06cf80ea 100644 (file)
@@ -1,3 +1,18 @@
+2016-05-25  Geoffrey Garen  <ggaren@apple.com>
+
+        replaceable own properties seem to ignore replacement after property caching
+        https://bugs.webkit.org/show_bug.cgi?id=158091
+
+        Reviewed by Darin Adler.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateImplementation): Use our new replacement helper if we're replacing
+        an own static property with an own direct property. Because we advertise
+        that our own static properties are cacheable, we need to do a structure
+        transition to indicate when they change. (Only own properties need this 
+        special treatment because JSC considers it normal to shadow a prototype
+        property with an own property.)
+
 2016-05-26  Said Abou-Hallawa  <sabouhallawa@apple,com>
 
         BitmapImage::checkForSolidColor() cleanup
index d880ddd6af60ed5a5f10f55c12cac8f36d292b7a..019ff614abfdadeaedf90193e10afab87f61fb71 100644 (file)
@@ -2910,8 +2910,12 @@ sub GenerateImplementation
                 push(@implContent, "    // Shadowing a built-in constructor.\n");
                 push(@implContent, "    return castedThis->putDirect(state->vm(), Identifier::fromString(state, \"$name\"), value);\n");
             } elsif ($attribute->signature->extendedAttributes->{"Replaceable"}) {
-                push(@implContent, "    // Shadowing a built-in object.\n");
-                push(@implContent, "    return castedThis->putDirect(state->vm(), Identifier::fromString(state, \"$name\"), value);\n");
+                push(@implContent, "    // Shadowing a built-in property.\n");
+                if (AttributeShouldBeOnInstance($interface, $attribute)) {
+                    push(@implContent, "    return replaceStaticPropertySlot(state->vm(), castedThis, Identifier::fromString(state, \"$name\"), value);\n");
+                } else {
+                    push(@implContent, "    return castedThis->putDirect(state->vm(), Identifier::fromString(state, \"$name\"), value);\n");
+                }
             } else {
                 if (!$attribute->isStatic) {
                     my $putForwards = $attribute->signature->extendedAttributes->{"PutForwards"};