Pass current value of attributes to WebDOMActivityLogger Setter logs.

Source/bindings/templates/attributes.cpp

 {##############################################################################}
 {% macro attribute_getter(attribute, world_suffix) %}
 {% filter conditional(attribute.conditional_string) %}
 static void {{attribute.name}}AttributeGetter{{world_suffix}}(
 {%- if attribute.is_expose_js_accessors %}
 const v8::FunctionCallbackInfo<v8::Value>& info
 {%- else %}
 const v8::PropertyCallbackInfo<v8::Value>& info
 {%- endif %})
 {
@@ skipping 153 matching lines @@
     TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
     {% if attribute.deprecate_as %}
     UseCounter::countDeprecation(callingExecutionContext(info.GetIsolate()), UseCounter::{{attribute.deprecate_as}});
     {% endif %}
     {% if attribute.measure_as %}
     UseCounter::count(callingExecutionContext(info.GetIsolate()), UseCounter::{{attribute.measure_as}});
     {% endif %}
     {% if world_suffix in attribute.activity_logging_world_list_for_getter %}
     V8PerContextData* contextData = V8PerContextData::from(info.GetIsolate()->GetCurrentContext());
     if (contextData && contextData->activityLogger())
-        contextData->activityLogger()->log("{{interface_name}}.{{attribute.name}}", 0, 0, "Getter");
+        contextData->activityLogger()->logGetter("{{interface_name}}.{{attribute.name}}");
     {% endif %}
     {% if attribute.has_custom_getter %}
     {{v8_class}}::{{attribute.name}}AttributeGetterCustom(info);
     {% else %}
     {{cpp_class}}V8Internal::{{attribute.name}}AttributeGetter{{world_suffix}}(info);
     {% endif %}
     TRACE_EVENT_SET_SAMPLING_STATE("V8", "V8Execution");
 }
 {% endfilter %}
 {% endmacro %}
@@ skipping 119 matching lines @@
     TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMSetter");
     {% if attribute.deprecate_as %}
     UseCounter::countDeprecation(callingExecutionContext(info.GetIsolate()), UseCounter::{{attribute.deprecate_as}});
     {% endif %}
     {% if attribute.measure_as %}
     UseCounter::count(callingExecutionContext(info.GetIsolate()), UseCounter::{{attribute.measure_as}});
     {% endif %}
     {% if world_suffix in attribute.activity_logging_world_list_for_setter %}
     V8PerContextData* contextData = V8PerContextData::from(info.GetIsolate()->GetCurrentContext());
     if (contextData && contextData->activityLogger()) {
-        v8::Handle<v8::Value> loggerArg[] = { v8Value };
-        contextData->activityLogger()->log("{{interface_name}}.{{attribute.name}}", 1, &loggerArg[0], "Setter");
+        {% if attribute.activity_logging_include_old_value_for_setter %}
+        {{cpp_class}}* impl = {{v8_class}}::toNative(info.Holder());
+        {% if attribute.cpp_value_original %}
+        {{attribute.cpp_type}} original = {{attribute.cpp_value_original}};
+        {% else %}
+        {{attribute.cpp_type}} original = {{attribute.cpp_value}};
+        {% endif %}
+        v8::Handle<v8::Value> originalValue = {{attribute.cpp_value_to_v8_value(cpp_value='original', isolate='info.GetIsolate()', creation_context='v8::Handle<v8::Object>()')}};

[Nils Barth (inactive), 2014/04/23 01:12:02]

Could you put the function call in the Python code, not the template?

[Devlin, 2014/04/23 01:31:43]

Yes.  But I didn't because there's so much context that's lost when that happens
(it's unclear then what 'original' and 'info.GetIsolate()' refer to).  But if
it's preferred, I have no problem doing so.  Preference?  (Made the change, but
happy to change back.)

[Nils Barth (inactive), 2014/04/23 03:04:42]

That's a good point, but as a rule we're putting all these calls in the Python
logic, to avoid Python ⇄ Jinja cycles:
this way you can read the Jinja and just refer to Python to understand variable
values, rather than pingponging; also simplifies the template, which is
otherwise a bit hard to read.
(Your suggestion is sane, but not how we've been doing it: we've been keeping a
strict pipeline.)

I've documented this at:
http://www.chromium.org/developers/jinja#TOC-Python-Jinja-split

In this case the only context is the variable name 'original' (isolate is
default), and we've considered that acceptable.

Thanks for your explanation!

+        {% else %}
+        {# If we don't log the old value, we send an empty value in its place. #}

[Nils Barth (inactive), 2014/04/23 01:12:02]

Would it be clearer to overload logSetter so we can just omit sending a dummy
value?

[Devlin, 2014/04/23 01:31:43]

Sure.  (Originally didn't because chrome doesn't like overloading, but this
isn't chrome.) :)

[Nils Barth (inactive), 2014/04/23 03:04:42]

...at least for the next day or two ;)

+        v8::Handle<v8::Value> originalValue;
+        {% endif %}
+        contextData->activityLogger()->logSetter("{{interface_name}}.{{attribute.name}}", v8Value, originalValue);
     }
     {% endif %}
     {% if attribute.is_custom_element_callbacks or attribute.is_reflect %}
     CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
     {% endif %}
     {% if attribute.has_custom_setter %}
     {{v8_class}}::{{attribute.name}}AttributeSetterCustom(v8Value, info);
     {% else %}
     {{cpp_class}}V8Internal::{{attribute.name}}AttributeSetter{{world_suffix}}(v8Value, info);
     {% endif %}
     TRACE_EVENT_SET_SAMPLING_STATE("V8", "V8Execution");
 }
 {% endfilter %}
 {% endmacro %}