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

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

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 13 matching lines @@
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "bindings/core/v8/V8DOMConfiguration.h"
 
 #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 {
 
 namespace {
 
 void installAttributeInternal(v8::Isolate* isolate, v8::Local<v8::ObjectTemplate> instanceTemplate, v8::Local<v8::ObjectTemplate> prototypeTemplate, const V8DOMConfiguration::AttributeConfiguration& attribute, const DOMWrapperWorld& world)
 {
     if (attribute.exposeConfiguration == V8DOMConfiguration::OnlyExposedToPrivateScript
         && !world.isPrivateScriptIsolatedWorld())
@@ skipping 40 matching lines @@
     DCHECK(attribute.propertyLocationConfiguration);
     if (attribute.propertyLocationConfiguration & V8DOMConfiguration::OnInstance)
         instance->DefineOwnProperty(isolate->GetCurrentContext(), name, data, static_cast<v8::PropertyAttribute>(attribute.attribute)).ToChecked();
     if (attribute.propertyLocationConfiguration & V8DOMConfiguration::OnPrototype)
         prototype->DefineOwnProperty(isolate->GetCurrentContext(), name, data, static_cast<v8::PropertyAttribute>(attribute.attribute)).ToChecked();
     if (attribute.propertyLocationConfiguration & V8DOMConfiguration::OnInterface)
         NOTREACHED();
 }
 
 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) {
         // Fast accessors are guarded by the FastDOMAccessors runtime flag.
         if (RuntimeEnabledFeatures::fastDOMAccessorsEnabled() && fastCallback) {
             v8::experimental::FastAccessorBuilder* fab = fastCallback(isolate, callback);
             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()) {
             functionTemplate->RemovePrototype();
             functionTemplate->SetAcceptAnyReceiver(false);
         }
     }
     return functionTemplate;
 }
 
 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>();
 
     v8::Local<v8::Context> context = isolate->GetCurrentContext();
     v8::Local<v8::Function> function;
     if (!functionTemplate->GetFunction(context).ToLocal(&function))
         return v8::Local<v8::Function>();
     return function;
 }
 
@@ skipping 20 matching lines @@
     if (accessor.holderCheckConfiguration == V8DOMConfiguration::DoNotCheckHolder)
         signature = v8::Local<v8::Signature>();
     v8::Local<v8::Value> data = v8::External::New(isolate, const_cast<WrapperTypeInfo*>(accessor.data));
 
     DCHECK(accessor.propertyLocationConfiguration);
 
     V8DOMConfiguration::FastAccessorBuilderCallback fastGetterCallback = accessor.fastGetter;
 
     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));
     }
     if (accessor.propertyLocationConfiguration & V8DOMConfiguration::OnInterface) {
         // 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));
     }
 }
 
 v8::Local<v8::Primitive> valueForConstant(v8::Isolate* isolate, const V8DOMConfiguration::ConstantConfiguration& constant)
 {
     v8::Local<v8::Primitive> value;
     switch (constant.type) {
     case V8DOMConfiguration::ConstantTypeShort:
     case V8DOMConfiguration::ConstantTypeLong:
@@ skipping 210 matching lines @@
     data->setInterfaceTemplate(world, wrapperTypeInfo, result);
     return result;
 }
 
 void V8DOMConfiguration::setClassString(v8::Isolate* isolate, v8::Local<v8::ObjectTemplate> objectTemplate, const char* classString)
 {
     objectTemplate->Set(v8::Symbol::GetToStringTag(isolate), v8AtomicString(isolate, classString), static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontEnum));
 }
 
 } // namespace blink