[chromedriver] Added option to make element references W3C compliant.

chrome/test/chromedriver/js/call_function.js

Index: chrome/test/chromedriver/js/call_function.js
diff --git a/chrome/test/chromedriver/js/call_function.js b/chrome/test/chromedriver/js/call_function.js
index a5c9b1f985ac39c7b82ac16f57305c2ec75cabc7..7fce8d5b7809f7faf2b6e7d6a6a60bb5ef04481c 100644
--- a/chrome/test/chromedriver/js/call_function.js
+++ b/chrome/test/chromedriver/js/call_function.js
@@ -28,6 +28,13 @@ var NodeType = {
 var ELEMENT_KEY = 'ELEMENT';
 
 /**
+ * True if using W3C Element references.
+ * @const
+ * @type {boolean}
+ */
+var w3cEnabled = false;

[samuong, 2016/08/11 20:45:14]

if you're going to have this here, is there a need to add an argument to
getPageCache()?

although i think it might be slightly better to just pass it to wrap as an
argument...

+
+/**
  * True if shadow dom is enabled.
  * @const
  * @type {boolean}
@@ -35,6 +42,95 @@ var ELEMENT_KEY = 'ELEMENT';
 var SHADOW_DOM_ENABLED = typeof ShadowRoot === 'function';
 
 /**
+ * Generates a unique ID to identify an element.
+ * @void
+ * @return {string} Randomly generated ID.
+ */
+function generateUUID() {
+  var UUID = "";
+  for (var i = 0; i < 16; i++) {
+    if (i == 6 || i == 12 || i == 14 || i == 15) {
+      UUID += "00";
+    } else if (i == 7 || i == 13) {
+      UUID += "01";

[samuong, 2016/08/11 20:45:14]

If I'm reading https://tools.ietf.org/html/rfc4122 correctly, version 1 UUIDs
are generated using the MAC address and a timer. Since you're generating these
from random numbers, you should be setting the version (the 13th hex digit) to
4.

For the 17th hex digit, the most significant two bits should be 10, which means
that the hex representation for this octet should be one of [89ab].

Other octets should be generated randomly, not set to 00 or 01.

+    } else {
+      var rand = Math.round(Math.random() * 225).toString(16);

[samuong, 2016/08/11 20:45:14]

Let's use window.crypto.getRandomValues() instead of Math.random():

https://developer.mozilla.org/en-US/docs/Web/API/RandomSource/getRandomValues

Also, a UUID should always have 32 hex digits, but I don't always get the same
length each time I run this function:

generateUUID().replace(/-/g, '').length
32
generateUUID().replace(/-/g, '').length
31
generateUUID().replace(/-/g, '').length
32
generateUUID().replace(/-/g, '').length
32
generateUUID().replace(/-/g, '').length
31
generateUUID().replace(/-/g, '').length
30

This is because the toString() function will not always left-pad with 0s.

Please add a test that generates many UUIDs and checks that they are all valid.
It should look at the length of the UUID (including the length of each
individual field) and check that all the characters are hex (0-9a-f) except for
the hyphens, which should be in the right place.

+      UUID += rand;
+    }
+    if (i == 3 || i == 5 || i == 7 || i == 9) {
+      UUID += "-";
+    }
+  }
+  return UUID;
+};
+
+/**
+ * A cache which maps IDs <-> cached objects for the purpose of identifying
+ * a script object remotely. Uses UUIDs for identification.
+ * @constructor
+ */
+function CacheWithUUID() {
+  this.cache_ = {};
+}
+
+CacheWithUUID.prototype = {
+  /**
+   * Stores a given item in the cache and returns a unique UUID.
+   *
+   * @param {!Object} item The item to store in the cache.
+   * @return {number} The UUID for the cached item.
+   */
+  storeItem: function(item) {
+    for (var i in this.cache_) {
+      if (item == this.cache_[i])
+        return i;
+    }
+    var id = generateUUID();
+    this.cache_[id] = item;
+    return id;
+  },
+
+  /**
+   * Retrieves the cached object for the given ID.
+   *
+   * @param {number} id The ID for the cached item to retrieve.
+   * @return {!Object} The retrieved item.
+   */
+  retrieveItem: function(id) {
+    var item = this.cache_[id];
+    if (item)
+      return item;
+    var error = new Error('not in cache');
+    error.code = StatusCode.STALE_ELEMENT_REFERENCE;
+    error.message = 'element is not attached to the page document';
+    throw error;
+  },
+
+  /**
+   * Clears stale items from the cache.
+   */
+  clearStale: function() {
+    for (var id in this.cache_) {
+      var node = this.cache_[id];
+      if (!this.isNodeReachable_(node))
+        delete this.cache_[id];
+    }
+  },
+
+  /**
+    * @private
+    * @param {!Node} node The node to check.
+    * @return {boolean} If the nodes is reachable.
+    */
+  isNodeReachable_: function(node) {
+    var nodeRoot = getNodeRootThroughAnyShadows(node);
+    return (nodeRoot == document);
+  }
+
+
+};
+
+/**
  * A cache which maps IDs <-> cached objects for the purpose of identifying
  * a script object remotely.
  * @constructor
@@ -133,12 +229,19 @@ function getNodeRootThroughAnyShadows(node) {
  *     the current document.
  * @return {!Cache} The page's object cache.
  */
-function getPageCache(opt_doc) {
+function getPageCache(opt_doc, opt_w3c) {
   var doc = opt_doc || document;
+  var w3c = opt_w3c || false;

[samuong, 2016/08/11 20:45:14]

is there ever a case where we don't pass a value for opt_w3c?
maybe we should make it non-optional? (and move it to the first arg)

[roisinmcl, 2016/08/13 01:47:26]

Done.

   var key = '$cdc_asdjflasutopfhvcZLmcfl_';
-  if (!(key in doc))
-    doc[key] = new Cache();
-  return doc[key];
+  if (w3c) {
+    if (!(key in doc))
+      doc[key] = new CacheWithUUID();
+    return doc[key];
+  } else {
+    if (!(key in doc))
+      doc[key] = new Cache();
+    return doc[key];
+  }

[samuong, 2016/08/11 20:45:14]

if you make the "if (w3c)" check the inner condition, then you won't need to
duplicate "if (!(key in doc))" and "return doc[key];"

[roisinmcl, 2016/08/13 01:47:26]

Done.

 }
 
 /**
@@ -163,7 +266,7 @@ function wrap(value) {
         || (SHADOW_DOM_ENABLED && value instanceof ShadowRoot)) {
       var wrapped = {};
       var root = getNodeRootThroughAnyShadows(value);
-      wrapped[ELEMENT_KEY] = getPageCache(root).storeItem(value);
+      wrapped[ELEMENT_KEY] = getPageCache(root, w3cEnabled).storeItem(value);
       return wrapped;
     }
 
@@ -216,15 +319,20 @@ function unwrap(value, cache) {
  *     unwrapped return was specified, this will be the function's pure return
  *     value.
  */
-function callFunction(shadowHostIds, func, args, opt_unwrappedReturn) {
-  var cache = getPageCache();
+function callFunction(shadowHostIds, func, args, w3c, opt_unwrappedReturn) {
+  if (w3c) {
+    w3cEnabled = true;
+    ELEMENT_KEY = 'element-6066-11e4-a52e-4f735466cecf';
+
+  }
+  var cache = getPageCache(null, w3cEnabled);
   cache.clearStale();
   if (shadowHostIds && SHADOW_DOM_ENABLED) {
     for (var i = 0; i < shadowHostIds.length; i++) {
       var host = cache.retrieveItem(shadowHostIds[i]);
       // TODO(zachconrad): Use the olderShadowRoot API when available to check
       // all of the shadow roots.
-      cache = getPageCache(host.webkitShadowRoot);
+      cache = getPageCache(host.webkitShadowRoot, w3cEnabled);
       cache.clearStale();
     }
   }