Skip to content

Commit

Permalink
Revert "[fuchsia] Migrate web_engine_shell to launch web_instance.cm"
Browse files Browse the repository at this point in the history 
This reverts commit 986da25.

Reason for revert: Causing failures in WES on EME video performance tests.

Original change's description:
> [fuchsia] Migrate web_engine_shell to launch web_instance.cm
>
> In so doing:
>
> - A relauncher is introduced to launch a new component between
>   test_manager and web_engine_shell when WebInstanceHost is used, as
>   this is required for it to route capabilities to WebEngine.
> - WebInstanceHost now supports the test-only --with-webui switch to run
>   a WebEngine with access to WebUI resources.
> - WebInstanceHost now destroys any active children upon destruction
>   rather than requiring that the consumer wait for them to exit.
>
> Bug: 1280703
> Change-Id: I8481b854542524f7eef69d66059ff1767ba60f93
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4134846
> Reviewed-by: Wez <wez@chromium.org>
> Auto-Submit: Greg Thompson <grt@chromium.org>
> Commit-Queue: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1095363}

Bug: 1280703, b/266732759
Change-Id: Iedea48b1424b558aa7a3d7f6d6edcfdfa41fa3d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195534
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Auto-Submit: Rohan Pavone <rohpavone@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1098257}
  • Loading branch information
Rohan Pavone authored and Chromium LUCI CQ committed Jan 28, 2023
1 parent 4330e7b commit 1328dca
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 298 deletions.
18 changes: 1 addition & 17 deletions fuchsia_web/shell/BUILD.gn
View file
Expand Up @@ -56,16 +56,6 @@ fuchsia_component("web_engine_shell_component") {
data_deps = [ ":web_engine_shell_exec" ]
}

# WebInstanceHost needs to serve capabilities to web_instance. Since the
# primary web_engine_shell component is a test component, it is not able to do
# this directly. Instead, it launches this here component to start instances via
# WebInstanceHost.
fuchsia_component("web_engine_shell_for_web_instance_host_component") {
testonly = true
manifest = "web_engine_shell_for_web_instance_host.cml"
data_deps = [ ":web_engine_shell_exec" ]
}

