Add [CachedAccessor] attribute to cache (almost) constant accessors (window.document).

third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp

Index: third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp b/third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp
index 77d1a1ab3dc07c8734a8aef35a79b05615d20f55..e869bee02f9433fe26ae2458d618ed42e191bc70 100644
--- a/third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/V8DOMConfiguration.cpp
@@ -31,6 +31,7 @@
 #include "bindings/core/v8/GeneratedCodeHelper.h" // just for DCHECK
 #include "bindings/core/v8/V8ObjectConstructor.h"
 #include "bindings/core/v8/V8PerContextData.h"
+#include "bindings/core/v8/WindowProxy.h"
 #include "platform/TraceEvent.h"
 
 namespace blink {
@@ -91,10 +92,10 @@ void installAttributeInternal(v8::Isolate* isolate, v8::Local<v8::Object> instan
 }
 
 template <class FunctionOrTemplate>
-v8::Local<FunctionOrTemplate> createAccessorFunctionOrTemplate(v8::Isolate*, v8::FunctionCallback, V8DOMConfiguration::FastAccessorBuilderCallback, v8::Local<v8::Value> data, v8::Local<v8::Signature>, int length);
+v8::Local<FunctionOrTemplate> createAccessorFunctionOrTemplate(v8::Isolate*, v8::FunctionCallback, V8DOMConfiguration::FastAccessorBuilderCallback, const char* const surrogateName, v8::Local<v8::Value> data, v8::Local<v8::Signature>, int length);
 
 template <>
-v8::Local<v8::FunctionTemplate> createAccessorFunctionOrTemplate<v8::FunctionTemplate>(v8::Isolate* isolate, v8::FunctionCallback callback, V8DOMConfiguration::FastAccessorBuilderCallback fastCallback, v8::Local<v8::Value> data, v8::Local<v8::Signature> signature, int length)
+v8::Local<v8::FunctionTemplate> createAccessorFunctionOrTemplate<v8::FunctionTemplate>(v8::Isolate* isolate, v8::FunctionCallback callback, V8DOMConfiguration::FastAccessorBuilderCallback fastCallback, const char* const surrogateName, v8::Local<v8::Value> data, v8::Local<v8::Signature> signature, int length)
 {
     v8::Local<v8::FunctionTemplate> functionTemplate;
     if (callback) {
@@ -104,7 +105,18 @@ v8::Local<v8::FunctionTemplate> createAccessorFunctionOrTemplate<v8::FunctionTem
             DCHECK(fab);
             functionTemplate = v8::FunctionTemplate::NewWithFastHandler(isolate, callback, fab, data, signature, length);
         } else {
-            functionTemplate = v8::FunctionTemplate::New(isolate, callback, data, signature, length);
+            if (surrogateName) {
+                v8::Local<v8::String> name =
+                    v8::String::NewFromUtf8(isolate, surrogateName, v8::NewStringType::kNormal).ToLocalChecked();

[vogelheim, 2016/09/15 15:29:47]

nitpick: The line breaking is strange. Since Blink doesn't have an 80 columns
rule, maybe write it all in one line?

[Alfonso, 2016/09/16 14:21:47]

Done.

+
+                // Persist the private property in the global map.
+                v8::Local<v8::Private> property = v8::Private::New(isolate, name);
+                WindowProxy::cachedAccessors()[surrogateName].Set(isolate, property);
+
+                functionTemplate = v8::FunctionTemplate::NewSurrogate(isolate, callback, property, data, signature, length);
+            } else {
+                functionTemplate = v8::FunctionTemplate::New(isolate, callback, data, signature, length);
+            }
         }
 
         if (!functionTemplate.IsEmpty()) {
@@ -116,12 +128,12 @@ v8::Local<v8::FunctionTemplate> createAccessorFunctionOrTemplate<v8::FunctionTem
 }
 
 template <>
-v8::Local<v8::Function> createAccessorFunctionOrTemplate<v8::Function>(v8::Isolate* isolate, v8::FunctionCallback callback, V8DOMConfiguration::FastAccessorBuilderCallback, v8::Local<v8::Value> data, v8::Local<v8::Signature> signature, int length)
+v8::Local<v8::Function> createAccessorFunctionOrTemplate<v8::Function>(v8::Isolate* isolate, v8::FunctionCallback callback, V8DOMConfiguration::FastAccessorBuilderCallback, const char* const, v8::Local<v8::Value> data, v8::Local<v8::Signature> signature, int length)

[jochen (gone - plz use gerrit), 2016/09/15 08:20:51]

why not just const char*?

 {
     if (!callback)
         return v8::Local<v8::Function>();
 
-    v8::Local<v8::FunctionTemplate> functionTemplate = createAccessorFunctionOrTemplate<v8::FunctionTemplate>(isolate, callback, nullptr, data, signature, length);
+    v8::Local<v8::FunctionTemplate> functionTemplate = createAccessorFunctionOrTemplate<v8::FunctionTemplate>(isolate, callback, nullptr, 0, data, signature, length);

[jochen (gone - plz use gerrit), 2016/09/15 08:20:51]

nullptr

[Alfonso, 2016/09/16 14:21:47]

I'm confused about when to use 0 vs nullptr, I've read at least two
contradicting guidelines.
Done.

     if (functionTemplate.IsEmpty())
         return v8::Local<v8::Function>();
 
@@ -162,10 +174,20 @@ void installAccessorInternal(v8::Isolate* isolate, v8::Local<ObjectOrTemplate> i
 
     if (accessor.propertyLocationConfiguration &
         (V8DOMConfiguration::OnInstance | V8DOMConfiguration::OnPrototype)) {
-        v8::Local<FunctionOrTemplate> getter = createAccessorFunctionOrTemplate<FunctionOrTemplate>(isolate, getterCallback, fastGetterCallback, data, signature, 0);
-        v8::Local<FunctionOrTemplate> setter = createAccessorFunctionOrTemplate<FunctionOrTemplate>(isolate, setterCallback, nullptr, data, signature, 1);
-        if (accessor.propertyLocationConfiguration & V8DOMConfiguration::OnInstance)
+        v8::Local<FunctionOrTemplate> getter = createAccessorFunctionOrTemplate<FunctionOrTemplate>(isolate, getterCallback, fastGetterCallback, accessor.surrogatePropertyName, data, signature, 0);
+        v8::Local<FunctionOrTemplate> setter = createAccessorFunctionOrTemplate<FunctionOrTemplate>(isolate, setterCallback, nullptr, 0, data, signature, 1);
+        if (accessor.propertyLocationConfiguration & V8DOMConfiguration::OnInstance) {
             instanceOrTemplate->SetAccessorProperty(name, getter, setter, static_cast<v8::PropertyAttribute>(accessor.attribute), static_cast<v8::AccessControl>(accessor.settings));
+
+            // TODO(peterssen): Replicate for other 'locations' besides instance (prototype, interface...).
+            if (accessor.surrogatePropertyName) {
+                v8::Local<v8::Private> property =
+                    blink::WindowProxy::cachedAccessors().at(accessor.surrogatePropertyName).Get(isolate);
+
+                instanceOrTemplate->Set(v8::Private::AsSymbol(property).As<v8::Name>(), v8::Undefined(isolate));

[jochen (gone - plz use gerrit), 2016/09/15 08:20:51]

undefined would be the wrong value here, shouldn't that be whatever the getter
returns?

Do layout tests pass with this?

[vogelheim, 2016/09/15 15:29:47]

Wouldn't this usually be overwritten by WindowProxy::updateDocumentProperty()?

I'd think this would only be observable if one does the
new-window-then-navigate-away trick that forces this whole hack in the first
place. Not sure if this is tested, though. Anyway, would your suggestion be to
just call the getterCallback right here?

[Alfonso, 2016/09/16 14:21:47]

Ack-ed.
haraken@ also suggested to use the normal callback to initialize it, but I'm not
sure if everything is correctly initialized at this point.

[Alfonso, 2016/09/16 14:21:47]

I only have ran V8-side tests, but it will be removed anyway.
Done.

+            }
+        }
+
         if (accessor.propertyLocationConfiguration & V8DOMConfiguration::OnPrototype)
             prototypeOrTemplate->SetAccessorProperty(name, getter, setter, static_cast<v8::PropertyAttribute>(accessor.attribute), static_cast<v8::AccessControl>(accessor.settings));
     }
@@ -173,8 +195,8 @@ void installAccessorInternal(v8::Isolate* isolate, v8::Local<ObjectOrTemplate> i
         // Attributes installed on the interface object must be static
         // attributes, so no need to specify a signature, i.e. no need to do
         // type check against a holder.
-        v8::Local<FunctionOrTemplate> getter = createAccessorFunctionOrTemplate<FunctionOrTemplate>(isolate, getterCallback, fastGetterCallback, data, v8::Local<v8::Signature>(), 0);
-        v8::Local<FunctionOrTemplate> setter = createAccessorFunctionOrTemplate<FunctionOrTemplate>(isolate, setterCallback, nullptr, data, v8::Local<v8::Signature>(), 1);
+        v8::Local<FunctionOrTemplate> getter = createAccessorFunctionOrTemplate<FunctionOrTemplate>(isolate, getterCallback, fastGetterCallback, 0, data, v8::Local<v8::Signature>(), 0);
+        v8::Local<FunctionOrTemplate> setter = createAccessorFunctionOrTemplate<FunctionOrTemplate>(isolate, setterCallback, nullptr, 0, data, v8::Local<v8::Signature>(), 1);
         interfaceOrTemplate->SetAccessorProperty(name, getter, setter, static_cast<v8::PropertyAttribute>(accessor.attribute), static_cast<v8::AccessControl>(accessor.settings));
     }
 }