Commit 2ba63532 authored by John Chen's avatar John Chen Committed by Commit Bot

Reland "[ChromeDriver] Remove unused shadow DOM object cache"

This is a reland of 4fe92e6c

Original change's description:
> [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}

Bug: chromedriver:1461 chromedriver:2794
Change-Id: Iedd6b7e43eb256d65bf71fec7e66454ceb0f776c
Reviewed-on: https://chromium-review.googlesource.com/c/1490855Reviewed-by: default avatarCaleb Rouleau <crouleau@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635876}
parent c7f0e69d
...@@ -361,7 +361,7 @@ Status WebViewImpl::CallFunction(const std::string& frame, ...@@ -361,7 +361,7 @@ Status WebViewImpl::CallFunction(const std::string& frame,
std::string w3c = w3c_compliant_ ? "true" : "false"; std::string w3c = w3c_compliant_ ? "true" : "false";
// TODO(zachconrad): Second null should be array of shadow host ids. // TODO(zachconrad): Second null should be array of shadow host ids.
std::string expression = base::StringPrintf( std::string expression = base::StringPrintf(
"(%s).apply(null, [null, %s, %s, %s])", "(%s).apply(null, [%s, %s, %s])",
kCallFunctionScript, kCallFunctionScript,
function.c_str(), function.c_str(),
json.c_str(), json.c_str(),
...@@ -1141,7 +1141,7 @@ Status GetNodeIdFromFunction(DevToolsClient* client, ...@@ -1141,7 +1141,7 @@ Status GetNodeIdFromFunction(DevToolsClient* client,
std::string w3c = w3c_compliant ? "true" : "false"; std::string w3c = w3c_compliant ? "true" : "false";
// TODO(zachconrad): Second null should be array of shadow host ids. // TODO(zachconrad): Second null should be array of shadow host ids.
std::string expression = base::StringPrintf( std::string expression = base::StringPrintf(
"(%s).apply(null, [null, %s, %s, %s, true])", "(%s).apply(null, [%s, %s, %s, true])",
kCallFunctionScript, kCallFunctionScript,
function.c_str(), function.c_str(),
json.c_str(), json.c_str(),
......
...@@ -232,6 +232,7 @@ function getNodeRootThroughAnyShadows(node) { ...@@ -232,6 +232,7 @@ function getNodeRootThroughAnyShadows(node) {
function getPageCache(opt_doc, opt_w3c) { function getPageCache(opt_doc, opt_w3c) {
var doc = opt_doc || document; var doc = opt_doc || document;
var w3c = opt_w3c || false; var w3c = opt_w3c || false;
// |key| is a long random string, unlikely to conflict with anything else.
var key = '$cdc_asdjflasutopfhvcZLmcfl_'; var key = '$cdc_asdjflasutopfhvcZLmcfl_';
if (w3c) { if (w3c) {
if (!(key in doc)) if (!(key in doc))
...@@ -323,8 +324,6 @@ function unwrap(value, cache) { ...@@ -323,8 +324,6 @@ function unwrap(value, cache) {
* between cached object reference IDs and actual JS objects. The cache will * between cached object reference IDs and actual JS objects. The cache will
* automatically be pruned each call to remove stale references. * 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 {function(...[*]) : *} func The function to invoke.
* @param {!Array<*>} args The array of arguments to supply to the function, * @param {!Array<*>} args The array of arguments to supply to the function,
* which will be unwrapped before invoking the function. * which will be unwrapped before invoking the function.
...@@ -336,7 +335,7 @@ function unwrap(value, cache) { ...@@ -336,7 +335,7 @@ function unwrap(value, cache) {
* unwrapped return was specified, this will be the function's pure return * unwrapped return was specified, this will be the function's pure return
* value. * value.
*/ */
function callFunction(shadowHostIds, func, args, w3c, opt_unwrappedReturn) { function callFunction(func, args, w3c, opt_unwrappedReturn) {
if (w3c) { if (w3c) {
w3cEnabled = true; w3cEnabled = true;
ELEMENT_KEY = 'element-6066-11e4-a52e-4f735466cecf'; ELEMENT_KEY = 'element-6066-11e4-a52e-4f735466cecf';
...@@ -344,15 +343,6 @@ function callFunction(shadowHostIds, func, args, w3c, opt_unwrappedReturn) { ...@@ -344,15 +343,6 @@ function callFunction(shadowHostIds, func, args, w3c, opt_unwrappedReturn) {
} }
var cache = getPageCache(null, w3cEnabled); var cache = getPageCache(null, w3cEnabled);
cache.clearStale(); 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) if (opt_unwrappedReturn)
return func.apply(null, unwrap(args, cache)); return func.apply(null, unwrap(args, cache));
......
...@@ -33,7 +33,7 @@ function testUUID() { ...@@ -33,7 +33,7 @@ function testUUID() {
function testCallFunctionNoArgs() { function testCallFunctionNoArgs() {
clearCache(); clearCache();
var result = callFunction(null, function() { return 1; }, []); var result = callFunction(function() { return 1; }, []);
assertEquals(0, result.status); assertEquals(0, result.status);
assertEquals(1, result.value); assertEquals(1, result.value);
} }
...@@ -41,12 +41,12 @@ function testCallFunctionNoArgs() { ...@@ -41,12 +41,12 @@ function testCallFunctionNoArgs() {
function testCallFunctionThrows() { function testCallFunctionThrows() {
clearCache(); 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(StatusCode.JAVA_SCRIPT_ERROR, result.status);
assertEquals('fake error', result.value); assertEquals('fake error', result.value);
result = callFunction(null, function() { result = callFunction(function() {
var e = new Error('fake error'); var e = new Error('fake error');
e.code = 77; e.code = 77;
e.message = 'CUSTOM'; e.message = 'CUSTOM';
...@@ -62,7 +62,7 @@ function testCallFunctionArgs() { ...@@ -62,7 +62,7 @@ function testCallFunctionArgs() {
function func(primitive, elem) { function func(primitive, elem) {
return [primitive, elem.querySelector('div')]; 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(0, result.status);
assertEquals(1, result.value[0]); assertEquals(1, result.value[0]);
var cache = getPageCache(); var cache = getPageCache();
...@@ -75,7 +75,7 @@ function testCallFunctionArgsUnwrappedReturn() { ...@@ -75,7 +75,7 @@ function testCallFunctionArgsUnwrappedReturn() {
function func(elem) { function func(elem) {
return elem.querySelector('div'); 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); assertEquals(document.querySelector('div'), result);
} }
...@@ -195,7 +195,7 @@ function testCallFunctionWithShadowHost() { ...@@ -195,7 +195,7 @@ function testCallFunctionWithShadowHost() {
} }
var wrappedHost = wrap(host); var wrappedHost = wrap(host);
var result = callFunction(null, func, [wrappedHost]); var result = callFunction(func, [wrappedHost]);
assertEquals(0, result.status); assertEquals(0, result.status);
assertEquals(wrappedHost['ELEMENT'], result.value['ELEMENT']); assertEquals(wrappedHost['ELEMENT'], result.value['ELEMENT']);
var cache = getPageCache(); var cache = getPageCache();
...@@ -204,7 +204,7 @@ function testCallFunctionWithShadowHost() { ...@@ -204,7 +204,7 @@ function testCallFunctionWithShadowHost() {
document.body.removeChild(host); document.body.removeChild(host);
} }
function DISABLED_testCallFunctionWithShadowRoot() { function testCallFunctionWithShadowRoot() {
clearCache(); clearCache();
// Set up something in the shadow DOM. // Set up something in the shadow DOM.
...@@ -218,20 +218,17 @@ function DISABLED_testCallFunctionWithShadowRoot() { ...@@ -218,20 +218,17 @@ function DISABLED_testCallFunctionWithShadowRoot() {
var wrappedHost = wrap(host); var wrappedHost = wrap(host);
var wrappedRoot = wrap(root); var wrappedRoot = wrap(root);
// Trying without setting the shadow_path should fail. // Should handle shadow root as an argument.
var result = callFunction(null, func, [wrappedRoot]); result = callFunction(func, [wrappedRoot]);
assertEquals(StatusCode.STALE_ELEMENT_REFERENCE, result.status);
// Should succeed with the shadow_path.
result = callFunction([wrappedHost['ELEMENT']], func, [wrappedRoot]);
assertEquals(0, result.status); assertEquals(0, result.status);
assertEquals(wrappedRoot['ELEMENT'], result.value['ELEMENT']); assertEquals(wrappedRoot['ELEMENT'], result.value['ELEMENT']);
var cache = getPageCache(root); var cache = getPageCache();
assertEquals(root, unwrap(result.value, cache)); assertEquals(root, unwrap(result.value, cache));
document.body.removeChild(host); document.body.removeChild(host);
} }
function DISABLED_testCacheWithShadowDomAttached() { function testCacheWithShadowDomAttached() {
clearCache(); clearCache();
var pageCache = getPageCache(); var pageCache = getPageCache();
...@@ -242,17 +239,8 @@ function DISABLED_testCacheWithShadowDomAttached() { ...@@ -242,17 +239,8 @@ function DISABLED_testCacheWithShadowDomAttached() {
// Test with attached element in shadow DOM. // Test with attached element in shadow DOM.
var wrappedDiv = wrap(shadowDiv); var wrappedDiv = wrap(shadowDiv);
// It should NOT be in the page cache. pageCache.clearStale();
try { var unwrappedDiv = unwrap(wrappedDiv, pageCache);
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);
assertEquals(shadowDiv, unwrappedDiv); assertEquals(shadowDiv, unwrappedDiv);
document.body.removeChild(host); document.body.removeChild(host);
...@@ -269,7 +257,7 @@ function testCacheWithShadowDomDetachedChild() { ...@@ -269,7 +257,7 @@ function testCacheWithShadowDomDetachedChild() {
// Test with detached element in shadow DOM. // Test with detached element in shadow DOM.
var wrappedDiv = wrap(shadowDiv); var wrappedDiv = wrap(shadowDiv);
root.removeChild(shadowDiv); root.removeChild(shadowDiv);
var rootCache = getPageCache(root); var rootCache = getPageCache();
rootCache.clearStale(); rootCache.clearStale();
try { try {
unwrap(wrappedDiv, rootCache); unwrap(wrappedDiv, rootCache);
......
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