[ChromeDriver] Remove unused shadow DOM object cache

ChromeDriver uses object caches for use while calling JavaScript
functions. The data structure was designed to allow a separate cache for
each shadow DOM, but that feature was never used, and objects in shadow
DOMs have always been stored in the main page cache. This CL removes
unused code, and resolves differences between test expectancies and
actual code behavior. It prepares for future changes to make the object
cache standard compliant.

Bug: chromedriver:1461
Change-Id: I10fcd94a7fd2359cc43fe6bc7241d36647e9d744
Reviewed-on: https://chromium-review.googlesource.com/c/1485011
Commit-Queue: John Chen <johnchen@chromium.org>
Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635754}

chrome/test/chromedriver/chrome/web_view_impl.cc

@@ -361,7 +361,7 @@ Status WebViewImpl::CallFunction(const std::string& frame,
   std::string w3c = w3c_compliant_ ? "true" : "false";
   // TODO(zachconrad): Second null should be array of shadow host ids.
   std::string expression = base::StringPrintf(
-      "(%s).apply(null, [null, %s, %s, %s])",
+      "(%s).apply(null, [%s, %s, %s])",
       kCallFunctionScript,
       function.c_str(),
       json.c_str(),
@@ -1141,7 +1141,7 @@ Status GetNodeIdFromFunction(DevToolsClient* client,
   std::string w3c = w3c_compliant ? "true" : "false";
   // TODO(zachconrad): Second null should be array of shadow host ids.
   std::string expression = base::StringPrintf(
-      "(%s).apply(null, [null, %s, %s, %s, true])",
+      "(%s).apply(null, [%s, %s, %s, true])",
       kCallFunctionScript,
       function.c_str(),
       json.c_str(),

chrome/test/chromedriver/js/call_function.js

@@ -229,18 +229,18 @@ function getNodeRootThroughAnyShadows(node) {
  *     the current document.
  * @return {!Cache} The page's object cache.
  */
-function getPageCache(opt_doc, opt_w3c) {
-  var doc = opt_doc || document;
+function getPageCache(opt_w3c) {
   var w3c = opt_w3c || false;
+  // |key| is a long random string, unlikely to conflict with anything else.
   var key = '$cdc_asdjflasutopfhvcZLmcfl_';
   if (w3c) {
-    if (!(key in doc))
-      doc[key] = new CacheWithUUID();
-    return doc[key];
+    if (!(key in document))
+      document[key] = new CacheWithUUID();
+    return document[key];
   } else {
-    if (!(key in doc))
-      doc[key] = new Cache();
-    return doc[key];
+    if (!(key in document))
+      document[key] = new Cache();
+    return document[key];
   }
 }
 
@@ -267,8 +267,7 @@ function wrap(value) {
     if (nodeType == NodeType.ELEMENT || nodeType == NodeType.DOCUMENT
         || (SHADOW_DOM_ENABLED && value instanceof ShadowRoot)) {
       var wrapped = {};
-      var root = getNodeRootThroughAnyShadows(value);
-      wrapped[ELEMENT_KEY] = getPageCache(root, w3cEnabled).storeItem(value);
+      wrapped[ELEMENT_KEY] = getPageCache(w3cEnabled).storeItem(value);
       return wrapped;
     }
 
@@ -323,8 +322,6 @@ function unwrap(value, cache) {
  * between cached object reference IDs and actual JS objects. The cache will
  * automatically be pruned each call to remove stale references.
  *
- * @param  {Array<string>} shadowHostIds The host ids of the nested shadow
- *     DOMs the function should be executed in the context of.
  * @param {function(...[*]) : *} func The function to invoke.
  * @param {!Array<*>} args The array of arguments to supply to the function,
  *     which will be unwrapped before invoking the function.
@@ -336,23 +333,14 @@ function unwrap(value, cache) {
  *     unwrapped return was specified, this will be the function's pure return
  *     value.
  */
-function callFunction(shadowHostIds, func, args, w3c, opt_unwrappedReturn) {
+function callFunction(func, args, w3c, opt_unwrappedReturn) {
   if (w3c) {
     w3cEnabled = true;
     ELEMENT_KEY = 'element-6066-11e4-a52e-4f735466cecf';
 
   }
-  var cache = getPageCache(null, w3cEnabled);
+  var cache = getPageCache(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, w3cEnabled);
-      cache.clearStale();
-    }
-  }
 
   if (opt_unwrappedReturn)
     return func.apply(null, unwrap(args, cache));

chrome/test/chromedriver/js/call_function_test.html

@@ -33,7 +33,7 @@ function testUUID() {
 function testCallFunctionNoArgs() {
   clearCache();
 
-  var result = callFunction(null, function() { return 1; }, []);
+  var result = callFunction(function() { return 1; }, []);
   assertEquals(0, result.status);
   assertEquals(1, result.value);
 }
@@ -41,12 +41,12 @@ function testCallFunctionNoArgs() {
 function testCallFunctionThrows() {
   clearCache();
 
-  var result = callFunction(null, function() { throw new Error('fake error'); },
+  var result = callFunction(function() { throw new Error('fake error'); },
       []);
   assertEquals(StatusCode.JAVA_SCRIPT_ERROR, result.status);
   assertEquals('fake error', result.value);
 
-  result = callFunction(null, function() {
+  result = callFunction(function() {
     var e = new Error('fake error');
     e.code = 77;
     e.message = 'CUSTOM';
@@ -62,7 +62,7 @@ function testCallFunctionArgs() {
   function func(primitive, elem) {
     return [primitive, elem.querySelector('div')];
   }
-  var result = callFunction(null, func, [1, wrap(document)]);
+  var result = callFunction(func, [1, wrap(document)]);
   assertEquals(0, result.status);
   assertEquals(1, result.value[0]);
   var cache = getPageCache();
@@ -75,7 +75,7 @@ function testCallFunctionArgsUnwrappedReturn() {
   function func(elem) {
     return elem.querySelector('div');
   }
-  var result = callFunction(null, func, [wrap(document)], false, true);
+  var result = callFunction(func, [wrap(document)], false, true);
   assertEquals(document.querySelector('div'), result);
 }
 
@@ -195,7 +195,7 @@ function testCallFunctionWithShadowHost() {
   }
   var wrappedHost = wrap(host);
   
-  var result = callFunction(null, func, [wrappedHost]);
+  var result = callFunction(func, [wrappedHost]);
   assertEquals(0, result.status);
   assertEquals(wrappedHost['ELEMENT'], result.value['ELEMENT']);
   var cache = getPageCache();
@@ -204,7 +204,7 @@ function testCallFunctionWithShadowHost() {
   document.body.removeChild(host);
 }
 
-function DISABLED_testCallFunctionWithShadowRoot() {
+function testCallFunctionWithShadowRoot() {
   clearCache();
 
   // Set up something in the shadow DOM.
@@ -218,20 +218,17 @@ function DISABLED_testCallFunctionWithShadowRoot() {
   var wrappedHost = wrap(host);
   var wrappedRoot = wrap(root);
   
-  // Trying without setting the shadow_path should fail.
-  var result = callFunction(null, func, [wrappedRoot]);
-  assertEquals(StatusCode.STALE_ELEMENT_REFERENCE, result.status);
-  // Should succeed with the shadow_path.
-  result = callFunction([wrappedHost['ELEMENT']], func, [wrappedRoot]);
+  // Should handle shadow root as an argument.
+  result = callFunction(func, [wrappedRoot]);
   assertEquals(0, result.status);
   assertEquals(wrappedRoot['ELEMENT'], result.value['ELEMENT']);
-  var cache = getPageCache(root);
+  var cache = getPageCache();
   assertEquals(root, unwrap(result.value, cache));
   
   document.body.removeChild(host);
 }
 
-function DISABLED_testCacheWithShadowDomAttached() {
+function testCacheWithShadowDomAttached() {
   clearCache();
   var pageCache = getPageCache();
   
@@ -242,17 +239,8 @@ function DISABLED_testCacheWithShadowDomAttached() {
   
   // Test with attached element in shadow DOM.
   var wrappedDiv = wrap(shadowDiv);
-  // It should NOT be in the page cache.
-  try {
-    unwrap(wrappedDiv, pageCache);
-    assert(false);
-  } catch (e) {
-    assertEquals(StatusCode.STALE_ELEMENT_REFERENCE, e.code);
-  }
-  // It should be in the shadow root cache.
-  var rootCache = getPageCache(root);
-  rootCache.clearStale();
-  var unwrappedDiv = unwrap(wrappedDiv, rootCache);
+  pageCache.clearStale();
+  var unwrappedDiv = unwrap(wrappedDiv, pageCache);
   assertEquals(shadowDiv, unwrappedDiv);
   
   document.body.removeChild(host);
@@ -269,7 +257,7 @@ function testCacheWithShadowDomDetachedChild() {
   // Test with detached element in shadow DOM.
   var wrappedDiv = wrap(shadowDiv);
   root.removeChild(shadowDiv);
-  var rootCache = getPageCache(root);
+  var rootCache = getPageCache();
   rootCache.clearStale();
   try {
     unwrap(wrappedDiv, rootCache);