IDL compiler: [InitializedByEventConstructor] attribute any

IDL compiler: [InitializedByEventConstructor] attribute any

Pretty complex special case.

BUG=239771
NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163416

[haraken, 2013-12-09 08:37:00]

LGTM

https://codereview.chromium.org/110023002/diff/20001/Source/bindings/template...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/110023002/diff/20001/Source/bindings/template...
Source/bindings/templates/interface.cpp:186: {% if has_any_type_attributes %}

Nit: You're passing the result of

  any(attribute for attribute in interface.attributes if attribute.idl_type ==
'any')

from the code generator to the template file as |has_any_type_attributes|. On
the other hand, you're writing

You're writing

  for attribute in attributes if attribute.idl_type == 'any'

in the template file.

Isn't there a better way to remove the redundancy?

https://codereview.chromium.org/110023002/diff/20001/Source/bindings/template...
Source/bindings/templates/interface.cpp:189: another world. The main world case
is handled lazily (in Custom code). #}

Custom => custom

https://codereview.chromium.org/110023002/diff/20001/Source/bindings/template...
Source/bindings/templates/interface.cpp:193:
event->{{attribute.set_serialized}}(SerializedScriptValue::createAndSwallowExceptions({{attribute.name}},
info.GetIsolate()));

set_serialized => set_serialized_script_value (since SerializedScriptValue is a
well-know class name) ?

[Nils Barth (inactive), 2013-12-09 09:55:01]

Thanks for comments;
overall logic will be clearer when I post followup CL for
[Unserialized].

https://codereview.chromium.org/110023002/diff/20001/Source/bindings/template...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/110023002/diff/20001/Source/bindings/template...
Source/bindings/templates/interface.cpp:186: {% if has_any_type_attributes %}
On 2013/12/09 08:37:00, haraken wrote:
> 
> Nit: You're passing the result of
> 
>   any(attribute for attribute in interface.attributes if attribute.idl_type ==
> 'any')
> 
> from the code generator to the template file as |has_any_type_attributes|. On
> the other hand, you're writing
> 
> You're writing
> 
>   for attribute in attributes if attribute.idl_type == 'any'
> 
> in the template file.
> 
> Isn't there a better way to remove the redundancy?

