Whitelist side-effect-free indexed, named property getters in IDLs

This CL whitelists indexed and named property getters of common
collections with [Affects] extended attribute. This enables:
- div.classList[0]
- div.attributes.specific_attribute

to evaluate with throwOnSideEffect: true.

Bug: 829571
Change-Id: I64fb6ad154c14e0119d3b09870f4c9dd10c9de41
Reviewed-on: https://chromium-review.googlesource.com/1025256
Commit-Queue: Erik Luo <luoe@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553314}

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/side-effects/evaluate-embedder-side-effect-free-methods-expected.txt

@@ -85,4 +85,42 @@ Expression `textNode.querySelectorAll('div')`
 has side effect: false, expected: false
 Expression `global_performance.now()`
 has side effect: false, expected: false
+Expression `htmlCollection[0]`
+has side effect: false, expected: false
+Expression `htmlCollection.item(0)`
+has side effect: false, expected: false
+Expression `htmlCollection.length`
+has side effect: false, expected: false
+Expression `nodeList[0]`
+has side effect: false, expected: false
+Expression `nodeList.item(0)`
+has side effect: false, expected: false
+Expression `nodeList.length`
+has side effect: false, expected: false
+Expression `$$result[0]`
+has side effect: false, expected: false
+Expression `$$result.item(0)`
+has side effect: false, expected: false
+Expression `$$result.length`
+has side effect: false, expected: false
+Expression `domTokenList[0]`
+has side effect: false, expected: false
+Expression `domTokenList.item(0)`
+has side effect: false, expected: false
+Expression `domTokenList.length`
+has side effect: false, expected: false
+Expression `bodyStyle[0]`
+has side effect: false, expected: false
+Expression `bodyStyle.item(0)`
+has side effect: false, expected: false
+Expression `bodyStyle.length`
+has side effect: false, expected: false
+Expression `namedNodeMap[0]`
+has side effect: false, expected: false
+Expression `namedNodeMap.item(0)`
+has side effect: false, expected: false
+Expression `namedNodeMap.length`
+has side effect: false, expected: false
+Expression `namedNodeMap.attr1`
+has side effect: false, expected: false
 

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/side-effects/evaluate-embedder-side-effect-free-methods.js

@@ -22,7 +22,14 @@
 
     var textNode = document.createTextNode('footext');
     var divNoAttrs = document.createElement('div');