fuchsia_component("web_engine_shell_component_v1") {
testonly = true
manifest = "web_engine_shell.cmx"
Expand All @@ -80,7 +70,6 @@ fuchsia_package("web_engine_shell_pkg") {
deps = [
":web_engine_shell_component",
":web_engine_shell_component_v1",
":web_engine_shell_for_web_instance_host_component",
]
}

Expand Down Expand Up @@ -112,13 +101,8 @@ executable("web_engine_shell_exec") {
"//base",
"//components/fuchsia_component_support:annotations_manager",
"//fuchsia_web/common",
"//fuchsia_web/webinstance_host:webinstance_host",
"//third_party/fuchsia-sdk/sdk/fidl/fuchsia.component",
"//third_party/fuchsia-sdk/sdk/fidl/fuchsia.component.decl",
"//third_party/fuchsia-sdk/sdk/fidl/fuchsia.web",
"//fuchsia_web/webinstance_host:webinstance_host_v1",
"//third_party/fuchsia-sdk/sdk/pkg/fdio",
"//third_party/fuchsia-sdk/sdk/pkg/fidl_cpp",
"//third_party/fuchsia-sdk/sdk/pkg/sys_component_cpp_testing",
"//third_party/fuchsia-sdk/sdk/pkg/sys_cpp",
"//third_party/widevine/cdm:buildflags",
"//url",
Expand Down
103 changes: 14 additions & 89 deletions fuchsia_web/shell/web_engine_shell.cc
View file
Expand Up @@ -2,21 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <fuchsia/component/cpp/fidl.h>
#include <fuchsia/element/cpp/fidl.h>
#include <fuchsia/sys/cpp/fidl.h>
#include <fuchsia/web/cpp/fidl.h>
#include <lib/fdio/directory.h>
#include <lib/fidl/cpp/interface_ptr.h>
#include <lib/sys/component/cpp/testing/realm_builder.h>
#include <lib/sys/cpp/component_context.h>
#include <lib/sys/cpp/service_directory.h>

#include <lib/vfs/cpp/pseudo_file.h>
#include <iostream>
#include <utility>

#include "base/base_paths.h"
#include "base/check.h"
#include "base/command_line.h"
#include "base/files/file_util.h"
#include "base/fuchsia/file_utils.h"
Expand All @@ -26,7 +21,6 @@
#include "base/logging.h"
#include "base/message_loop/message_pump_type.h"
#include "base/path_service.h"
#include "base/ranges/algorithm.h"
#include "base/run_loop.h"
#include "base/strings/stringprintf.h"
#include "base/task/single_thread_task_executor.h"
Expand All @@ -36,7 +30,7 @@
#include "fuchsia_web/common/init_logging.h"
#include "fuchsia_web/shell/present_frame.h"
#include "fuchsia_web/shell/remote_debugging_port.h"
#include "fuchsia_web/webinstance_host/web_instance_host.h"
#include "fuchsia_web/webinstance_host/web_instance_host_v1.h"
#include "third_party/widevine/cdm/buildflags.h"
#include "url/gurl.h"

Expand All @@ -48,7 +42,6 @@ constexpr char kHeadlessSwitch[] = "headless";
constexpr char kEnableProtectedMediaIdentifier[] =
"enable-protected-media-identifier";
constexpr char kWebEnginePackageName[] = "web-engine-package-name";
constexpr char kFromLauncher[] = "from-launcher";
constexpr char kUseWebInstance[] = "use-web-instance";
constexpr char kEnableWebInstanceTmp[] = "enable-web-instance-tmp";

Expand Down Expand Up @@ -118,87 +111,17 @@ fuchsia::web::ContextProviderPtr ConnectToContextProvider(
return web_engine_service_dir.Connect<fuchsia::web::ContextProvider>();
}

// Appends the arguments of `command_line` (ignoring the program name at
// position zero) to the command line for the realm.
void AppendCommandLineArguments(component_testing::RealmBuilder& realm_builder,
const base::CommandLine& command_line) {
auto decl = realm_builder.GetRealmDecl();

// Find the "args" list in the program declaration.
fuchsia::data::DictionaryEntry* args_entry = nullptr;
auto* entries = decl.mutable_program()->mutable_info()->mutable_entries();
for (auto& entry : *entries) {
if (entry.key == "args") {
DCHECK(entry.value->is_str_vec());
args_entry = &entry;
break;
}
}
if (!args_entry) {
// Create a new "args" list and insert it at the proper location in the
// program's entries.
auto lower_bound = base::ranges::lower_bound(
*entries, "args", /*comp=*/{},
[](const fuchsia::data::DictionaryEntry& entry) { return entry.key; });
auto it = entries->emplace(lower_bound);
it->key = "args";
it->value = fuchsia::data::DictionaryValue::New();
it->value->set_str_vec({});
args_entry = &*it;
}

// Append all args following the program name in `command_line` to the
// program's args.
args_entry->value->str_vec().insert(args_entry->value->str_vec().end(),
command_line.argv().begin() + 1,
command_line.argv().end());
realm_builder.ReplaceRealmDecl(std::move(decl));
}

// web_engine_shell needs to provide capabilities to children it launches (via
// WebInstanceHost, for example). Test components are not able to do this, so
// use RealmBuilder to relaunch web_engine_shell via
// `web_engine_shell_for_web_instance_host_component` (which includes
// `--from-launcher` on its command line) with the contents of this process's
// command line.
int RelaunchForWebInstanceHost(const base::CommandLine& command_line) {
auto realm_builder = component_testing::RealmBuilder::CreateFromRelativeUrl(
"#meta/web_engine_shell_for_web_instance_host.cm");
AppendCommandLineArguments(realm_builder, command_line);

auto realm = realm_builder.Build();

fuchsia::component::BinderPtr binder_proxy =
realm.component().Connect<fuchsia::component::Binder>();

// Wait for binder_proxy to be closed.
base::RunLoop run_loop;
binder_proxy.set_error_handler(
[quit_closure = run_loop.QuitClosure()](zx_status_t status) {
std::move(quit_closure).Run();
});
run_loop.Run();

// Nothing depends on the process exit code of web_engine_shell today, so
// simply return success in all cases.
return 0;
}

} // namespace

int main(int argc, char** argv) {
base::CommandLine::Init(argc, argv);
base::SingleThreadTaskExecutor executor(base::MessagePumpType::IO);

// Parse the command line arguments and set up logging.
CHECK(base::CommandLine::Init(argc, argv));
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
CHECK(InitLoggingFromCommandLine(*command_line));

base::SingleThreadTaskExecutor executor(base::MessagePumpType::IO);

const bool is_run_from_launcher = command_line->HasSwitch(kFromLauncher);
const bool use_context_provider = !command_line->HasSwitch(kUseWebInstance);
if (!is_run_from_launcher && !use_context_provider) {
return RelaunchForWebInstanceHost(*command_line);
}
CHECK(InitLoggingFromCommandLineDefaultingToStderrForTest( // IN-TEST
command_line));

absl::optional<uint16_t> remote_debugging_port =
GetRemoteDebuggingPort(*command_line);
Expand All @@ -210,6 +133,7 @@ int main(int argc, char** argv) {
const bool is_headless = command_line->HasSwitch(kHeadlessSwitch);
const bool enable_protected_media_identifier_access =
command_line->HasSwitch(kEnableProtectedMediaIdentifier);
const bool use_context_provider = !command_line->HasSwitch(kUseWebInstance);
const bool enable_web_instance_tmp =
command_line->HasSwitch(kEnableWebInstanceTmp);

Expand Down Expand Up @@ -281,9 +205,12 @@ int main(int argc, char** argv) {

base::RunLoop run_loop;

fuchsia::web::ContextProviderPtr web_context_provider;
std::unique_ptr<WebInstanceHost> web_instance_host;
// Create the browser |context|.
fuchsia::web::ContextPtr context;

// Keep alive in run_loop scope.
fuchsia::web::ContextProviderPtr web_context_provider;
std::unique_ptr<WebInstanceHostV1> web_instance_host;
fuchsia::io::DirectoryHandle tmp_directory;

if (use_context_provider) {
Expand All @@ -293,7 +220,7 @@ int main(int argc, char** argv) {
web_context_provider->Create(std::move(create_context_params),
context.NewRequest());
} else {
web_instance_host = std::make_unique<WebInstanceHost>();
web_instance_host = std::make_unique<WebInstanceHostV1>();
if (enable_web_instance_tmp) {
const zx_status_t status = fdio_open(
"/tmp",
Expand Down Expand Up @@ -397,8 +324,6 @@ int main(int argc, char** argv) {

LOG(INFO) << "Launched browser at URL " << url.spec();

base::ComponentContextForProcess()->outgoing()->ServeFromStartupInfo();

// Run until the process is killed with CTRL-C or the connections to Web
// Engine interfaces are dropped.
run_loop.Run();
Expand Down
91 changes: 1 addition & 90 deletions fuchsia_web/shell/web_engine_shell.cml
View file
Expand Up @@ -10,7 +10,6 @@
// to function correctly.
"//build/config/fuchsia/test/chromium_test_facet.shard.test-cml",
"//build/config/fuchsia/test/elf_test_ambient_exec_runner.shard.test-cml",
"sys/component/realm_builder_absolute.shard.cml",
"syslog/client.shard.cml",
],
program: {
Expand All @@ -19,6 +18,7 @@
use: [
{
protocol: [
"fuchsia.element.GraphicalPresenter",
"fuchsia.feedback.ComponentDataRegister",
"fuchsia.feedback.CrashReportingProductRegister",
"fuchsia.sys.Environment",
Expand All @@ -33,14 +33,10 @@
protocol: "fuchsia.web.ContextProvider",
availability: "optional",
},

// Used to hold the cdm_data directory passed to web_instance.
{
storage: "data",
path: "/data",
},

// Needed when launched with --enable-web-instance-tmp.
{
storage: "tmp",
path: "/tmp",
Expand Down Expand Up @@ -84,107 +80,22 @@
{
protocol: [
"fuchsia.accessibility.semantics.SemanticsManager",
"fuchsia.element.GraphicalPresenter",
"fuchsia.tracing.provider.Registry",
"fuchsia.ui.composition.Allocator",
"fuchsia.ui.composition.Flatland",
"fuchsia.ui.policy.Presenter",
"fuchsia.ui.scenic.Scenic",
"fuchsia.vulkan.loader.Loader",
],
availability: "optional",
},
{
protocol: "fuchsia.tracing.perfetto.ProducerConnector",
availability: "optional",
},
],
offer: [
{
storage: [
"data",
"tmp",
],
from: "parent",
to: "#realm_builder",
},
{
directory: [
"config-data",
"root-ssl-certificates",
],
from: "parent",
to: "#realm_builder",
},

// Offers for web_instance.cm.
{
protocol: [
"fuchsia.buildinfo.Provider",
"fuchsia.device.NameProvider",
"fuchsia.feedback.ComponentDataRegister",
"fuchsia.feedback.CrashReportingProductRegister",
"fuchsia.fonts.Provider",
"fuchsia.hwinfo.Product",
"fuchsia.input.virtualkeyboard.ControllerCreator",
"fuchsia.intl.PropertyProvider",
"fuchsia.kernel.VmexResource",
"fuchsia.media.Audio",
"fuchsia.media.AudioDeviceEnumerator",
"fuchsia.media.ProfileProvider",
"fuchsia.media.SessionAudioConsumerFactory",
"fuchsia.mediacodec.CodecFactory",
"fuchsia.memorypressure.Provider",
"fuchsia.net.interfaces.State",
"fuchsia.net.name.Lookup",
"fuchsia.posix.socket.Provider",
"fuchsia.process.Launcher",
"fuchsia.sysmem.Allocator",
"fuchsia.ui.input3.Keyboard",
],
from: "parent",
to: "#realm_builder",
},

// Used conditionally based on the value of `enable_widevine` at build time.
// TODO(crbug.com/1379411): Use a shard to conditionally use based on
// build-time config.
{
protocol: "fuchsia.media.drm.Widevine",
from: "parent",
to: "#realm_builder",
availability: "optional",
},

// Used conditionally based on the absence of `--headless` on the command
// line at runtime.
{
protocol: [
"fuchsia.accessibility.semantics.SemanticsManager",
"fuchsia.element.GraphicalPresenter",
"fuchsia.tracing.provider.Registry",
"fuchsia.ui.composition.Allocator",
"fuchsia.ui.composition.Flatland",
"fuchsia.ui.policy.Presenter",
"fuchsia.ui.scenic.Scenic",
"fuchsia.vulkan.loader.Loader",
],
from: "parent",
to: "#realm_builder",
availability: "optional",
},

// Optional capabilities, dependent on availability of tracing services.
{
protocol: "fuchsia.tracing.perfetto.ProducerConnector",
from: "parent",
to: "#realm_builder",
availability: "optional",
},
],
facets: {
"fuchsia.test.deprecated-allowed-packages": [
"test_manager",
"web_engine",
"web_engine_with_webui",
],
Expand Down

0 comments on commit 1328dca

Please sign in to comment.