From cc75dee3bad38eeafa083f0a8042e1145ccda4bd Mon Sep 17 00:00:00 2001 From: Yang Guo Date: Sat, 2 Nov 2019 10:05:22 +0000 Subject: [PATCH] Revert "Implement lazy loading of modules" This reverts commit 7c08ccfd54f32b8f2ccd5b6e7d771e5d4f5c4ffa. Reason for revert: Breaks browser_tests. Steps to reproduce: - Build Chromium's browser_tests target - Run `xvfb-run -s "-screen 0 1024x768x24" out/Default/browser_tests --gtest_filter=DevToolsExtensionTest.TestDevToolsExtensionAPI` - Observe this failure: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8897915672067501312/+/steps/browser_tests__with_patch_/0/logs/Deterministic_failure:_DevToolsExtensionTest.TestDevToolsExtensionAPI__status_FAILURE_/0 Original change's description: > Implement lazy loading of modules > > Any module that is not autostart and has `modules` specified in its `module.json` > will now have its entrypoint dynamically imported. Update the release build script > to output the `modules` array as well so that the Runtime can load it. > > Add additional entrypoints to `platform` and `dom_extension` to be consistent > with the naming patterns of all other modules > > Bug:1006759 > Change-Id: If6d10a13a62354079e3f8ee49bee4ecdcffa6758 > Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1893085 > Commit-Queue: Tim van der Lippe > Reviewed-by: Paul Lewis TBR=yangguo@chromium.org,aerotwist@chromium.org,tvanderlippe@chromium.org Change-Id: I22e8a341e841a99df0df9808a419cc070dd81bea No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 1006759 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1893087 Reviewed-by: Yang Guo Commit-Queue: Yang Guo --- BUILD.gn | 8 +-- front_end/Runtime.js | 65 ++++++--------------- front_end/dom_extension/dom_extension.js | 29 --------- front_end/dom_extension/module.json | 1 - front_end/platform/module.json | 1 - front_end/platform/platform.js | 29 --------- front_end/root.js | 6 +- scripts/build/build_release_applications.py | 1 - 8 files changed, 23 insertions(+), 117 deletions(-) delete mode 100644 front_end/dom_extension/dom_extension.js delete mode 100644 front_end/platform/platform.js diff --git a/BUILD.gn b/BUILD.gn index 1608826ee8..fd84896d23 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -924,11 +924,9 @@ if (external_devtools_frontend) { "front_end/host/InspectorFrontendHost.js", "front_end/host/InspectorFrontendHostAPI.js", "front_end/dom_extension/DOMExtension.js", - "front_end/dom_extension/dom_extension.js", "front_end/root.js", "front_end/Runtime.js", "front_end/platform/utilities.js", - "front_end/platform/platform.js", "front_end/ui/ARIAUtils.js", "front_end/ui/ZoomManager.js", "front_end/ui/XWidget.js", @@ -1325,11 +1323,9 @@ if (external_devtools_frontend) { "$resources_out_dir/host/InspectorFrontendHost.js", "$resources_out_dir/host/InspectorFrontendHostAPI.js", "$resources_out_dir/dom_extension/DOMExtension.js", - "$resources_out_dir/dom_extension/dom_extension.js", "$resources_out_dir/root.js", "$resources_out_dir/Runtime.js", "$resources_out_dir/platform/utilities.js", - "$resources_out_dir/platform/platform.js", "$resources_out_dir/ui/ui.js", "$resources_out_dir/common/common.js", "$resources_out_dir/ui/ZoomManager.js", @@ -1617,10 +1613,10 @@ if (external_devtools_frontend) { copy("copy_wasm_deps") { sources = [ - "front_end/sdk/wasm_source_map/pkg/wasm_source_map_bg.wasm", + "front_end/sdk/wasm_source_map/pkg/wasm_source_map_bg.wasm" ] outputs = [ - "$resources_out_dir/sdk/wasm_source_map/pkg/wasm_source_map_bg.wasm", + "$resources_out_dir/sdk/wasm_source_map/pkg/wasm_source_map_bg.wasm" ] } diff --git a/front_end/Runtime.js b/front_end/Runtime.js index eb460f5cac..c48107e252 100644 --- a/front_end/Runtime.js +++ b/front_end/Runtime.js @@ -683,11 +683,6 @@ class ModuleDescriptor { */ this.scripts; - /** - * @type {!Array.} - */ - this.modules; - /** * @type {string|undefined} */ @@ -729,21 +724,6 @@ class RuntimeExtensionDescriptor { } } -// Module namespaces. -// NOTE: Update scripts/build/special_case_namespaces.json if you add a special cased namespace. -const specialCases = { - 'sdk': 'SDK', - 'js_sdk': 'JSSDK', - 'browser_sdk': 'BrowserSDK', - 'ui': 'UI', - 'object_ui': 'ObjectUI', - 'javascript_metadata': 'JavaScriptMetadata', - 'perf_ui': 'PerfUI', - 'har_importer': 'HARImporter', - 'sdk_test_runner': 'SDKTestRunner', - 'cpu_profiler_test_runner': 'CPUProfilerTestRunner' -}; - /** * @unrestricted */ @@ -817,7 +797,6 @@ class Module { this._pendingLoadPromise = Promise.all(dependencyPromises) .then(this._loadResources.bind(this)) - .then(this._loadModules.bind(this)) .then(this._loadScripts.bind(this)) .then(() => this._loadedForTest = true); @@ -842,23 +821,6 @@ class Module { return Promise.all(promises).then(undefined); } - _loadModules() { - if (!this._descriptor.modules || !this._descriptor.modules.length) { - return Promise.resolve(); - } - - const namespace = this._computeNamespace(); - self[namespace] = self[namespace] || {}; - - // TODO(crbug.com/680046): We are in a worker and we dont support modules yet - if (typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope) { - return Promise.resolve(); - } - - // TODO(crbug.com/1011811): Remove eval when we use TypeScript which does support dynamic imports - return eval(`import('./${this._name}/${this._name}.js')`); - } - /** * @return {!Promise.} */ @@ -867,19 +829,28 @@ class Module { return Promise.resolve(); } - const namespace = this._computeNamespace(); + // Module namespaces. + // NOTE: Update scripts/build/special_case_namespaces.json if you add a special cased namespace. + // The namespace keyword confuses clang-format. + // clang-format off + const specialCases = { + 'sdk': 'SDK', + 'js_sdk': 'JSSDK', + 'browser_sdk': 'BrowserSDK', + 'ui': 'UI', + 'object_ui': 'ObjectUI', + 'javascript_metadata': 'JavaScriptMetadata', + 'perf_ui': 'PerfUI', + 'har_importer': 'HARImporter', + 'sdk_test_runner': 'SDKTestRunner', + 'cpu_profiler_test_runner': 'CPUProfilerTestRunner' + }; + const namespace = specialCases[this._name] || this._name.split('_').map(a => a.substring(0, 1).toUpperCase() + a.substring(1)).join(''); self[namespace] = self[namespace] || {}; + // clang-format on return Runtime._loadScriptsPromise(this._descriptor.scripts.map(this._modularizeURL, this), this._remoteBase()); } - /** - * @return {string} - */ - _computeNamespace() { - return specialCases[this._name] || - this._name.split('_').map(a => a.substring(0, 1).toUpperCase() + a.substring(1)).join(''); - } - /** * @param {string} resourceName */ diff --git a/front_end/dom_extension/dom_extension.js b/front_end/dom_extension/dom_extension.js deleted file mode 100644 index 6bad05557e..0000000000 --- a/front_end/dom_extension/dom_extension.js +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (C) 2019 Google Inc. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. Neither the name of Apple Computer, Inc. ("Apple") nor the names of - * its contributors may be used to endorse or promote products derived - * from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY - * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED - * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND - * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF - * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -import './DOMExtension.js'; diff --git a/front_end/dom_extension/module.json b/front_end/dom_extension/module.json index 8bf13b1d4a..3d7c22e005 100644 --- a/front_end/dom_extension/module.json +++ b/front_end/dom_extension/module.json @@ -4,7 +4,6 @@ ], "scripts": [], "modules": [ - "dom_extension.js", "DOMExtension.js" ] } diff --git a/front_end/platform/module.json b/front_end/platform/module.json index 9c6a9af511..592d1b14ed 100644 --- a/front_end/platform/module.json +++ b/front_end/platform/module.json @@ -3,7 +3,6 @@ ], "scripts": [], "modules": [ - "platform.js", "utilities.js" ] } diff --git a/front_end/platform/platform.js b/front_end/platform/platform.js deleted file mode 100644 index 71cfebfaac..0000000000 --- a/front_end/platform/platform.js +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (C) 2019 Google Inc. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. Neither the name of Apple Computer, Inc. ("Apple") nor the names of - * its contributors may be used to endorse or promote products derived - * from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY - * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED - * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND - * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF - * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -import './utilities.js'; diff --git a/front_end/root.js b/front_end/root.js index fb0452287d..d0e4750fb5 100644 --- a/front_end/root.js +++ b/front_end/root.js @@ -3,8 +3,8 @@ // found in the LICENSE file. import './Runtime.js'; -import './platform/platform.js'; -import './dom_extension/dom_extension.js'; +import './platform/utilities.js'; +import './dom_extension/DOMExtension.js'; import './common/common.js'; import './host/host.js'; import './protocol/protocol.js'; @@ -18,4 +18,4 @@ import './components/components.js'; import './persistence/persistence.js'; import './browser_sdk/browser_sdk.js'; import './extensions/extensions.js'; -import './console_counters/console_counters.js'; +import './console_counters/console_counters.js'; \ No newline at end of file diff --git a/scripts/build/build_release_applications.py b/scripts/build/build_release_applications.py index dd7f4edc83..2a1bedd5e2 100644 --- a/scripts/build/build_release_applications.py +++ b/scripts/build/build_release_applications.py @@ -170,7 +170,6 @@ class ReleaseBuilder(object): else: # Non-autostart modules are vulcanized. module['scripts'] = [name + '_module.js'] - module['modules'] = module.get('modules', []) # Resources are already baked into scripts. if resources is not None: del module['resources']