In this case, no, because we need to add a special case for
[Unserializable], which changes 2 of these 4 lists,
and thus this would get confusing if we try to abstract it away.
(It's a bit verbose, but the logic really is that verbose.)

In general I've been putting the filters directly at the loop sites,
so we can see directly from the template what's going on, and
to flatten the logic.

For example, we have:
{% for attribute in attributes if attribute.runtime_enabled_function %}
...instead of
template:
{% for attribute in runtime_enabled_attributes %}
Python:
runtime_enabled_attributes = [attribute for attribute in attributes if
attribute.runtime_enabled_function]

In this case there are 3 loops, so it's a bit redundant and we
could use an auxiliary variable for the loop range, but as noted, the
logic is about to get more complicated and we can't do this.

In the followup CL (implementing [Unserialized]) I'll see what we
can do to keep this simple.

https://codereview.chromium.org/110023002/diff/20001/Source/bindings/template...
Source/bindings/templates/interface.cpp:189: another world. The main world case
is handled lazily (in Custom code). #}
On 2013/12/09 08:37:00, haraken wrote:
> 
> Custom => custom

Done.

https://codereview.chromium.org/110023002/diff/20001/Source/bindings/template...
Source/bindings/templates/interface.cpp:193:
event->{{attribute.set_serialized}}(SerializedScriptValue::createAndSwallowExceptions({{attribute.name}},
info.GetIsolate()));
On 2013/12/09 08:37:00, haraken wrote:
> 
> set_serialized => set_serialized_script_value (since SerializedScriptValue is
a
> well-know class name) ?

Done.

[commit-bot: I haz the power, 2013-12-09 09:55:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/110023002/40001

[commit-bot: I haz the power, 2013-12-09 09:55:38]

Failed to apply patch for
Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp
  Hunk #1 FAILED at 37.
  Hunk #2 succeeded at 71 (offset -3 lines).
  Hunk #3 succeeded at 118 (offset -3 lines).
  Hunk #4 succeeded at 127 (offset -3 lines).
  Hunk #5 succeeded at 145 (offset -3 lines).
  Hunk #6 succeeded at 156 (offset -3 lines).
  1 out of 6 hunks FAILED -- saving rejects to file
Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp.rej

Patch:       Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp
Index: Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp
diff --git a/Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp
b/Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp
index
5e9a5b963d63289deb9debd78e37d2c6d95020d8..20902c965c872fb5b4c8f93037d026a0f5bca2d0
100644
--- a/Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp
+++ b/Source/bindings/tests/results/V8TestInterfaceEventConstructor.cpp
@@ -37,6 +37,8 @@
 #include "V8Event.h"
 #include "bindings/v8/Dictionary.h"
 #include "bindings/v8/ExceptionMessages.h"
+#include "bindings/v8/ScriptValue.h"
+#include "bindings/v8/SerializedScriptValue.h"
 #include "bindings/v8/V8Binding.h"
 #include "bindings/v8/V8DOMConfiguration.h"
 #include "bindings/v8/V8DOMWrapper.h"
@@ -74,29 +76,42 @@ namespace TestInterfaceEventConstructorV8Internal {
 
 template <typename T> void V8_USE(T) { }
 
-static void attr1AttributeGetter(const v8::PropertyCallbackInfo<v8::Value>&
info)
+static void readonlyStringAttributeAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
 {
     TestInterfaceEventConstructor* imp =
V8TestInterfaceEventConstructor::toNative(info.Holder());
-    v8SetReturnValueString(info, imp->attr1(), info.GetIsolate());
+    v8SetReturnValueString(info, imp->readonlyStringAttribute(),
info.GetIsolate());
 }
 
-static void attr1AttributeGetterCallback(v8::Local<v8::String>, const
v8::PropertyCallbackInfo<v8::Value>& info)
+static void
readonlyStringAttributeAttributeGetterCallback(v8::Local<v8::String>, const
v8::PropertyCallbackInfo<v8::Value>& info)
 {
     TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
-    TestInterfaceEventConstructorV8Internal::attr1AttributeGetter(info);
+   
TestInterfaceEventConstructorV8Internal::readonlyStringAttributeAttributeGetter(info);
     TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
 }
 
-static void attr2AttributeGetter(const v8::PropertyCallbackInfo<v8::Value>&
info)
+static void
InitializedByEventConstructorReadonlyStringAttributeAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
 {
     TestInterfaceEventConstructor* imp =
V8TestInterfaceEventConstructor::toNative(info.Holder());
-    v8SetReturnValueString(info, imp->attr2(), info.GetIsolate());
+    v8SetReturnValueString(info,
imp->initializedByEventConstructorReadonlyStringAttribute(), info.GetIsolate());
 }
 
-static void attr2AttributeGetterCallback(v8::Local<v8::String>, const
v8::PropertyCallbackInfo<v8::Value>& info)
+static void
InitializedByEventConstructorReadonlyStringAttributeAttributeGetterCallback(v8::Local<v8::String>,
const v8::PropertyCallbackInfo<v8::Value>& info)
 {
     TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
-    TestInterfaceEventConstructorV8Internal::attr2AttributeGetter(info);
+   
TestInterfaceEventConstructorV8Internal::InitializedByEventConstructorReadonlyStringAttributeAttributeGetter(info);
+    TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
+static void
InitializedByEventConstructorReadonlyAnyAttributeAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
+{
+    TestInterfaceEventConstructor* imp =
V8TestInterfaceEventConstructor::toNative(info.Holder());
+    v8SetReturnValue(info,
imp->initializedByEventConstructorReadonlyAnyAttribute().v8Value());
+}
+
+static void
InitializedByEventConstructorReadonlyAnyAttributeAttributeGetterCallback(v8::Local<v8::String>,
const v8::PropertyCallbackInfo<v8::Value>& info)
+{
+    TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
+   
TestInterfaceEventConstructorV8Internal::InitializedByEventConstructorReadonlyAnyAttributeAttributeGetter(info);
     TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
 }
 
@@ -108,6 +123,7 @@ static void constructor(const
v8::FunctionCallbackInfo<v8::Value>& info)
     }
 
     V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<>, type, info[0]);
+    v8::Local<v8::Value> InitializedByEventConstructorReadonlyAnyAttribute;
     TestInterfaceEventConstructorInit eventInit;
     if (info.Length() >= 2) {
         V8TRYCATCH_VOID(Dictionary, options, Dictionary(info[1],
info.GetIsolate()));
@@ -116,8 +132,16 @@ static void constructor(const
v8::FunctionCallbackInfo<v8::Value>& info)
             exceptionState.throwIfNeeded();
             return;
         }
+        options.get("InitializedByEventConstructorReadonlyAnyAttribute",
InitializedByEventConstructorReadonlyAnyAttribute);
+        if (!InitializedByEventConstructorReadonlyAnyAttribute.IsEmpty())
+           
info.Holder()->SetHiddenValue(V8HiddenPropertyName::InitializedByEventConstructorReadonlyAnyAttribute(info.GetIsolate()),
InitializedByEventConstructorReadonlyAnyAttribute);
     }
     RefPtr<TestInterfaceEventConstructor> event =
TestInterfaceEventConstructor::create(type, eventInit);
+    if (isolatedWorldForIsolate(info.GetIsolate())) {
+        if (!InitializedByEventConstructorReadonlyAnyAttribute.IsEmpty())
+           
event->setSerializedInitializedByEventConstructorReadonlyAnyAttribute(SerializedScriptValue::createAndSwallowExceptions(InitializedByEventConstructorReadonlyAnyAttribute,
info.GetIsolate()));
+    }
+
     v8::Handle<v8::Object> wrapper = info.Holder();
    
V8DOMWrapper::associateObjectWithWrapper<V8TestInterfaceEventConstructor>(event.release(),
&V8TestInterfaceEventConstructor::wrapperTypeInfo, wrapper, info.GetIsolate(),
WrapperConfiguration::Dependent);
     v8SetReturnValue(info, wrapper);
@@ -126,8 +150,9 @@ static void constructor(const
v8::FunctionCallbackInfo<v8::Value>& info)
 } // namespace TestInterfaceEventConstructorV8Internal
 
 static const V8DOMConfiguration::AttributeConfiguration
