Title: [201428] trunk
Revision
201428
Author
[email protected]
Date
2016-05-26 12:51:26 -0700 (Thu, 26 May 2016)

Log Message

replaceable own properties seem to ignore replacement after property caching
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/_javascript_Core:

* 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.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (201427 => 201428)


--- trunk/LayoutTests/ChangeLog	2016-05-26 19:29:02 UTC (rev 201427)
+++ trunk/LayoutTests/ChangeLog	2016-05-26 19:51:26 UTC (rev 201428)
@@ -1,3 +1,15 @@
+2016-05-25  Geoffrey Garen  <[email protected]>
+
+        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  <[email protected]>
 
         Sites served over insecure connections should not be allowed to use geolocation.

Modified: trunk/LayoutTests/js/cached-window-properties.html (201427 => 201428)


--- trunk/LayoutTests/js/cached-window-properties.html	2016-05-26 19:29:02 UTC (rev 201427)
+++ trunk/LayoutTests/js/cached-window-properties.html	2016-05-26 19:51:26 UTC (rev 201428)
@@ -1,31 +1,37 @@
-<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<!DOCTYPE HTML>
 <html>
 <head>
 <script src=""
 </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);
+var test = function(id, x)
+{
+	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 for " + id);
 }
-if (sum !== x * niters)
-    throw new Error("Incorrect sum");
 
-window.screenX = 42;
+var x = window.screenX;
+test("x", x);
 
-sum = 0;
-for (var i = 0; i < niters; ++i) {
-    sum += foo(window);
-}
-if (sum !== 42 * niters)
-    throw new Error("Incorrect sum");
+var newX = window.screenX + 1;
+window.screenX = newX;
+test("newX", newX);
 </script>
 <script src=""
 </body>

Modified: trunk/PerformanceTests/ChangeLog (201427 => 201428)


--- trunk/PerformanceTests/ChangeLog	2016-05-26 19:29:02 UTC (rev 201427)
+++ trunk/PerformanceTests/ChangeLog	2016-05-26 19:51:26 UTC (rev 201428)
@@ -1,3 +1,20 @@
+2016-05-25  Geoffrey Garen  <[email protected]>
+
+        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  <[email protected]>
 
         Unreviewed, add JSBench to the skipped list for now since it doesn't

Modified: trunk/Source/_javascript_Core/ChangeLog (201427 => 201428)


--- trunk/Source/_javascript_Core/ChangeLog	2016-05-26 19:29:02 UTC (rev 201427)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-05-26 19:51:26 UTC (rev 201428)
@@ -1,3 +1,15 @@
+2016-05-25  Geoffrey Garen  <[email protected]>
+
+        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  <[email protected]>
 
         [JSC] RegExp with deeply nested subexpressions overflow the stack in Yarr

Modified: trunk/Source/_javascript_Core/runtime/Lookup.h (201427 => 201428)


--- trunk/Source/_javascript_Core/runtime/Lookup.h	2016-05-26 19:29:02 UTC (rev 201427)
+++ trunk/Source/_javascript_Core/runtime/Lookup.h	2016-05-26 19:51:26 UTC (rev 201428)
@@ -288,6 +288,17 @@
     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).

Modified: trunk/Source/WebCore/ChangeLog (201427 => 201428)


--- trunk/Source/WebCore/ChangeLog	2016-05-26 19:29:02 UTC (rev 201427)
+++ trunk/Source/WebCore/ChangeLog	2016-05-26 19:51:26 UTC (rev 201428)
@@ -1,3 +1,18 @@
+2016-05-25  Geoffrey Garen  <[email protected]>
+
+        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

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (201427 => 201428)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2016-05-26 19:29:02 UTC (rev 201427)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2016-05-26 19:51:26 UTC (rev 201428)
@@ -2910,8 +2910,12 @@
                 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"};
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to