Commit ac464a7c authored by Trent Apted's avatar Trent Apted Committed by Commit Bot

CrOS Files: Don't bake-in so much test code to background_scripts.js.

Instead, load it lazily the first time a testing extension connects
and wants to use the remote call APIs.

This avoids having to parse the testing code every time any of the
ui/file_manager apps start up (in release, or in tests). Note that
current tests (e.g. gallery) may start up a background page with this
test code up to *four times* for each test. After this change, only
the app under test will load the testing code.

There may still be some added latency. To help balance that, this
CL caches the result of RemoteCall.isStepByStepEnabled(). That makes
it consistent with the newly added function anyway.

This still distributes the testing code in release, which is not ideal.
Loading from a filesystem:// URL might avoid that in future. This should
probably also use an ES6 module.. Baby steps.

Bug: 903669
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I7acf6b55b10fa775d40bae48b81b0cfd1859df56
Reviewed-on: https://chromium-review.googlesource.com/c/1322340
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606742}
parent 2ef7eef6
...@@ -51,6 +51,7 @@ js_type_check("closure_compile_module") { ...@@ -51,6 +51,7 @@ js_type_check("closure_compile_module") {
":metadata_proxy", ":metadata_proxy",
":mock_volume_manager", ":mock_volume_manager",
":progress_center", ":progress_center",
":runtime_loaded_test_util",
":task_queue", ":task_queue",
":test_util_base", ":test_util_base",
":volume_info_impl", ":volume_info_impl",
...@@ -259,14 +260,19 @@ js_library("task_queue") { ...@@ -259,14 +260,19 @@ js_library("task_queue") {
} }
js_library("test_util_base") { js_library("test_util_base") {
}
js_library("runtime_loaded_test_util") {
# TODO(tapted): Move this target to //ui/file_manager/base. It is used in the # TODO(tapted): Move this target to //ui/file_manager/base. It is used in the
# background page of all |related_apps|, but accessed via extension messaging. # background page of all |related_apps|, but loaded at runtime by
# So it doesn't need to be visible as a closure dependency, except for the # :test_util_base via extension messaging, so doesn't need to be depended on
# "unpacked" test framework. # except by the closure compilation target. The exception is the "unpacked"
# test framework, which copies some testing functions into its test context.
visibility += [ "//ui/file_manager/file_manager/test/js:test_util" ] visibility += [ "//ui/file_manager/file_manager/test/js:test_util" ]
deps = [ deps = [
":app_windows", ":app_windows",
":test_util_base",
"../../../externs:webview_tag", "../../../externs:webview_tag",
"//ui/file_manager/base/js:error_counter", "//ui/file_manager/base/js:error_counter",
] ]
......
...@@ -175,6 +175,7 @@ ...@@ -175,6 +175,7 @@
"foreground/elements/files_toggle_ripple.js", "foreground/elements/files_toggle_ripple.js",
"foreground/elements/files_tooltip.html", "foreground/elements/files_tooltip.html",
"foreground/elements/files_tooltip.js", "foreground/elements/files_tooltip.js",
"background/js/runtime_loaded_test_util.js",
"background/js/background_common_scripts.js", "background/js/background_common_scripts.js",
"foreground/js/metadata/byte_reader.js", "foreground/js/metadata/byte_reader.js",
"foreground/js/metadata/exif_parser.js", "foreground/js/metadata/exif_parser.js",
......
...@@ -25,7 +25,7 @@ js_library("strings") { ...@@ -25,7 +25,7 @@ js_library("strings") {
js_library("test_util") { js_library("test_util") {
deps = [ deps = [
":chrome_file_manager_private_test_impl", ":chrome_file_manager_private_test_impl",
"../../background/js:test_util_base", "../../background/js:runtime_loaded_test_util",
"../../foreground/js:constants", "../../foreground/js:constants",
"//ui/webui/resources/js:webui_resource_test", "//ui/webui/resources/js:webui_resource_test",
] ]
......
...@@ -149,6 +149,13 @@ scripts += ['<script src="%s%s"></script>' % (ROOT, s) for s in [ ...@@ -149,6 +149,13 @@ scripts += ['<script src="%s%s"></script>' % (ROOT, s) for s in [
includes2scripts('foreground/js/main_scripts.js') includes2scripts('foreground/js/main_scripts.js')
includes2scripts('background/js/background_common_scripts.js') includes2scripts('background/js/background_common_scripts.js')
includes2scripts('background/js/background_scripts.js') includes2scripts('background/js/background_scripts.js')
# test_util_base.js in background_common_scripts.js loads this at runtime.
# However, test/js/test_util.js copies some functions from it into its own
# test context, so provide it here.
scripts += ['<script src="%s%s"></script>' %
(ROOT, 'background/js/runtime_loaded_test_util.js')]
main_html = replaceline(main_html, 'foreground/js/main_scripts.js', [ main_html = replaceline(main_html, 'foreground/js/main_scripts.js', [
('<link rel="import" href="%s../../../third_party/polymer/v1_0/' ('<link rel="import" href="%s../../../third_party/polymer/v1_0/'
'components-chromium/polymer/polymer.html">' % ROOT), 'components-chromium/polymer/polymer.html">' % ROOT),
...@@ -157,7 +164,6 @@ main_html = replaceline(main_html, 'foreground/js/main_scripts.js', [ ...@@ -157,7 +164,6 @@ main_html = replaceline(main_html, 'foreground/js/main_scripts.js', [
"<script>var FILE_MANAGER_ROOT = '%s';</script>" % ROOT, "<script>var FILE_MANAGER_ROOT = '%s';</script>" % ROOT,
] + scripts) ] + scripts)
# Get strings from grdp files. Remove any ph/ex elements before getting text. # Get strings from grdp files. Remove any ph/ex elements before getting text.
# Parse private_api_strings.cc to match the string name to the grdp message. # Parse private_api_strings.cc to match the string name to the grdp message.
strings = {} strings = {}
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
<!-- Common Scripts. --> <!-- Common Scripts. -->
<include name="IDR_FILE_MANAGER_BACKGROUND_COMMON_JS" file="file_manager/background/js/background_common_scripts.js" flattenhtml="true" type="BINDATA" /> <include name="IDR_FILE_MANAGER_BACKGROUND_COMMON_JS" file="file_manager/background/js/background_common_scripts.js" flattenhtml="true" type="BINDATA" />
<include name="IDR_FILE_MANAGER_BACKGROUND_RUNTIME_LOADED_TEST_UTIL_JS" file="file_manager/background/js/runtime_loaded_test_util.js" flattenhtml="true" type="BINDATA" />
<include name="IDR_FILE_MANAGER_ANALYTICS_JS" file="../webui/resources/js/analytics.js" flattenhtml="false" type="BINDATA" /> <include name="IDR_FILE_MANAGER_ANALYTICS_JS" file="../webui/resources/js/analytics.js" flattenhtml="false" type="BINDATA" />
<!-- Polymer elements --> <!-- Polymer elements -->
......
...@@ -25,16 +25,49 @@ function autoStep() { ...@@ -25,16 +25,49 @@ function autoStep() {
*/ */
function RemoteCall(extensionId) { function RemoteCall(extensionId) {
this.extensionId_ = extensionId; this.extensionId_ = extensionId;
this.testRuntimeLoaded_ = false;
/**
* Tristate holding the cached result of isStepByStepEnabled_().
* @type{?bool}
*/
this.cachedStepByStepEnabled_ = null;
} }
/** /**
* Checks whether step by step tests are enabled or not. * Checks whether step by step tests are enabled or not.
* @private
* @return {Promise<bool>} * @return {Promise<bool>}
*/ */
RemoteCall.isStepByStepEnabled = function() { RemoteCall.prototype.isStepByStepEnabled_ = function() {
return new Promise(function(fulfill) { if (this.cachedStepByStepEnabled_ != null)
return Promise.resolve(this.cachedStepByStepEnabled_);
return new Promise((fulfill) => {
chrome.commandLinePrivate.hasSwitch( chrome.commandLinePrivate.hasSwitch(
'enable-file-manager-step-by-step-tests', fulfill); 'enable-file-manager-step-by-step-tests', (/** bool */ result) => {
this.cachedStepByStepEnabled_ = result;
fulfill(result);
});
});
};
/**
* Asks the extension under test to load its testing functions.
* @private
* @return {Promise<bool>}
*/
RemoteCall.prototype.ensureLoaded_ = function() {
if (this.testRuntimeLoaded_)
return Promise.resolve(true);
return new Promise((fulfill) => {
chrome.runtime.sendMessage(
this.extensionId_, {enableTesting: true}, {}, (/** bool */ success) => {
chrome.test.assertTrue(success);
this.testRuntimeLoaded_ = success;
fulfill(success);
});
}); });
}; };
...@@ -50,46 +83,43 @@ RemoteCall.isStepByStepEnabled = function() { ...@@ -50,46 +83,43 @@ RemoteCall.isStepByStepEnabled = function() {
* @return {Promise} Promise to be fulfilled with the result of the remote * @return {Promise} Promise to be fulfilled with the result of the remote
* utility. * utility.
*/ */
RemoteCall.prototype.callRemoteTestUtil = RemoteCall.prototype.callRemoteTestUtil = function(
function(func, appId, args, opt_callback) { func, appId, args, opt_callback) {
return RemoteCall.isStepByStepEnabled().then(function(stepByStep) { return this.ensureLoaded_()
if (!stepByStep) .then(this.isStepByStepEnabled_.bind(this))
return false; .then((stepByStep) => {
return new Promise(function(onFulfilled) { if (!stepByStep)
console.info('Executing: ' + func + ' on ' + appId + ' with args: '); return false;
console.info(args); return new Promise((onFulfilled) => {
if (window.autostep !== true) { console.info('Executing: ' + func + ' on ' + appId + ' with args: ');
console.info('Type step() to continue...'); console.info(args);
window.step = function() { if (window.autostep !== true) {
window.step = null; console.info('Type step() to continue...');
onFulfilled(stepByStep); window.step = function() {
}; window.step = null;
} else { onFulfilled(stepByStep);
console.info('Auto calling step() ...'); };
onFulfilled(stepByStep); } else {
} console.info('Auto calling step() ...');
}); onFulfilled(stepByStep);
}).then(function(stepByStep) { }
return new Promise(function(onFulfilled) { });
chrome.runtime.sendMessage( })
this.extensionId_, .then((stepByStep) => {
{ return new Promise((onFulfilled) => {
func: func, chrome.runtime.sendMessage(
appId: appId, this.extensionId_, {func: func, appId: appId, args: args}, {},
args: args function(var_args) {
}, if (stepByStep) {
{}, console.info('Returned value:');
function(var_args) { console.info(JSON.stringify(var_args));
if (stepByStep) { }
console.info('Returned value:'); if (opt_callback)
console.info(JSON.stringify(var_args)); opt_callback.apply(null, arguments);
} onFulfilled(arguments[0]);
if (opt_callback) });
opt_callback.apply(null, arguments); });
onFulfilled(arguments[0]); });
});
}.bind(this));
}.bind(this));
}; };
/** /**
......
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