V8TestInterfaceEventConstructorAttributes[] = {
-    {"attr1",
TestInterfaceEventConstructorV8Internal::attr1AttributeGetterCallback, 0, 0, 0,
0, static_cast<v8::AccessControl>(v8::DEFAULT),
static_cast<v8::PropertyAttribute>(v8::None), 0 /* on instance */},
-    {"attr2",
TestInterfaceEventConstructorV8Internal::attr2AttributeGetterCallback, 0, 0, 0,
0, static_cast<v8::AccessControl>(v8::DEFAULT),
static_cast<v8::PropertyAttribute>(v8::None), 0 /* on instance */},
+    {"readonlyStringAttribute",
TestInterfaceEventConstructorV8Internal::readonlyStringAttributeAttributeGetterCallback,
0, 0, 0, 0, static_cast<v8::AccessControl>(v8::DEFAULT),
static_cast<v8::PropertyAttribute>(v8::None), 0 /* on instance */},
+    {"InitializedByEventConstructorReadonlyStringAttribute",
TestInterfaceEventConstructorV8Internal::InitializedByEventConstructorReadonlyStringAttributeAttributeGetterCallback,
0, 0, 0, 0, static_cast<v8::AccessControl>(v8::DEFAULT),
static_cast<v8::PropertyAttribute>(v8::None), 0 /* on instance */},
+    {"InitializedByEventConstructorReadonlyAnyAttribute",
TestInterfaceEventConstructorV8Internal::InitializedByEventConstructorReadonlyAnyAttributeAttributeGetterCallback,
0, 0, 0, 0, static_cast<v8::AccessControl>(v8::DEFAULT),
static_cast<v8::PropertyAttribute>(v8::None), 0 /* on instance */},
 };
 
 bool initializeTestInterfaceEventConstructor(TestInterfaceEventConstructorInit&
eventInit, const Dictionary& options, ExceptionState& exceptionState, const
String& forEventName)
@@ -136,7 +161,7 @@ bool
initializeTestInterfaceEventConstructor(TestInterfaceEventConstructorInit&
     if (!initializeEvent(eventInit, options, exceptionState,
forEventName.isEmpty() ? String("TestInterfaceEventConstructor") :
forEventName))
         return false;
 
-    if (!options.convert(conversionContext, "attr2", eventInit.attr2))
+    if (!options.convert(conversionContext,
"InitializedByEventConstructorReadonlyStringAttribute",
eventInit.InitializedByEventConstructorReadonlyStringAttribute))
         return false;
     return true;
 }

[commit-bot: I haz the power, 2013-12-09 09:57:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/110023002/60001

[commit-bot: I haz the power, 2013-12-09 09:57:51]

Change committed as 163416