+
+    var htmlCollection = document.getElementsByTagName('div');
+    var nodeList = document.getElementsByName('div-name');
+    var domTokenList = spanWithClass.classList;
+    var bodyStyle = document.body.style;
+    var namedNodeMap = div.attributes;
   `);
+  await dp.Runtime.evaluate({expression: `var $$result = $$('div')`, includeCommandLineAPI: true});
 
   // Sanity check: test that setters are not allowed on whitelisted methods.
   await checkHasSideEffect(`document.querySelector('div').x = "foo"`);
@@ -74,6 +81,19 @@
   // Window
   await checkHasNoSideEffect(`global_performance.now()`);
 
+  // Collection getters (e.g. HTMLCollection, NodeList)
+  var indexedCollections = [
+    'htmlCollection', 'nodeList', '$$result', 'domTokenList', 'bodyStyle', 'namedNodeMap'
+  ];
+  for (var collection of indexedCollections) {
+    await checkHasNoSideEffect(`${collection}[0]`);
+    await checkHasNoSideEffect(`${collection}.item(0)`);
+    await checkHasNoSideEffect(`${collection}.length`);
+  }
+
+  // Named getters (e.g. CSSStyleDeclaration)
+  await checkHasNoSideEffect(`namedNodeMap.attr1`);
+
   testRunner.completeTest();
 
 

third_party/blink/renderer/bindings/scripts/v8_interface.py

@@ -1400,6 +1400,7 @@ def property_getter(getter, cpp_arguments):
         return ''
 
     extended_attributes = getter.extended_attributes
+    has_no_side_effect = v8_utilities.has_extended_attribute_value(getter, 'Affects', 'Nothing')
     idl_type = getter.idl_type
     idl_type.add_includes_for_type(extended_attributes)
     is_call_with_script_state = v8_utilities.has_extended_attribute_value(getter, 'CallWith', 'ScriptState')
@@ -1421,6 +1422,7 @@ def property_getter(getter, cpp_arguments):
     return {
         'cpp_type': idl_type.cpp_type,
         'cpp_value': cpp_value,
+        'has_no_side_effect': has_no_side_effect,
         'is_call_with_script_state': is_call_with_script_state,
         'is_cross_origin': 'CrossOrigin' in extended_attributes,
         'is_custom':

third_party/blink/renderer/bindings/templates/interface.cpp.tmpl

@@ -910,7 +910,8 @@ for (const auto& attributeConfig : {{method.name}}OriginSafeAttributeConfigurati
        '%s::indexedPropertyDefinerCallback' % v8_class_or_partial
        if indexed_property_getter else 'nullptr' %}
 {% set property_handler_flags =
-       'v8::PropertyHandlerFlags::kNone' %}
+       'v8::PropertyHandlerFlags::kHasNoSideEffect'
+       if indexed_property_getter.has_no_side_effect else 'v8::PropertyHandlerFlags::kNone' %}
 v8::IndexedPropertyHandlerConfiguration indexedPropertyHandlerConfig(
     {{indexed_property_getter_callback}},
     {{indexed_property_setter_callback}},
@@ -946,6 +947,10 @@ v8::IndexedPropertyHandlerConfiguration indexedPropertyHandlerConfig(
 {% set property_handler_flags_list =
        property_handler_flags_list + ['int(v8::PropertyHandlerFlags::kNonMasking)'] %}
 {% endif %}
+{% if named_property_getter.has_no_side_effect %}
+{% set property_handler_flags_list =
+       property_handler_flags_list + ['int(v8::PropertyHandlerFlags::kHasNoSideEffect)'] %}
+{% endif %}
 {% set property_handler_flags =
        'static_cast<v8::PropertyHandlerFlags>(%s)' %
            ' | '.join(property_handler_flags_list) %}

third_party/blink/renderer/core/css/css_style_declaration.idl

@@ -24,8 +24,8 @@
     Exposed=Window
 ] interface CSSStyleDeclaration {
     [CEReactions, RaisesException=Setter, SetterCallWith=ExecutionContext] attribute DOMString cssText;
-    readonly attribute unsigned long length;
-    getter DOMString item(unsigned long index);
+    [Affects=Nothing] readonly attribute unsigned long length;
+    [Affects=Nothing] getter DOMString item(unsigned long index);
     DOMString getPropertyValue(DOMString property);
     DOMString getPropertyPriority(DOMString property);
     [CEReactions, RaisesException, CallWith=ExecutionContext] void setProperty(DOMString property, [TreatNullAs=EmptyString] DOMString value, [TreatNullAs=EmptyString] optional DOMString priority = "");

third_party/blink/renderer/core/dom/dom_token_list.idl

@@ -28,8 +28,8 @@
 [
     Exposed=Window
 ] interface DOMTokenList {
-    readonly attribute unsigned long length;
-    getter DOMString? item(unsigned long index);
+    [Affects=Nothing] readonly attribute unsigned long length;
+    [Affects=Nothing] getter DOMString? item(unsigned long index);
     boolean contains(DOMString token);
     [RaisesException, CEReactions, CustomElementCallbacks] void add(DOMString... tokens);
     [RaisesException, CEReactions, CustomElementCallbacks] void remove(DOMString... tokens);

third_party/blink/renderer/core/dom/named_node_map.idl

@@ -23,9 +23,9 @@
 [
     LegacyUnenumerableNamedProperties
 ] interface NamedNodeMap {
-    readonly attribute unsigned long length;
-    [MeasureAs=NamedNodeMapItem] getter Attr? item(unsigned long index);
-    [MeasureAs=NamedNodeMapGetNamedItem] getter Attr? getNamedItem(DOMString name);
+    [Affects=Nothing] readonly attribute unsigned long length;
+    [Affects=Nothing, MeasureAs=NamedNodeMapItem] getter Attr? item(unsigned long index);
+    [Affects=Nothing, MeasureAs=NamedNodeMapGetNamedItem] getter Attr? getNamedItem(DOMString name);
     [MeasureAs=NamedNodeMapGetNamedItemNS] Attr? getNamedItemNS(DOMString? namespaceURI, DOMString localName);
     [RaisesException, CEReactions, CustomElementCallbacks, MeasureAs=NamedNodeMapSetNamedItem] Attr? setNamedItem(Attr attr);
     [RaisesException, CEReactions, CustomElementCallbacks, MeasureAs=NamedNodeMapSetNamedItemNS] Attr? setNamedItemNS(Attr attr);

third_party/blink/renderer/core/dom/node_list.idl

@@ -21,7 +21,7 @@
 // https://dom.spec.whatwg.org/#interface-nodelist
 
 interface NodeList {
-    getter Node? item(unsigned long index);
-    readonly attribute unsigned long length;
+    [Affects=Nothing] getter Node? item(unsigned long index);
+    [Affects=Nothing] readonly attribute unsigned long length;
     iterable<Node>;
 };

third_party/blink/renderer/core/html/html_collection.idl

@@ -24,7 +24,7 @@
 [
     LegacyUnenumerableNamedProperties
 ] interface HTMLCollection {
-    readonly attribute unsigned long length;
-    getter Element? item(unsigned long index);
+    [Affects=Nothing] readonly attribute unsigned long length;
+    [Affects=Nothing] getter Element? item(unsigned long index);
     getter Element? namedItem(DOMString name);
 };