Bug 1447977 - Handle cyclic references in element prototypes. r=automatedtester a=test-only
authorAndreas Tolfsen <ato@sny.no>
Fri, 06 Jul 2018 20:08:37 +0100
changeset 478127 20c023fb254c10ad540825753b49ca75af96e0c6
parent 478126 43f6112e957876670aa24461d6d751a8de5b3cfb
child 478128 b8e6986512d3b080e9b7a07c62df6917459e3d5a
push id9539
push userarchaeopteryx@coole-files.de
push dateThu, 26 Jul 2018 08:07:25 +0000
treeherdermozilla-beta@20c023fb254c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersautomatedtester, test-only
bugs1447977
milestone62.0
Bug 1447977 - Handle cyclic references in element prototypes. r=automatedtester a=test-only JavaScript objects can be nested to any depth, and as such we must check that the value to be serialised contains a cyclic structure before attempting to marshaling it. We handle this correctly for collections and arbitrary objects by relying on JSON.stringify. For example with arrays: let arr = []; arr.push(arr); And for objects: let obj = {}; obj.reference = obj; However, members of the different element prototypes (HTMLElement, SVGElement, XULElement, et al.) may also contain cyclic references via user-defined own properties: let body = document.documentElement; body.reference = body; JSON.stringify enumerates an object's own properties, which means it picks up on body's "reference" property in the above example. Marionette needs to special case element prototypes because we want to marshal them as web elements. This patch replaces JSON.stringify with a custom function for testing if a value contains cyclic structures that special-cases elements. MozReview-Commit-ID: 1TQtHrjf401
testing/marionette/evaluate.js
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/webdriver/tests/execute_script/cyclic.py
--- a/testing/marionette/evaluate.js
+++ b/testing/marionette/evaluate.js
@@ -292,29 +292,74 @@ evaluate.toJSON = function(obj, seenEls)
     }
   }
   return rv;
 };
 
 /**
  * Tests if an arbitrary object is cyclic.
  *
- * @param {*} obj
+ * Element prototypes are by definition acyclic, even when they
+ * contain cyclic references.  This is because `evaluate.toJSON`
+ * ensures they are marshaled as web elements.
+ *
+ * @param {*} value
  *     Object to test for cyclical references.
  *
  * @return {boolean}
  *     True if object is cyclic, false otherwise.
  */
