Commit f71d954c authored by Erik Luo's avatar Erik Luo Committed by Commit Bot

Mark side-effect-free methods/attributes with simple return types

Adds [Affects=Nothing] to common IDL methods/attributes with return
types: short, long, double, int, boolean, DOMString, USVString.

Followups will whitelist ones that use DOMWrapper and update style
or layout.
List of some common attributes: https://goo.gl/qGd2i7

Bug: 829571
Change-Id: I4c335b7eeac449bd7454df7f829f393469d84387
Reviewed-on: https://chromium-review.googlesource.com/1013126Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551456}
parent f92d0146
Tests that evaluating V8-embedder callbacks allows side-effect-free attribute getters. Should not crash. Tests that evaluating V8-embedder callbacks allows side-effect-free attribute getters. Should not crash.
Checking: `div.isConnected` for no side effect
...@@ -4,9 +4,45 @@ ...@@ -4,9 +4,45 @@
await session.evaluate(` await session.evaluate(`
var div = document.createElement('div'); var div = document.createElement('div');
div.id = 'foo';
div.className = 'bar baz';
var textNode = document.createTextNode('footext');
div.appendChild(textNode);
`); `);
await checkHasNoSideEffect('div.isConnected'); // Sanity check: test that setters are not allowed on whitelisted accessors.
await checkHasSideEffect(`document.title = "foo"`);
// Document
await checkHasNoSideEffect(`document.domain`);
await checkHasNoSideEffect(`document.referrer`);
await checkHasNoSideEffect(`document.cookie`);
await checkHasNoSideEffect(`document.title`);
// Element
await checkHasNoSideEffect(`div.tagName`);
await checkHasNoSideEffect(`div.id`);
await checkHasNoSideEffect(`div.className`);
// Node
var testNodes = ['div', 'document', 'textNode'];
for (var node of testNodes) {
await checkHasNoSideEffect(`${node}.nodeType`);
await checkHasNoSideEffect(`${node}.nodeName`);
await checkHasNoSideEffect(`${node}.nodeValue`);
await checkHasNoSideEffect(`${node}.textContent`);
await checkHasNoSideEffect(`${node}.isConnected`);
}
// ParentNode
await checkHasNoSideEffect(`document.childElementCount`);
await checkHasNoSideEffect(`div.childElementCount`);
// Window
await checkHasNoSideEffect(`devicePixelRatio`);
await checkHasNoSideEffect(`screenX`);
await checkHasNoSideEffect(`screenY`);
testRunner.completeTest(); testRunner.completeTest();
...@@ -19,7 +55,6 @@ ...@@ -19,7 +55,6 @@
} }
async function checkExpression(expression, expectSideEffect) { async function checkExpression(expression, expectSideEffect) {
testRunner.log(`Checking: \`${expression}\` for ${expectSideEffect ? '' : 'no'} side effect`);
var response = await dp.Runtime.evaluate({expression, throwOnSideEffect: true}); var response = await dp.Runtime.evaluate({expression, throwOnSideEffect: true});
var hasSideEffect = false; var hasSideEffect = false;
var exceptionDetails = response.result.exceptionDetails; var exceptionDetails = response.result.exceptionDetails;
...@@ -27,7 +62,7 @@ ...@@ -27,7 +62,7 @@
exceptionDetails.exception.description.startsWith('EvalError: Possible side-effect in debug-evaluate')) exceptionDetails.exception.description.startsWith('EvalError: Possible side-effect in debug-evaluate'))
hasSideEffect = true; hasSideEffect = true;
if (hasSideEffect !== expectSideEffect) { if (hasSideEffect !== expectSideEffect) {
testRunner.log(`FAIL: hasSideEffect = ${hasSideEffect}, expectSideEffect = ${expectSideEffect}`); testRunner.log(`FAIL: "${expression}" hasSideEffect = ${hasSideEffect}, expectSideEffect = ${expectSideEffect}`);
testRunner.completeTest(); testRunner.completeTest();
return; return;
} }
......
Tests that evaluating V8-embedder callbacks allows side-effect-free methods. Should not crash. Tests that evaluating V8-embedder callbacks allows side-effect-free methods. Should not crash.
Checking: `global_performance.now()` for no side effect
...@@ -4,9 +4,34 @@ ...@@ -4,9 +4,34 @@
await session.evaluate(` await session.evaluate(`
var global_performance = window.performance; var global_performance = window.performance;
var div = document.createElement('div');
div.setAttribute('attr1', 'attr1-value');
div.setAttribute('attr2', 'attr2-value');
var textNode = document.createTextNode('footext');
var divNoAttrs = document.createElement('div');
`); `);
await checkHasNoSideEffect('global_performance.now()'); // Element
await checkHasNoSideEffect(`div.getAttributeNames()`);
await checkHasNoSideEffect(`divNoAttrs.getAttributeNames()`);
await checkHasNoSideEffect(`div.getAttribute()`);
await checkHasNoSideEffect(`div.getAttribute('attr1')`);
await checkHasNoSideEffect(`div.getAttribute({})`);
await checkHasNoSideEffect(`divNoAttrs.getAttribute('attr1')`);
await checkHasNoSideEffect(`div.hasAttribute()`);
await checkHasNoSideEffect(`div.hasAttribute('attr1')`);
await checkHasNoSideEffect(`div.hasAttribute({})`);
await checkHasNoSideEffect(`divNoAttrs.hasAttribute('attr1')`);
// Node
await checkHasNoSideEffect(`div.contains(textNode)`);
await checkHasNoSideEffect(`div.contains()`);
await checkHasNoSideEffect(`div.contains({})`);
await checkHasNoSideEffect(`textNode.contains(textNode)`);
// Window
await checkHasNoSideEffect(`global_performance.now()`);
testRunner.completeTest(); testRunner.completeTest();
...@@ -19,7 +44,6 @@ ...@@ -19,7 +44,6 @@
} }
async function checkExpression(expression, expectSideEffect) { async function checkExpression(expression, expectSideEffect) {
testRunner.log(`Checking: \`${expression}\` for ${expectSideEffect ? '' : 'no'} side effect`);
var response = await dp.Runtime.evaluate({expression, throwOnSideEffect: true}); var response = await dp.Runtime.evaluate({expression, throwOnSideEffect: true});
var hasSideEffect = false; var hasSideEffect = false;
var exceptionDetails = response.result.exceptionDetails; var exceptionDetails = response.result.exceptionDetails;
...@@ -27,7 +51,7 @@ ...@@ -27,7 +51,7 @@
exceptionDetails.exception.description.startsWith('EvalError: Possible side-effect in debug-evaluate')) exceptionDetails.exception.description.startsWith('EvalError: Possible side-effect in debug-evaluate'))
hasSideEffect = true; hasSideEffect = true;
if (hasSideEffect !== expectSideEffect) { if (hasSideEffect !== expectSideEffect) {
testRunner.log(`FAIL: hasSideEffect = ${hasSideEffect}, expectSideEffect = ${expectSideEffect}`); testRunner.log(`FAIL: ${expression} hasSideEffect = ${hasSideEffect}, expectSideEffect = ${expectSideEffect}`);
testRunner.completeTest(); testRunner.completeTest();
return; return;
} }
......
...@@ -93,9 +93,9 @@ typedef (HTMLScriptElement or SVGScriptElement) HTMLOrSVGScriptElement; ...@@ -93,9 +93,9 @@ typedef (HTMLScriptElement or SVGScriptElement) HTMLOrSVGScriptElement;
// resource metadata management // resource metadata management
[PutForwards=href, Unforgeable] readonly attribute Location? location; [PutForwards=href, Unforgeable] readonly attribute Location? location;
[RaisesException=Setter] attribute USVString domain; [Affects=Nothing, RaisesException=Setter] attribute USVString domain;
readonly attribute USVString referrer; [Affects=Nothing] readonly attribute USVString referrer;
[RaisesException, RuntimeCallStatsCounter=DocumentCookie] attribute DOMString cookie; [Affects=Nothing, RaisesException, RuntimeCallStatsCounter=DocumentCookie] attribute DOMString cookie;
readonly attribute DOMString lastModified; readonly attribute DOMString lastModified;
readonly attribute DocumentReadyState readyState; readonly attribute DocumentReadyState readyState;
...@@ -103,7 +103,7 @@ typedef (HTMLScriptElement or SVGScriptElement) HTMLOrSVGScriptElement; ...@@ -103,7 +103,7 @@ typedef (HTMLScriptElement or SVGScriptElement) HTMLOrSVGScriptElement;
// Named getter is implemented without IDL code generation for better // Named getter is implemented without IDL code generation for better
// performance. See LocalWindowProxy.cpp. // performance. See LocalWindowProxy.cpp.
// getter object (DOMString name); // getter object (DOMString name);
[CEReactions, CustomElementCallbacks] attribute DOMString title; [Affects=Nothing, CEReactions, CustomElementCallbacks] attribute DOMString title;
[CEReactions, CustomElementCallbacks] attribute DOMString dir; [CEReactions, CustomElementCallbacks] attribute DOMString dir;
[CEReactions, RaisesException=Setter, CustomElementCallbacks, PerWorldBindings] attribute HTMLElement? body; [CEReactions, RaisesException=Setter, CustomElementCallbacks, PerWorldBindings] attribute HTMLElement? body;
readonly attribute HTMLHeadElement? head; readonly attribute HTMLHeadElement? head;
......
...@@ -30,10 +30,10 @@ interface Element : Node { ...@@ -30,10 +30,10 @@ interface Element : Node {
readonly attribute DOMString? namespaceURI; readonly attribute DOMString? namespaceURI;
readonly attribute DOMString? prefix; readonly attribute DOMString? prefix;
readonly attribute DOMString localName; readonly attribute DOMString localName;
readonly attribute DOMString tagName; [Affects=Nothing] readonly attribute DOMString tagName;
[CEReactions, Reflect] attribute DOMString id; [Affects=Nothing, CEReactions, Reflect] attribute DOMString id;
[CEReactions, Reflect=class] attribute DOMString className; [Affects=Nothing, CEReactions, Reflect=class] attribute DOMString className;
[SameObject, PerWorldBindings, PutForwards=value] readonly attribute DOMTokenList classList; [SameObject, PerWorldBindings, PutForwards=value] readonly attribute DOMTokenList classList;
[Unscopable, CEReactions, Reflect] attribute DOMString slot; [Unscopable, CEReactions, Reflect] attribute DOMString slot;
...@@ -45,14 +45,14 @@ interface Element : Node { ...@@ -45,14 +45,14 @@ interface Element : Node {
boolean hasAttributes(); boolean hasAttributes();
[SameObject, PerWorldBindings, ImplementedAs=attributesForBindings] readonly attribute NamedNodeMap attributes; [SameObject, PerWorldBindings, ImplementedAs=attributesForBindings] readonly attribute NamedNodeMap attributes;
sequence<DOMString> getAttributeNames(); [Affects=Nothing] sequence<DOMString> getAttributeNames();
DOMString? getAttribute(DOMString name); [Affects=Nothing] DOMString? getAttribute(DOMString name);
DOMString? getAttributeNS(DOMString? namespaceURI, DOMString localName); DOMString? getAttributeNS(DOMString? namespaceURI, DOMString localName);
[RaisesException, CEReactions, CustomElementCallbacks] void setAttribute(DOMString name, DOMString value); [RaisesException, CEReactions, CustomElementCallbacks] void setAttribute(DOMString name, DOMString value);
[RaisesException, CEReactions, CustomElementCallbacks] void setAttributeNS(DOMString? namespaceURI, DOMString name, DOMString value); [RaisesException, CEReactions, CustomElementCallbacks] void setAttributeNS(DOMString? namespaceURI, DOMString name, DOMString value);
[CEReactions, CustomElementCallbacks] void removeAttribute(DOMString name); [CEReactions, CustomElementCallbacks] void removeAttribute(DOMString name);
[CEReactions, CustomElementCallbacks] void removeAttributeNS(DOMString? namespaceURI, DOMString localName); [CEReactions, CustomElementCallbacks] void removeAttributeNS(DOMString? namespaceURI, DOMString localName);
boolean hasAttribute(DOMString name); [Affects=Nothing] boolean hasAttribute(DOMString name);
boolean hasAttributeNS(DOMString? namespaceURI, DOMString localName); boolean hasAttributeNS(DOMString? namespaceURI, DOMString localName);
Attr? getAttributeNode(DOMString name); Attr? getAttributeNode(DOMString name);
......
...@@ -33,8 +33,8 @@ interface Node : EventTarget { ...@@ -33,8 +33,8 @@ interface Node : EventTarget {
const unsigned short DOCUMENT_TYPE_NODE = 10; const unsigned short DOCUMENT_TYPE_NODE = 10;
const unsigned short DOCUMENT_FRAGMENT_NODE = 11; const unsigned short DOCUMENT_FRAGMENT_NODE = 11;
const unsigned short NOTATION_NODE = 12; // historical const unsigned short NOTATION_NODE = 12; // historical
[ImplementedAs=getNodeType] readonly attribute unsigned short nodeType; [Affects=Nothing, ImplementedAs=getNodeType] readonly attribute unsigned short nodeType;
[RuntimeCallStatsCounter=NodeName] readonly attribute DOMString nodeName; [Affects=Nothing, RuntimeCallStatsCounter=NodeName] readonly attribute DOMString nodeName;
readonly attribute USVString baseURI; readonly attribute USVString baseURI;
...@@ -50,9 +50,9 @@ interface Node : EventTarget { ...@@ -50,9 +50,9 @@ interface Node : EventTarget {
[PerWorldBindings] readonly attribute Node? nextSibling; [PerWorldBindings] readonly attribute Node? nextSibling;
[MeasureAs=NodeGetRootNode] Node getRootNode(optional GetRootNodeOptions options); [MeasureAs=NodeGetRootNode] Node getRootNode(optional GetRootNodeOptions options);
[CEReactions, CustomElementCallbacks] attribute DOMString? nodeValue; [Affects=Nothing, CEReactions, CustomElementCallbacks] attribute DOMString? nodeValue;
[CEReactions, CustomElementCallbacks] attribute DOMString? textContent; [Affects=Nothing, CEReactions, CustomElementCallbacks] attribute DOMString? textContent;
[CEReactions, CustomElementCallbacks] void normalize(); [CEReactions, CustomElementCallbacks] void normalize();
[NewObject, DoNotTestNewObject, CEReactions, CustomElementCallbacks, RaisesException] Node cloneNode(optional boolean deep = false); [NewObject, DoNotTestNewObject, CEReactions, CustomElementCallbacks, RaisesException] Node cloneNode(optional boolean deep = false);
...@@ -66,7 +66,7 @@ interface Node : EventTarget { ...@@ -66,7 +66,7 @@ interface Node : EventTarget {
const unsigned short DOCUMENT_POSITION_CONTAINED_BY = 0x10; const unsigned short DOCUMENT_POSITION_CONTAINED_BY = 0x10;
const unsigned short DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC = 0x20; const unsigned short DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC = 0x20;
unsigned short compareDocumentPosition(Node other); unsigned short compareDocumentPosition(Node other);
boolean contains(Node? other); [Affects=Nothing] boolean contains(Node? other);
DOMString? lookupPrefix(DOMString? namespaceURI); DOMString? lookupPrefix(DOMString? namespaceURI);
DOMString? lookupNamespaceURI(DOMString? prefix); DOMString? lookupNamespaceURI(DOMString? prefix);
......
...@@ -37,7 +37,7 @@ ...@@ -37,7 +37,7 @@
[SameObject, PerWorldBindings] readonly attribute HTMLCollection children; [SameObject, PerWorldBindings] readonly attribute HTMLCollection children;
[PerWorldBindings] readonly attribute Element? firstElementChild; [PerWorldBindings] readonly attribute Element? firstElementChild;
[PerWorldBindings] readonly attribute Element? lastElementChild; [PerWorldBindings] readonly attribute Element? lastElementChild;
readonly attribute unsigned long childElementCount; [Affects=Nothing] readonly attribute unsigned long childElementCount;
[Unscopable, RaisesException, CEReactions, CustomElementCallbacks] void prepend((Node or DOMString)... nodes); [Unscopable, RaisesException, CEReactions, CustomElementCallbacks] void prepend((Node or DOMString)... nodes);
[Unscopable, RaisesException, CEReactions, CustomElementCallbacks] void append((Node or DOMString)... nodes); [Unscopable, RaisesException, CEReactions, CustomElementCallbacks] void append((Node or DOMString)... nodes);
......
...@@ -149,11 +149,11 @@ ...@@ -149,11 +149,11 @@
[RuntimeEnabled=VisualViewportAPI, Replaceable, SameObject] readonly attribute VisualViewport visualViewport; [RuntimeEnabled=VisualViewportAPI, Replaceable, SameObject] readonly attribute VisualViewport visualViewport;
// client // client
[Replaceable] readonly attribute long screenX; [Affects=Nothing, Replaceable] readonly attribute long screenX;
[Replaceable] readonly attribute long screenY; [Affects=Nothing, Replaceable] readonly attribute long screenY;
[Replaceable] readonly attribute long outerWidth; [Replaceable] readonly attribute long outerWidth;
[Replaceable] readonly attribute long outerHeight; [Replaceable] readonly attribute long outerHeight;
[Replaceable] readonly attribute double devicePixelRatio; [Affects=Nothing, Replaceable] readonly attribute double devicePixelRatio;
// Selection API // Selection API
// https://w3c.github.io/selection-api/#extensions-to-window-interface // https://w3c.github.io/selection-api/#extensions-to-window-interface
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment