Skip to content

Commit

Permalink
Drop shell browser web test code from the Android build.
Browse files Browse the repository at this point in the history 
In order to add a WebTestShellPlatformDelegate override for other
platforms, I need to add one for each web test build configuration to
avoid messy defines in the header. That would also need an Android
version even though we don't run web tests there.

So in this CL we group all the web test files/deps together in the
BUILD.gn and do not include them on android. This allows us to avoid
adding a WebTestShellPlatformDelegate implementation for the platform.

Moves the MockClientHintsControllerDelegate class up from web_test/
to the content/test/ directory, as it is shared test code between
content_browsertests and web tests. We also move its dependent
mock_client_hints_utils.{h,cc} helpers with it to content/test/.

This removes the 2 dependencies from content_browsertests on web test
code via content shell, now that the web test code will not be present
on Android, but content_browsertests are.

R=avi@chromium.org

Change-Id: I7f40fc67f4a6c993dd7eccac0da5bc7482806fe3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2270800
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784089}
  • Loading branch information
@danakj
danakj authored and Commit Bot committed Jun 30, 2020
1 parent 64c1949 commit 0e17d64
Show file tree
Hide file tree
Showing 17 changed files with 300 additions and 122 deletions.
5 changes: 3 additions & 2 deletions content/browser/web_contents/web_contents_impl_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@
#include "content/shell/browser/shell.h"
#include "content/shell/browser/shell_browser_context.h"
#include "content/shell/browser/shell_content_browser_client.h"
#include "content/shell/browser/web_test/mock_client_hints_controller_delegate.h"
#include "content/test/content_browser_test_utils_internal.h"
#include "content/test/mock_client_hints_controller_delegate.h"
#include "content/test/resource_load_observer.h"
#include "content/test/test_content_browser_client.h"
#include "net/base/features.h"
Expand Down Expand Up @@ -2341,7 +2341,8 @@ class WebContentsImplBrowserTestClientHintsEnabled
// DidStartNavigation().
IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTestClientHintsEnabled,
SetUserAgentOverrideFromDidStartNavigation) {
MockClientHintsControllerDelegate client_hints_controller_delegate;
MockClientHintsControllerDelegate client_hints_controller_delegate(
content::GetShellUserAgentMetadata());
ShellContentBrowserClient::Get()
->browser_context()
->set_client_hints_controller_delegate(&client_hints_controller_delegate);
Expand Down
163 changes: 85 additions & 78 deletions content/shell/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ static_library("web_test_renderer") {
"renderer/web_test/web_widget_test_proxy.h",
]
deps = [
":client_hints_util",
":content_shell_lib",
":web_test_common",
"//base",
Expand All @@ -170,6 +169,7 @@ static_library("web_test_renderer") {
"//content/public/common",
"//content/public/renderer", # For component builds.
"//content/renderer:for_content_tests", # For non-component builds.
"//content/test:test_support",
"//device/gamepad/public/cpp:shared_with_blink",
"//device/gamepad/public/mojom",
"//gin",
Expand Down Expand Up @@ -221,11 +221,15 @@ static_library("content_shell_app") {
]
deps = [
":content_shell_lib",
":web_test_common",
":web_test_renderer",
"//content/public/app",
"//v8",
]
if (!is_android) {
deps += [
":web_test_common",
":web_test_renderer",
]
}
if (!is_fuchsia) {
deps += [
"//components/crash/core/app",
Expand Down Expand Up @@ -288,63 +292,6 @@ static_library("content_shell_lib") {
"browser/shell_web_contents_view_delegate_android.cc",
"browser/shell_web_contents_view_delegate_creator.h",
"browser/shell_web_contents_view_delegate_mac.mm",
"browser/web_test/devtools_protocol_test_bindings.cc",
"browser/web_test/devtools_protocol_test_bindings.h",
"browser/web_test/fake_bluetooth_chooser.cc",
"browser/web_test/fake_bluetooth_chooser.h",
"browser/web_test/fake_bluetooth_chooser_factory.cc",
"browser/web_test/fake_bluetooth_chooser_factory.h",
"browser/web_test/fake_bluetooth_delegate.cc",
"browser/web_test/fake_bluetooth_delegate.h",
"browser/web_test/leak_detector.cc",
"browser/web_test/leak_detector.h",
"browser/web_test/mock_client_hints_controller_delegate.cc",
"browser/web_test/mock_client_hints_controller_delegate.h",
"browser/web_test/mojo_web_test_helper.cc",
"browser/web_test/mojo_web_test_helper.h",
"browser/web_test/test_info_extractor.cc",
"browser/web_test/test_info_extractor.h",
"browser/web_test/web_test_background_fetch_delegate.cc",
"browser/web_test/web_test_background_fetch_delegate.h",
"browser/web_test/web_test_bluetooth_adapter_provider.cc",
"browser/web_test/web_test_bluetooth_adapter_provider.h",
"browser/web_test/web_test_bluetooth_chooser_factory.cc",
"browser/web_test/web_test_bluetooth_chooser_factory.h",
"browser/web_test/web_test_bluetooth_fake_adapter_setter_impl.cc",
"browser/web_test/web_test_bluetooth_fake_adapter_setter_impl.h",
"browser/web_test/web_test_browser_context.cc",
"browser/web_test/web_test_browser_context.h",
"browser/web_test/web_test_browser_main_parts.cc",
"browser/web_test/web_test_browser_main_parts.h",
"browser/web_test/web_test_browser_main_platform_support.h",
"browser/web_test/web_test_browser_main_platform_support_linux.cc",
"browser/web_test/web_test_browser_main_platform_support_mac.mm",
"browser/web_test/web_test_browser_main_platform_support_win.cc",
"browser/web_test/web_test_browser_main_runner.cc",
"browser/web_test/web_test_browser_main_runner.h",
"browser/web_test/web_test_client_impl.cc",
"browser/web_test/web_test_client_impl.h",
"browser/web_test/web_test_content_browser_client.cc",
"browser/web_test/web_test_content_browser_client.h",
"browser/web_test/web_test_control_host.cc",
"browser/web_test/web_test_control_host.h",
"browser/web_test/web_test_devtools_bindings.cc",
"browser/web_test/web_test_devtools_bindings.h",
"browser/web_test/web_test_download_manager_delegate.cc",
"browser/web_test/web_test_download_manager_delegate.h",
"browser/web_test/web_test_first_device_bluetooth_chooser.cc",
"browser/web_test/web_test_first_device_bluetooth_chooser.h",
"browser/web_test/web_test_javascript_dialog_manager.cc",
"browser/web_test/web_test_javascript_dialog_manager.h",
"browser/web_test/web_test_permission_manager.cc",
"browser/web_test/web_test_permission_manager.h",
"browser/web_test/web_test_push_messaging_service.cc",
"browser/web_test/web_test_push_messaging_service.h",
"browser/web_test/web_test_shell_platform_delegate.cc",
"browser/web_test/web_test_shell_platform_delegate.h",
"browser/web_test/web_test_shell_platform_delegate_mac.mm",
"browser/web_test/web_test_tts_platform.cc",
"browser/web_test/web_test_tts_platform.h",
"common/power_monitor_test_impl.cc",
"common/power_monitor_test_impl.h",
"common/shell_content_client.cc",
Expand Down Expand Up @@ -397,11 +344,9 @@ static_library("content_shell_lib") {
"//services/network:network_service",
]
deps = [
":client_hints_util",
":content_browsertests_mojom",
":resources",
":shell_controller_mojom",
":web_test_common",
"//base",
"//base:base_static",
"//base/third_party/dynamic_annotations",
Expand Down Expand Up @@ -430,9 +375,7 @@ static_library("content_shell_lib") {
"//content/public/common:service_names",
"//content/test:blink_test_browser_support",
"//content/test:content_test_mojo_bindings",
"//content/test:mojo_web_test_bindings",
"//content/test:test_support",
"//content/test:web_test_support_browser",
"//device/bluetooth",
"//device/bluetooth:fake_bluetooth",
"//device/bluetooth:mocks",
Expand Down Expand Up @@ -481,6 +424,84 @@ static_library("content_shell_lib") {
"//v8",
]

# Web test support not built on android, but is everywhere else.
support_web_tests = !is_android

# TODO(danakj): Move this stuff into a :web_test_browser library that
# only :content_shell_app depends on.
if (support_web_tests) {
sources += [
"browser/web_test/devtools_protocol_test_bindings.cc",
"browser/web_test/devtools_protocol_test_bindings.h",
"browser/web_test/fake_bluetooth_chooser.cc",
"browser/web_test/fake_bluetooth_chooser.h",
"browser/web_test/fake_bluetooth_chooser_factory.cc",
"browser/web_test/fake_bluetooth_chooser_factory.h",
"browser/web_test/fake_bluetooth_delegate.cc",
"browser/web_test/fake_bluetooth_delegate.h",
"browser/web_test/leak_detector.cc",
"browser/web_test/leak_detector.h",
"browser/web_test/mojo_web_test_helper.cc",
"browser/web_test/mojo_web_test_helper.h",
"browser/web_test/test_info_extractor.cc",
"browser/web_test/test_info_extractor.h",
"browser/web_test/web_test_background_fetch_delegate.cc",
"browser/web_test/web_test_background_fetch_delegate.h",
"browser/web_test/web_test_bluetooth_adapter_provider.cc",
"browser/web_test/web_test_bluetooth_adapter_provider.h",
"browser/web_test/web_test_bluetooth_chooser_factory.cc",
"browser/web_test/web_test_bluetooth_chooser_factory.h",
"browser/web_test/web_test_bluetooth_fake_adapter_setter_impl.cc",
"browser/web_test/web_test_bluetooth_fake_adapter_setter_impl.h",
"browser/web_test/web_test_browser_context.cc",
"browser/web_test/web_test_browser_context.h",
"browser/web_test/web_test_browser_main_parts.cc",
"browser/web_test/web_test_browser_main_parts.h",
"browser/web_test/web_test_browser_main_platform_support.h",
"browser/web_test/web_test_browser_main_platform_support_linux.cc",
"browser/web_test/web_test_browser_main_platform_support_mac.mm",
"browser/web_test/web_test_browser_main_platform_support_win.cc",
"browser/web_test/web_test_browser_main_runner.cc",
"browser/web_test/web_test_browser_main_runner.h",
"browser/web_test/web_test_client_impl.cc",
"browser/web_test/web_test_client_impl.h",
"browser/web_test/web_test_content_browser_client.cc",
"browser/web_test/web_test_content_browser_client.h",
"browser/web_test/web_test_control_host.cc",
"browser/web_test/web_test_control_host.h",
"browser/web_test/web_test_devtools_bindings.cc",
"browser/web_test/web_test_devtools_bindings.h",
"browser/web_test/web_test_download_manager_delegate.cc",
"browser/web_test/web_test_download_manager_delegate.h",
"browser/web_test/web_test_first_device_bluetooth_chooser.cc",
"browser/web_test/web_test_first_device_bluetooth_chooser.h",
"browser/web_test/web_test_javascript_dialog_manager.cc",
"browser/web_test/web_test_javascript_dialog_manager.h",
"browser/web_test/web_test_permission_manager.cc",
"browser/web_test/web_test_permission_manager.h",
"browser/web_test/web_test_push_messaging_service.cc",
"browser/web_test/web_test_push_messaging_service.h",
"browser/web_test/web_test_shell_platform_delegate.h",
"browser/web_test/web_test_tts_platform.cc",
"browser/web_test/web_test_tts_platform.h",
]

deps += [
":web_test_common",
"//content/test:mojo_web_test_bindings",
"//content/test:web_test_support_browser",
]

if (is_mac) {
sources += [ "browser/web_test/web_test_shell_platform_delegate_mac.mm" ]
} else if (toolkit_views && !is_chromecast) {
sources +=
[ "browser/web_test/web_test_shell_platform_delegate_views.cc" ]
} else {
sources += [ "browser/web_test/web_test_shell_platform_delegate_aura.cc" ]
}
}

if (is_fuchsia) {
deps -= [
"//components/crash/content/browser",
Expand Down Expand Up @@ -613,20 +634,6 @@ static_library("content_shell_lib") {
}
}

static_library("client_hints_util") {
testonly = true

sources = [
"utility/mock_client_hints_utils.cc",
"utility/mock_client_hints_utils.h",
]

deps = [
"//content/public/common",
"//third_party/blink/public:blink",
]
}

grit("content_shell_resources_grit") {
testonly = true

Expand Down
31 changes: 21 additions & 10 deletions content/shell/app/shell_main_delegate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,10 @@
#include "content/public/common/url_constants.h"
#include "content/shell/app/shell_crash_reporter_client.h"
#include "content/shell/browser/shell_content_browser_client.h"
#include "content/shell/browser/web_test/web_test_browser_main_runner.h"
#include "content/shell/browser/web_test/web_test_content_browser_client.h"
#include "content/shell/common/shell_content_client.h"
#include "content/shell/common/shell_switches.h"
#include "content/shell/gpu/shell_content_gpu_client.h"
#include "content/shell/renderer/shell_content_renderer_client.h"
#include "content/shell/renderer/web_test/web_test_content_renderer_client.h"
#include "content/shell/utility/shell_content_utility_client.h"
#include "ipc/ipc_buildflags.h"
#include "net/cookies/cookie_monster.h"
Expand All @@ -44,6 +41,12 @@
content::RegisterIPCLogger(msg_id, logger)
#endif

#if !defined(OS_ANDROID)
#include "content/shell/browser/web_test/web_test_browser_main_runner.h" // nogncheck
#include "content/shell/browser/web_test/web_test_content_browser_client.h" // nogncheck
#include "content/shell/renderer/web_test/web_test_content_renderer_client.h" // nogncheck
#endif

#if defined(OS_ANDROID)
#include "base/android/apk_assets.h"
#include "base/posix/global_descriptors.h"
Expand Down Expand Up @@ -168,6 +171,7 @@ bool ShellMainDelegate::BasicStartupComplete(int* exit_code) {

InitLogging(command_line);

#if !defined(OS_ANDROID)
if (switches::IsRunWebTestsSwitchPresent()) {
const bool browser_process =
command_line.GetSwitchValueASCII(switches::kProcessType).empty();
Expand All @@ -176,6 +180,7 @@ bool ShellMainDelegate::BasicStartupComplete(int* exit_code) {
web_test_runner_->Initialize();
}
}
#endif

return false;
}
Expand Down Expand Up @@ -224,6 +229,7 @@ int ShellMainDelegate::RunProcess(
base::trace_event::TraceLog::GetInstance()->SetProcessSortIndex(
kTraceEventBrowserProcessSortIndex);

#if !defined(OS_ANDROID)
if (switches::IsRunWebTestsSwitchPresent()) {
// Web tests implement their own BrowserMain() replacement.
web_test_runner_->RunBrowserMain(main_function_params);
Expand All @@ -234,7 +240,6 @@ int ShellMainDelegate::RunProcess(
return 0;
}

#if !defined(OS_ANDROID)
// On non-Android, we can return -1 and have the caller run BrowserMain()
// normally.
return -1;
Expand Down Expand Up @@ -326,10 +331,13 @@ ContentClient* ShellMainDelegate::CreateContentClient() {
}

ContentBrowserClient* ShellMainDelegate::CreateContentBrowserClient() {
if (switches::IsRunWebTestsSwitchPresent())
#if !defined(OS_ANDROID)
if (switches::IsRunWebTestsSwitchPresent()) {
browser_client_ = std::make_unique<WebTestContentBrowserClient>();
else
browser_client_ = std::make_unique<ShellContentBrowserClient>();
return browser_client_.get();
}
#endif
browser_client_ = std::make_unique<ShellContentBrowserClient>();
return browser_client_.get();
}

Expand All @@ -339,10 +347,13 @@ ContentGpuClient* ShellMainDelegate::CreateContentGpuClient() {
}

ContentRendererClient* ShellMainDelegate::CreateContentRendererClient() {
if (switches::IsRunWebTestsSwitchPresent())
#if !defined(OS_ANDROID)
if (switches::IsRunWebTestsSwitchPresent()) {
renderer_client_ = std::make_unique<WebTestContentRendererClient>();
else
renderer_client_ = std::make_unique<ShellContentRendererClient>();
return renderer_client_.get();
}
#endif
renderer_client_ = std::make_unique<ShellContentRendererClient>();
return renderer_client_.get();
}

Expand Down
5 changes: 5 additions & 0 deletions content/shell/app/shell_main_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ class ShellContentBrowserClient;
class ShellContentGpuClient;
class ShellContentRendererClient;
class ShellContentUtilityClient;

#if !defined(OS_ANDROID)
class WebTestBrowserMainRunner;
#endif

class ShellMainDelegate : public ContentMainDelegate {
public:
Expand Down Expand Up @@ -49,11 +52,13 @@ class ShellMainDelegate : public ContentMainDelegate {
// content_browsertests should not set the kRunWebTests command line flag, so
// |is_content_browsertests_| and |web_test_runner_| are mututally exclusive.
bool is_content_browsertests_;
#if !defined(OS_ANDROID)
// Only present when running web tests, which run inside Content Shell.
//
// Web tests are not browser tests, so |is_content_browsertests_| and
// |web_test_runner_| are mututally exclusive.
std::unique_ptr<WebTestBrowserMainRunner> web_test_runner_;
#endif

std::unique_ptr<ShellContentBrowserClient> browser_client_;
std::unique_ptr<ShellContentGpuClient> gpu_client_;
Expand Down
Loading

0 comments on commit 0e17d64

Please sign in to comment.