-evaluate.isCyclic = function(obj) {
-  try {
-    JSON.stringify(obj);
+evaluate.isCyclic = function(value, stack = []) {
+  let t = Object.prototype.toString.call(value);
+
+  // null
+  if (t == "[object Undefined]" || t == "[object Null]") {
+    return false;
+
+  // primitives
+  } else if (t == "[object Boolean]" ||
+      t == "[object Number]" ||
+      t == "[object String]") {
+    return false;
+
+  // HTMLElement, SVGElement, XULElement, et al.
+  } else if (element.isElement(value)) {
     return false;
-  } catch (e) {
+
+  // Array, NodeList, HTMLCollection, et al.
+  } else if (element.isCollection(value)) {
+    if (stack.includes(value)) {
+      return true;
+    }
+    stack.push(value);
+
+    for (let i = 0; i < value.length; i++) {
+      if (evaluate.isCyclic(value[i], stack)) {
+        return true;
+      }
+    }
+
+    stack.pop();
+    return false;
+  }
+
+  // arbitrary objects
+  if (stack.includes(value)) {
     return true;
   }
+  stack.push(value);
+
+  for (let prop in value) {
+    if (evaluate.isCyclic(value[prop], stack)) {
+      return true;
+    }
+  }
+
+  stack.pop();
+  return false;
 };
 
 /**
  * `Cu.isDeadWrapper` does not return true for a dead sandbox that
  * was assosciated with and extension popup.  This provides a way to
  * still test for a dead object.
  *
  * @param {Object} obj
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -523104,17 +523104,17 @@
    "ab3c3d205e59df800ba5b4217245b83685521c31",
    "reftest"
   ],
   "css/css-scoping/shadow-host-with-before-after.html": [
    "99af6e29e69b3131b59dbdc2b0eead52931123c2",
    "reftest"
   ],
   "css/css-scoping/shadow-link-rel-stylesheet-no-style-leak.html": [
-   "76a54dabd8bd09f7155ab0331e3d17d1a0cae243",
+   "a46be006762a16c2deb3d1d3a760e3c4e348668c",
    "reftest"
   ],
   "css/css-scoping/shadow-link-rel-stylesheet.html": [
    "07862ce7d2a954988bdbce882869a4c5f097089a",
    "reftest"
   ],
   "css/css-scoping/shadow-reassign-dynamic-001.html": [
    "11ed4da2e6ce88d8a2b98a8f1c814417ef7770dd",
@@ -548596,17 +548596,17 @@
    "cd3f0233cc0eaf9295e602ca25aef87fb68df851",
    "support"
   ],
   "css/mediaqueries/support/min-width-tables-001-iframe.html": [
    "29e7fb34c94e2e8411514d1e71d09aca2ddb642e",
    "support"
   ],
   "css/mediaqueries/test_media_queries.html": [
-   "a7d78b13e119f8cd1ffa8812a9af67e59280084d",
+   "cff3585932589f611a7101329d3b5b6ca27820aa",
    "testharness"
   ],
   "css/mediaqueries/viewport-script-dynamic-ref.html": [
    "7d55c513e2de39c9b362fc864233a3008ca6ced2",
    "support"
   ],
   "css/mediaqueries/viewport-script-dynamic.html": [
    "1c2ba1a9116942599804ed29553e85628afadb04",
@@ -559700,17 +559700,17 @@
    "d1661ab1734f7d1a252030aeac7e9842a7a4cb3b",
    "testharness"
   ],
   "custom-elements/Document-createElement-svg.svg": [
    "9af8f2dc7778feeea4fa8e557d7885b10d325dea",
    "testharness"
   ],
   "custom-elements/Document-createElement.html": [
-   "46e64c9f412fb04582f8ec287e08783ac83cb933",
+   "0edab8da0f16d5d2239ffb21f446c371fe4c76c3",
    "testharness"
   ],
   "custom-elements/Document-createElementNS.html": [
    "da90b2a1c13cf18fd5cade85dcae2dadef6243c9",
    "testharness"
   ],
   "custom-elements/HTMLElement-constructor.html": [
    "4dc04a8b026538bddee52586f2df50206abc9334",
@@ -577844,17 +577844,17 @@
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
   "html/resources/common.js": [
    "0f18ee2c61b99893cfe2a3d1ff549b170a8d715d",
    "support"
   ],
   "html/scripting/the-noscript-element/non-html-noscript.html": [
-   "c0c5453111f29e5a0206f988f4d127ec8ebc2f13",
+   "121760184777008c2ddeb598278216e40b34e367",
    "testharness"
   ],
   "html/semantics/.gitkeep": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
   "html/semantics/OWNERS": [
    "abd95839027a88741c4d351ff374d81b773c80fa",
@@ -609796,17 +609796,17 @@
    "9bea44e9ab6d8452aadc57d5e6d5a1eaa017ac78",
    "support"
   ],
   "service-workers/service-worker/resources/pass.txt": [
    "27d2303f215d7d1a8f12f0b80b9b56a2cdf6c9a7",
    "support"
   ],
   "service-workers/service-worker/resources/performance-timeline-worker.js": [
-   "fc275abc58d82c338ff369ba62994bd3d5609a67",
+   "c4c32d5b7ca352a3f18548928570a8a7339fa687",
    "support"
   ],
   "service-workers/service-worker/resources/postmessage-blob-url.js": [
    "728244e7f0b717aec29c3057bde7a6ba12768587",
    "support"
   ],
   "service-workers/service-worker/resources/postmessage-msgport-to-client-worker.js": [
    "11d46afb4cf17c8dcd9b49cda4e07e110a42a36d",
@@ -618860,17 +618860,17 @@
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
   "webdriver/tests/execute_script/collections.py": [
    "0ee4e340b38b6aa59043286755822460e76b2dbd",
    "wdspec"
   ],
   "webdriver/tests/execute_script/cyclic.py": [
-   "21bae43b3a6e966b8698b7c439b29a68029adc58",
+   "145a8a67226f31e0c1023aa0609947486be5c319",
    "wdspec"
   ],
   "webdriver/tests/execute_script/json_serialize_windowproxy.py": [
    "20db10d82ed2b28a22674fcdc37cac0323d33c95",
    "wdspec"
   ],
   "webdriver/tests/execute_script/user_prompts.py": [
    "4bd4e9089185505d8add4d5ebe9806498da42999",
--- a/testing/web-platform/tests/webdriver/tests/execute_script/cyclic.py
+++ b/testing/web-platform/tests/webdriver/tests/execute_script/cyclic.py
@@ -1,9 +1,10 @@
-from tests.support.asserts import assert_error
+from tests.support.asserts import assert_error, assert_same_element, assert_success
+from tests.support.inline import inline
 
 
 def execute_script(session, script, args=None):
     if args is None:
         args = []
     body = {"script": script, "args": args}
 
     return session.transport.send(
@@ -41,8 +42,35 @@ def test_array_in_object(session):
 
 def test_object_in_array(session):
     response = execute_script(session, """
         let obj = {};
         obj.reference = obj;
         return [obj];
         """)
     assert_error(response, "javascript error")
+
+
+def test_element_in_collection(session):
+    session.url = inline("<div></div>")
+    divs = session.find.css("div")
+
+    response = execute_script(session, """
+        let div = document.querySelector("div");
+        div.reference = div;
+        return [div];
+        """)
+    value = assert_success(response)
+    for expected, actual in zip(divs, value):
+        assert_same_element(session, expected, actual)
+
+
+def test_element_in_object(session):
+    session.url = inline("<div></div>")
+    div = session.find.css("div", all=False)
+
+    response = execute_script(session, """
+        let div = document.querySelector("div");
+        div.reference = div;
+        return {foo: div};
+        """)
+    value = assert_success(response)
+    assert_same_element(session, div, value["foo"])