Skip to content

Commit

Permalink
[Fuchsia] Set AEC effects flags only in CastRunner
Browse files Browse the repository at this point in the history 
Previously AudioManagerFuchsia was always setting echo cancellation,
noise suppression and gain control effect flags for input device, while
these effects are not supported in the default AudioCapturer
implementation. This CL adds a new command line switch for WebEngine
that indicates that these features are supported. The switch is passed
only for the main context in cast_runner. In all other cases
AudioCapturer is assumed to run without echo cancellation, which enables
echo cancellation in WebRTC.

Bug: 1322701
Change-Id: Ia67724473fcd4e1236fa64ef98b399cb75bbd63f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3631203
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001246}
  • Loading branch information
@SergeyUlanov
SergeyUlanov authored and Chromium LUCI CQ committed May 9, 2022
1 parent 497bf3b commit 969d3e9
Show file tree
Hide file tree
Showing 10 changed files with 172 additions and 109 deletions.
4 changes: 4 additions & 0 deletions fuchsia/runners/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ source_set("cast_runner_core") {
"//components/cast/named_message_port_connector:named_message_port_connector",
"//fuchsia/base",
"//fuchsia/base:modular",

# TODO(crbug.com/852834): Remove this dependency when
# kAudioCapturerWithEchoCancellation is removed.
"//media",
"//third_party/fuchsia-sdk/sdk/fidl/fuchsia.diagnostics",
"//third_party/fuchsia-sdk/sdk/fidl/fuchsia.mem",
"//third_party/fuchsia-sdk/sdk/fidl/fuchsia.modular",
Expand Down
5 changes: 5 additions & 0 deletions fuchsia/runners/cast/DEPS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ include_rules = [
"+components/cast",
"+content/public/test",
"+fuchsia/fidl/chromium/cast/cpp/fidl.h",

# TODO(crbug.com/852834): Remove this dependency when
# kAudioCapturerWithEchoCancellation is removed.
"+media/base/media_switches.h",

"+net",
]

Expand Down
117 changes: 67 additions & 50 deletions fuchsia/runners/cast/cast_runner.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <utility>

#include "base/bind.h"
#include "base/command_line.h"
#include "base/files/file_enumerator.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
Expand All @@ -30,6 +31,7 @@
#include "fuchsia/runners/cast/cast_streaming.h"
#include "fuchsia/runners/cast/pending_cast_component.h"
#include "fuchsia/runners/common/web_content_runner.h"
#include "media/base/media_switches.h"
#include "url/gurl.h"

namespace {
Expand Down Expand Up @@ -316,7 +318,7 @@ CastRunner::CastRunner(cr_fuchsia::WebInstanceHost* web_instance_host,
base::ComponentContextForProcess()->svc())),
main_context_(std::make_unique<WebContentRunner>(
web_instance_host_,
base::BindRepeating(&CastRunner::GetMainContextParams,
base::BindRepeating(&CastRunner::GetMainWebInstanceConfig,
base::Unretained(this)))),
isolated_services_(std::make_unique<base::FilteredServiceDirectory>(
base::ComponentContextForProcess()->svc())) {
Expand Down Expand Up @@ -477,13 +479,13 @@ void CastRunner::LaunchPendingComponent(PendingCastComponent* pending_component,
if (IsAppConfigForCastStreaming(params.application_config))
web_content_url = GURL(kCastStreamingWebUrl);

absl::optional<fuchsia::web::CreateContextParams> create_context_params =
GetContextParamsForAppConfig(&params.application_config);
auto web_instance_config =
GetWebInstanceConfigForAppConfig(&params.application_config);

WebContentRunner* component_owner = main_context_.get();
if (create_context_params) {
component_owner = CreateIsolatedContextForParams(
std::move(create_context_params.value()));
if (web_instance_config) {
component_owner =
CreateIsolatedRunner(std::move(web_instance_config.value()));
}

auto cast_component = std::make_unique<CastComponent>(
Expand Down Expand Up @@ -559,118 +561,133 @@ void CastRunner::OnComponentDestroyed(CastComponent* component) {
video_capturer_components_.erase(component);
}

fuchsia::web::CreateContextParams CastRunner::GetCommonContextParams() {
WebContentRunner::WebInstanceConfig CastRunner::GetCommonWebInstanceConfig() {
DCHECK(cors_exempt_headers_);

fuchsia::web::CreateContextParams params;
params.set_features(fuchsia::web::ContextFeatureFlags::AUDIO);
WebContentRunner::WebInstanceConfig config;

// Pass all arguments from `cast_runner` to the `web_instance`.
// TODO(crbug.com/1323372): Remove this.
config.extra_args = *base::CommandLine::ForCurrentProcess();

config.params.set_features(fuchsia::web::ContextFeatureFlags::AUDIO);

if (is_headless_) {
LOG(WARNING) << "Running in headless mode.";
*params.mutable_features() |= fuchsia::web::ContextFeatureFlags::HEADLESS;
*config.params.mutable_features() |=
fuchsia::web::ContextFeatureFlags::HEADLESS;
} else {
*params.mutable_features() |=
*config.params.mutable_features() |=
fuchsia::web::ContextFeatureFlags::HARDWARE_VIDEO_DECODER |
fuchsia::web::ContextFeatureFlags::VULKAN;
}

// When tests require that VULKAN be disabled, DRM must also be disabled.
if (disable_vulkan_for_test_) {
*params.mutable_features() &=
*config.params.mutable_features() &=
~(fuchsia::web::ContextFeatureFlags::VULKAN |
fuchsia::web::ContextFeatureFlags::HARDWARE_VIDEO_DECODER);
}

// If there is a list of headers to exempt from CORS checks, pass the list
// along to the Context.
if (!cors_exempt_headers_->empty())
params.set_cors_exempt_headers(*cors_exempt_headers_);
config.params.set_cors_exempt_headers(*cors_exempt_headers_);

return params;
return config;
}

fuchsia::web::CreateContextParams CastRunner::GetMainContextParams() {
fuchsia::web::CreateContextParams params = GetCommonContextParams();
*params.mutable_features() |=
WebContentRunner::WebInstanceConfig CastRunner::GetMainWebInstanceConfig() {
auto config = GetCommonWebInstanceConfig();

// AudioCapturer is redirected to the agent (see `OnAudioServiceRequest()`).
// The implementation provided by the agent supports echo cancellation.
//
// TODO(crbug.com/852834): Remove once AudioManagerFuchsia is updated to
// get this information from AudioCapturerFactory.
config.extra_args.AppendSwitch(switches::kAudioCapturerWithEchoCancellation);

*config.params.mutable_features() |=
fuchsia::web::ContextFeatureFlags::NETWORK |
fuchsia::web::ContextFeatureFlags::LEGACYMETRICS |
fuchsia::web::ContextFeatureFlags::KEYBOARD |
fuchsia::web::ContextFeatureFlags::VIRTUAL_KEYBOARD;
EnsureSoftwareVideoDecodersAreDisabled(params.mutable_features());
params.set_remote_debugging_port(CastRunner::kRemoteDebuggingPort);
EnsureSoftwareVideoDecodersAreDisabled(config.params.mutable_features());
config.params.set_remote_debugging_port(CastRunner::kRemoteDebuggingPort);

params.set_user_agent_product("CrKey");
params.set_user_agent_version(chromecast::kFrozenCrKeyValue);
config.params.set_user_agent_product("CrKey");
config.params.set_user_agent_version(chromecast::kFrozenCrKeyValue);

zx_status_t status = main_services_->ConnectClient(
params.mutable_service_directory()->NewRequest());
config.params.mutable_service_directory()->NewRequest());
ZX_CHECK(status == ZX_OK, status) << "ConnectClient failed";

if (!disable_vulkan_for_test_) {
SetCdmParamsForMainContext(&params);
SetCdmParamsForMainContext(&config.params);
}

SetDataParamsForMainContext(&params);
SetDataParamsForMainContext(&config.params);

// Create a sentinel file to detect if the cache is erased.
// TODO(crbug.com/1188780): Remove once an explicit cache flush signal exists.
CreatePersistedCacheSentinel();

// TODO(crbug.com/1023514): Remove this switch when it is no longer
// necessary.
params.set_unsafely_treat_insecure_origins_as_secure(
config.params.set_unsafely_treat_insecure_origins_as_secure(
{"allow-running-insecure-content", "disable-mixed-content-autoupgrade"});

return params;
return config;
}

fuchsia::web::CreateContextParams
CastRunner::GetIsolatedContextParamsWithFuchsiaDirs(
WebContentRunner::WebInstanceConfig
CastRunner::GetIsolatedWebInstanceConfigWithFuchsiaDirs(
std::vector<fuchsia::web::ContentDirectoryProvider> content_directories) {
fuchsia::web::CreateContextParams params = GetCommonContextParams();
auto config = GetCommonWebInstanceConfig();

EnsureSoftwareVideoDecodersAreDisabled(params.mutable_features());
*params.mutable_features() |= fuchsia::web::ContextFeatureFlags::NETWORK;
params.set_remote_debugging_port(kEphemeralRemoteDebuggingPort);
params.set_content_directories(std::move(content_directories));
EnsureSoftwareVideoDecodersAreDisabled(config.params.mutable_features());
*config.params.mutable_features() |=
fuchsia::web::ContextFeatureFlags::NETWORK;
config.params.set_remote_debugging_port(kEphemeralRemoteDebuggingPort);
config.params.set_content_directories(std::move(content_directories));

zx_status_t status = isolated_services_->ConnectClient(
params.mutable_service_directory()->NewRequest());
config.params.mutable_service_directory()->NewRequest());
ZX_CHECK(status == ZX_OK, status) << "ConnectClient failed";

return params;
return config;
}

fuchsia::web::CreateContextParams
CastRunner::GetIsolatedContextParamsForCastStreaming() {
fuchsia::web::CreateContextParams params = GetCommonContextParams();
WebContentRunner::WebInstanceConfig
CastRunner::GetIsolatedWebInstanceConfigForCastStreaming() {
auto config = GetCommonWebInstanceConfig();

ApplyCastStreamingContextParams(&params);
params.set_remote_debugging_port(kEphemeralRemoteDebuggingPort);
ApplyCastStreamingContextParams(&config.params);
config.params.set_remote_debugging_port(kEphemeralRemoteDebuggingPort);

// TODO(crbug.com/1069746): Use a different FilteredServiceDirectory for Cast
// Streaming Contexts.
zx_status_t status = main_services_->ConnectClient(
params.mutable_service_directory()->NewRequest());
config.params.mutable_service_directory()->NewRequest());
ZX_CHECK(status == ZX_OK, status) << "ConnectClient failed";

return params;
return config;
}

absl::optional<fuchsia::web::CreateContextParams>
CastRunner::GetContextParamsForAppConfig(
absl::optional<WebContentRunner::WebInstanceConfig>
CastRunner::GetWebInstanceConfigForAppConfig(
chromium::cast::ApplicationConfig* app_config) {
if (IsAppConfigForCastStreaming(*app_config)) {
// TODO(crbug.com/1082821): Remove this once the CastStreamingReceiver
// Component has been implemented.
return absl::make_optional(GetIsolatedContextParamsForCastStreaming());
return absl::make_optional(GetIsolatedWebInstanceConfigForCastStreaming());
}

const bool is_isolated_app =
app_config->has_content_directories_for_isolated_application();
if (is_isolated_app) {
return absl::make_optional(
GetIsolatedContextParamsWithFuchsiaDirs(std::move(
GetIsolatedWebInstanceConfigWithFuchsiaDirs(std::move(
*app_config
->mutable_content_directories_for_isolated_application())));
}
Expand All @@ -679,11 +696,11 @@ CastRunner::GetContextParamsForAppConfig(
return absl::nullopt;
}

WebContentRunner* CastRunner::CreateIsolatedContextForParams(
fuchsia::web::CreateContextParams create_context_params) {
WebContentRunner* CastRunner::CreateIsolatedRunner(
WebContentRunner::WebInstanceConfig config) {
// Create an isolated context which will own the CastComponent.
auto context = std::make_unique<WebContentRunner>(
web_instance_host_, std::move(create_context_params));
auto context =
std::make_unique<WebContentRunner>(web_instance_host_, std::move(config));
context->SetOnEmptyCallback(
base::BindOnce(&CastRunner::OnIsolatedContextEmpty,
base::Unretained(this), base::Unretained(context.get())));
Expand Down
30 changes: 16 additions & 14 deletions fuchsia/runners/cast/cast_runner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
#include "base/containers/flat_set.h"
#include "base/containers/unique_ptr_adapters.h"
#include "base/fuchsia/startup_context.h"
#include "fuchsia/runners/cast/fidl/fidl/chromium/cast/cpp/fidl.h"
#include "fuchsia/runners/cast/cast_component.h"
#include "fuchsia/runners/cast/fidl/fidl/chromium/cast/cpp/fidl.h"
#include "fuchsia/runners/cast/pending_cast_component.h"
#include "fuchsia/runners/common/web_content_runner.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace base {
Expand All @@ -31,8 +32,6 @@ namespace cr_fuchsia {
class WebInstanceHost;
} // namespace cr_fuchsia

class WebContentRunner;

// sys::Runner which instantiates Cast activities specified via cast/casts URIs.
class CastRunner final : public fuchsia::sys::Runner,
public PendingCastComponent::Delegate {
Expand Down Expand Up @@ -76,23 +75,26 @@ class CastRunner final : public fuchsia::sys::Runner,
void OnComponentDestroyed(CastComponent* component);

// Handlers used to provide parameters for main & isolated Contexts.
fuchsia::web::CreateContextParams GetCommonContextParams();
fuchsia::web::CreateContextParams GetMainContextParams();
fuchsia::web::CreateContextParams GetIsolatedContextParamsWithFuchsiaDirs(
WebContentRunner::WebInstanceConfig GetCommonWebInstanceConfig();
WebContentRunner::WebInstanceConfig GetMainWebInstanceConfig();
WebContentRunner::WebInstanceConfig
GetIsolatedWebInstanceConfigWithFuchsiaDirs(
std::vector<fuchsia::web::ContentDirectoryProvider> content_directories);
// TODO(crbug.com/1082821): Remove this once the CastStreamingReceiver
// Component has been implemented.
fuchsia::web::CreateContextParams GetIsolatedContextParamsForCastStreaming();
WebContentRunner::WebInstanceConfig
GetIsolatedWebInstanceConfigForCastStreaming();

// Returns CreateContextParams for |app_config|. Returns nullopt if there is
// no need to create an isolated context.
absl::optional<fuchsia::web::CreateContextParams>
GetContextParamsForAppConfig(chromium::cast::ApplicationConfig* app_config);

// Launches an isolated Context with the given |create_context_params| and
// returns the newly created WebContentRunner.
WebContentRunner* CreateIsolatedContextForParams(
fuchsia::web::CreateContextParams create_context_params);
absl::optional<WebContentRunner::WebInstanceConfig>
GetWebInstanceConfigForAppConfig(
chromium::cast::ApplicationConfig* app_config);

// Launches an isolated Context with the given `config` and returns the newly
// created WebContentRunner.
WebContentRunner* CreateIsolatedRunner(
WebContentRunner::WebInstanceConfig config);

// Called when an isolated component terminates, to allow the Context hosting
// it to be torn down.
Expand Down
33 changes: 21 additions & 12 deletions fuchsia/runners/common/web_content_runner.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,30 @@ std::string CreateUniqueComponentName() {

} // namespace

WebContentRunner::WebInstanceConfig::WebInstanceConfig()
: extra_args(base::CommandLine::NO_PROGRAM) {}
WebContentRunner::WebInstanceConfig::~WebInstanceConfig() = default;

WebContentRunner::WebInstanceConfig::WebInstanceConfig(WebInstanceConfig&&) =
default;
WebContentRunner::WebInstanceConfig&
WebContentRunner::WebInstanceConfig::operator=(WebInstanceConfig&&) = default;

WebContentRunner::WebContentRunner(
cr_fuchsia::WebInstanceHost* web_instance_host,
GetContextParamsCallback get_context_params_callback)
GetWebInstanceConfigCallback get_web_instance_config_callback)
: web_instance_host_(web_instance_host),
get_context_params_callback_(std::move(get_context_params_callback)) {
get_web_instance_config_callback_(
std::move(get_web_instance_config_callback)) {
DCHECK(web_instance_host_);
DCHECK(get_context_params_callback_);
DCHECK(get_web_instance_config_callback_);
}

WebContentRunner::WebContentRunner(
cr_fuchsia::WebInstanceHost* web_instance_host,
fuchsia::web::CreateContextParams context_params)
WebInstanceConfig web_instance_config)
: web_instance_host_(web_instance_host) {
CreateWebInstanceAndContext(std::move(context_params));
CreateWebInstanceAndContext(std::move(web_instance_config));
}

WebContentRunner::~WebContentRunner() = default;
Expand Down Expand Up @@ -125,7 +135,7 @@ void WebContentRunner::SetOnEmptyCallback(base::OnceClosure on_empty) {
}

void WebContentRunner::DestroyWebContext() {
DCHECK(get_context_params_callback_);
DCHECK(get_web_instance_config_callback_);
context_ = nullptr;
}

Expand All @@ -137,16 +147,15 @@ void WebContentRunner::EnsureWebInstanceAndContext() {
context_.Unbind();

if (!context_) {
DCHECK(get_context_params_callback_);
CreateWebInstanceAndContext(get_context_params_callback_.Run());
DCHECK(get_web_instance_config_callback_);
CreateWebInstanceAndContext(get_web_instance_config_callback_.Run());
}
}

void WebContentRunner::CreateWebInstanceAndContext(
fuchsia::web::CreateContextParams params) {
void WebContentRunner::CreateWebInstanceAndContext(WebInstanceConfig config) {
web_instance_host_->CreateInstanceForContextWithCopiedArgs(
std::move(params), web_instance_services_.NewRequest(),
*base::CommandLine::ForCurrentProcess());
std::move(config.params), web_instance_services_.NewRequest(),
config.extra_args);
zx_status_t result = fdio_service_connect_at(
web_instance_services_.channel().get(), fuchsia::web::Context::Name_,
context_.NewRequest().TakeChannel().release());
Expand Down

0 comments on commit 969d3e9

Please sign in to comment.