From 5fee819e24fb4de8ce9504d48323e76fd87d233a Mon Sep 17 00:00:00 2001 From: Kevin Marshall Date: Wed, 21 Oct 2020 00:50:25 +0000 Subject: [PATCH] [fuchsia] Add "web_engine_with_webui" package to CIPD. Defines a Fuchsia package, "web_engine_with_webui", which is a variation on the WebEngine package with WebUI static resources included. Adds web_engine_with_webui to CIPD recipe. Modifies web_engine_shell to support running packages other than "web_engine", so that "web_engine_with_webui" can be used. Prepares GPU integration test runner to use "web_engine_with_webui" (pending integration with Catapult). "web_engine" will have its resources removed in a followup CL. Bug: 1092729 Change-Id: I321c95b8f41902180e9edbe5e7404438956e1e91 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424514 Commit-Queue: Kevin Marshall Reviewed-by: Scott Violet Reviewed-by: David Dorwin Auto-Submit: Kevin Marshall Cr-Commit-Position: refs/heads/master@{#819179} --- content/test/BUILD.gn | 6 +- content/test/OWNERS | 3 + .../gpu/run_gpu_integration_test_fuchsia.py | 4 +- fuchsia/cipd/BUILD.gn | 2 + fuchsia/engine/BUILD.gn | 74 ++++++++++++------- fuchsia/engine/test/web_engine_shell.cc | 24 ++++-- fuchsia/engine/web_engine_main_delegate.cc | 24 ++++-- 7 files changed, 95 insertions(+), 42 deletions(-) diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn index 70670d0d61b6fe..33b3f3fb11061f 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn @@ -634,9 +634,13 @@ if (is_fuchsia) { data_deps = [ ":telemetry_gpu_integration_test_scripts_only", ":telemetry_gpu_integration_test_support", - "//fuchsia/engine:web_engine_runner", "//fuchsia/engine:web_engine_shell", + "//fuchsia/engine:web_engine_with_webui", ] + + # TODO(crbug.com/1092729): Remove once Catapult is migrated to use + # `web_engine_with_webui`. + data_deps += [ "//fuchsia/engine:web_engine" ] } } diff --git a/content/test/OWNERS b/content/test/OWNERS index a14569b743129b..d1693eb8a87023 100644 --- a/content/test/OWNERS +++ b/content/test/OWNERS @@ -24,6 +24,9 @@ per-file mock_badge_service.*=file://chrome/browser/badging/OWNERS # Trust Tokens-related files. per-file *trust_token*=file://services/network/trust_tokens/OWNERS +# Fuchsia related files. +per-file *fuchsia*=file://build/fuchsia/OWNERS + # Anyone can add rules to include new test files. per-file BUILD.gn=* diff --git a/content/test/gpu/run_gpu_integration_test_fuchsia.py b/content/test/gpu/run_gpu_integration_test_fuchsia.py index 6269808918e0db..0e87298a0db7a9 100755 --- a/content/test/gpu/run_gpu_integration_test_fuchsia.py +++ b/content/test/gpu/run_gpu_integration_test_fuchsia.py @@ -43,7 +43,9 @@ def main(): temp_log_file = True additional_target_args['system_log_file'] = args.system_log_file - package_names = ['web_engine', 'web_engine_shell'] + # TODO(crbug.com/1092729): Remove 'web_engine' when Catapult is migrated + # to use "web_engine_with_webui". + package_names = ['web_engine', 'web_engine_with_webui', 'web_engine_shell'] web_engine_dir = os.path.join(args.out_dir, 'gen', 'fuchsia', 'engine') gpu_script = [ os.path.join(path_util.GetChromiumSrcDir(), 'content', 'test', 'gpu', diff --git a/fuchsia/cipd/BUILD.gn b/fuchsia/cipd/BUILD.gn index a59fd83d727c5d..3e60eadcc73e9c 100644 --- a/fuchsia/cipd/BUILD.gn +++ b/fuchsia/cipd/BUILD.gn @@ -95,11 +95,13 @@ cipd_archive("webrunner") { deps = [ "//fuchsia/engine:web_engine", + "//fuchsia/engine:web_engine_with_webui", "//fuchsia/runners:web_runner_pkg", ] sources = [ "${root_gen_dir}/fuchsia/engine/web_engine/web_engine.far", + "${root_gen_dir}/fuchsia/engine/web_engine_with_webui/web_engine_with_webui.far", "${root_gen_dir}/fuchsia/runners/web_runner/web_runner.far", ] } diff --git a/fuchsia/engine/BUILD.gn b/fuchsia/engine/BUILD.gn index 3486d29d7c4684..a78c74410d9064 100644 --- a/fuchsia/engine/BUILD.gn +++ b/fuchsia/engine/BUILD.gn @@ -14,17 +14,6 @@ config("web_engine_implementation") { defines = [ "WEB_ENGINE_IMPLEMENTATION" ] } -declare_args() { - # If set, includes assets served under chrome://resources in web_engine.pak. - # They are required by some test-oriented diagnostic views like - # chrome://gpu. - # Including the resources will increase the application size, so this value - # should be left unset for release builds. - # TODO(crbug.com/1082420): change default to 'false' once all bots that run - # GPU integration tests set this flag to 'true'. - include_webui_resources = !is_official_build -} - mojom("mojom") { sources = [ "cast_streaming_session.mojom", @@ -56,10 +45,6 @@ repack("web_engine_pak") { "$root_gen_dir/ui/strings/ui_strings_en-US.pak", ] - if (include_webui_resources) { - sources += [ "$root_gen_dir/ui/resources/webui_resources.pak" ] - } - deps = [ "//components/resources:components_resources", "//components/strings", @@ -240,22 +225,49 @@ executable("web_engine_exe") { visibility = [ ":*" ] } -cr_fuchsia_package("web_engine") { - binary = ":web_engine_exe" - manifest = "context_provider.cmx" - component_name_override = "context_provider" +source_set("webui_resources") { + data = [ "$root_gen_dir/ui/resources/webui_resources.pak" ] + deps = [ "//ui/resources" ] +} - excluded_files = [ - "lib/libswiftshader_libEGL.so", - "lib/libswiftshader_libGLESv2.so", - ] +template("web_engine_package") { + cr_fuchsia_package(target_name) { + binary = ":web_engine_exe" + manifest = "context_provider.cmx" + component_name_override = "context_provider" + + if (invoker.include_webui_resources_pak) { + deps = [ ":webui_resources" ] + } + + excluded_files = [ + "lib/libswiftshader_libEGL.so", + "lib/libswiftshader_libGLESv2.so", + ] + } +} + +web_engine_package("web_engine") { + # TODO(crbug.com/2424514): Set to "false" once telemetry tests are migrated to + # use `web_engine_with_webui`. + include_webui_resources_pak = !is_official_build } -fuchsia_package_runner("web_engine_runner") { +fuchsia_package_runner("web_engine_installer") { package = ":web_engine" install_only = true } +# Build a WebEngine package that contains WebUI static resources. +web_engine_package("web_engine_with_webui") { + include_webui_resources_pak = true +} + +fuchsia_package_runner("web_engine_with_webui_installer") { + package = ":web_engine_with_webui" + install_only = true +} + source_set("browsertest_core") { testonly = true sources = [ @@ -385,10 +397,16 @@ fuchsia_package_runner("web_engine_shell") { testonly = true package = ":web_engine_shell_pkg" package_name_override = "web_engine_shell" - package_deps = [ [ - ":web_engine", - "web_engine", - ] ] + package_deps = [ + [ + ":web_engine", + "web_engine", + ], + [ + ":web_engine_with_webui", + "web_engine_with_webui", + ], + ] } executable("web_engine_shell_exec") { diff --git a/fuchsia/engine/test/web_engine_shell.cc b/fuchsia/engine/test/web_engine_shell.cc index a2d06f69a91684..f326b7bfc2cd6c 100644 --- a/fuchsia/engine/test/web_engine_shell.cc +++ b/fuchsia/engine/test/web_engine_shell.cc @@ -22,8 +22,8 @@ #include "base/message_loop/message_pump_type.h" #include "base/path_service.h" #include "base/run_loop.h" -#include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/stringprintf.h" #include "base/task/single_thread_task_executor.h" #include "base/values.h" #include "fuchsia/base/init_logging.h" @@ -37,12 +37,14 @@ constexpr char kRemoteDebuggingPortSwitch[] = "remote-debugging-port"; constexpr char kHeadlessSwitch[] = "headless"; constexpr char kEnableProtectedMediaIdentifier[] = "enable-protected-media-identifier"; +constexpr char kWebEnginePackageName[] = "web-engine-package-name"; void PrintUsage() { std::cerr << "Usage: " << base::CommandLine::ForCurrentProcess()->GetProgram().BaseName() << " [--" << kRemoteDebuggingPortSwitch << "] [--" - << kHeadlessSwitch << "] URL. [--] [--{extra_flag1}] " + << kHeadlessSwitch << "] [--" << kWebEnginePackageName + << "=name] URL. [--] [--{extra_flag1}] " << "[--{extra_flag2}]" << std::endl << "Setting " << kRemoteDebuggingPortSwitch << " to 0 will " << "automatically choose an available port." << std::endl @@ -80,24 +82,32 @@ GURL GetUrlFromArgs(const base::CommandLine::StringVector& args) { } fuchsia::web::ContextProviderPtr ConnectToContextProvider( + base::StringPiece web_engine_package_name_override, const base::CommandLine::StringVector& extra_command_line_arguments) { sys::ComponentContext* const component_context = base::ComponentContextForProcess(); // If there are no additional command-line arguments then use the // system instance of the ContextProvider. - if (extra_command_line_arguments.empty()) { + if (extra_command_line_arguments.empty() && + web_engine_package_name_override.empty()) { return component_context->svc()->Connect(); } + base::StringPiece web_engine_package_name = + web_engine_package_name_override.empty() + ? "web_engine" + : web_engine_package_name_override; + // Launch a private ContextProvider instance, with the desired command-line // arguments. fuchsia::sys::LauncherPtr launcher; component_context->svc()->Connect(launcher.NewRequest()); fuchsia::sys::LaunchInfo launch_info; - launch_info.url = - "fuchsia-pkg://fuchsia.com/web_engine#meta/context_provider.cmx"; + launch_info.url = base::StringPrintf( + "fuchsia-pkg://fuchsia.com/%s#meta/context_provider.cmx", + web_engine_package_name.data()); launch_info.arguments = extra_command_line_arguments; fidl::InterfaceHandle service_directory; launch_info.directory_request = service_directory.NewRequest().TakeChannel(); @@ -148,7 +158,9 @@ int main(int argc, char** argv) { additional_args.erase(additional_args.begin()); fuchsia::web::ContextProviderPtr web_context_provider = - ConnectToContextProvider(additional_args); + ConnectToContextProvider( + command_line->GetSwitchValueASCII(kWebEnginePackageName), + additional_args); // Set up the content directory fuchsia-pkg://shell-data/, which will host // the files stored under //fuchsia/engine/test/shell_data. diff --git a/fuchsia/engine/web_engine_main_delegate.cc b/fuchsia/engine/web_engine_main_delegate.cc index f3eccc76d49fde..2265ca9d3a8494 100644 --- a/fuchsia/engine/web_engine_main_delegate.cc +++ b/fuchsia/engine/web_engine_main_delegate.cc @@ -9,6 +9,7 @@ #include "base/base_paths.h" #include "base/base_paths_fuchsia.h" #include "base/command_line.h" +#include "base/files/file_util.h" #include "base/path_service.h" #include "content/public/common/content_switches.h" #include "fuchsia/base/init_logging.h" @@ -24,12 +25,23 @@ namespace { WebEngineMainDelegate* g_current_web_engine_main_delegate = nullptr; -void InitializeResourceBundle() { - base::FilePath pak_file; - bool result = base::PathService::Get(base::DIR_ASSETS, &pak_file); +void InitializeResources() { + constexpr char kWebEnginePakPath[] = "web_engine.pak"; + constexpr char kWebUiResourcesPakPath[] = "ui/resources/webui_resources.pak"; + + base::FilePath asset_root; + bool result = base::PathService::Get(base::DIR_ASSETS, &asset_root); DCHECK(result); - pak_file = pak_file.Append("web_engine.pak"); - ui::ResourceBundle::InitSharedInstanceWithPakPath(pak_file); + ui::ResourceBundle::InitSharedInstanceWithPakPath( + asset_root.Append(kWebEnginePakPath)); + + // Conditionally load WebUI resource PAK if visible from namespace. + base::FilePath webui_resources_path = + asset_root.Append(kWebUiResourcesPakPath); + if (base::PathExists(webui_resources_path)) { + ui::ResourceBundle::GetSharedInstance().AddDataPackFromPath( + webui_resources_path, ui::SCALE_FACTOR_NONE); + } } } // namespace @@ -64,7 +76,7 @@ bool WebEngineMainDelegate::BasicStartupComplete(int* exit_code) { } void WebEngineMainDelegate::PreSandboxStartup() { - InitializeResourceBundle(); + InitializeResources(); } int WebEngineMainDelegate::RunProcess(