Commit 431a27f5 authored by arthursonzogni's avatar arthursonzogni Committed by Commit Bot

Setup OriginTrialPolicy for BrowserTestBase on Android.

The OriginTrialPolicy is not setup on the browser process, on Android,
on tests derived from BrowserTestBase.

This is normally done from ContentMain(). This function is not called
here, but reproduced more or less accurately by BrowserTestBase.

To install an OriginTrialPolicy from the ContentClient, this requires
accessing the ContentClient. So this patch is essentially exposing it
through GetContentClientForTesting() and use it.

Fixed: 1123953,1119555
Bug: 1123953,1119555
Change-Id: I403a391a84e8504c80d8df57212f347381d25c30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2505979Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatardanakj <danakj@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824106}
parent 63a09d82
...@@ -99,10 +99,9 @@ source_set("app") { ...@@ -99,10 +99,9 @@ source_set("app") {
] ]
configs += extra_configs configs += extra_configs
deps = content_app_deps + [ deps = content_app_deps + [ "//ipc" ]
":content_main_runner_app",
"//ipc", public_deps = [ ":content_main_runner_app" ]
]
# Only the public target should depend on this. All other targets (even # Only the public target should depend on this. All other targets (even
# internal content ones) should depend on the public one. # internal content ones) should depend on the public one.
......
...@@ -61,7 +61,7 @@ void SetContentMainDelegate(ContentMainDelegate* delegate) { ...@@ -61,7 +61,7 @@ void SetContentMainDelegate(ContentMainDelegate* delegate) {
ContentClientCreator::Create(delegate); ContentClientCreator::Create(delegate);
} }
ContentMainDelegate* GetContentMainDelegateForTesting() { ContentMainDelegate* GetContentMainDelegate() {
DCHECK(g_content_main_delegate.Get().get()); DCHECK(g_content_main_delegate.Get().get());
return g_content_main_delegate.Get().get(); return g_content_main_delegate.Get().get();
} }
......
...@@ -70,6 +70,7 @@ ...@@ -70,6 +70,7 @@
#include "content/public/app/content_main_delegate.h" #include "content/public/app/content_main_delegate.h"
#include "content/public/browser/content_browser_client.h" #include "content/public/browser/content_browser_client.h"
#include "content/public/browser/system_connector.h" #include "content/public/browser/system_connector.h"
#include "content/public/common/content_client.h"
#include "content/public/common/content_constants.h" #include "content/public/common/content_constants.h"
#include "content/public/common/content_descriptor_keys.h" #include "content/public/common/content_descriptor_keys.h"
#include "content/public/common/content_features.h" #include "content/public/common/content_features.h"
...@@ -1041,4 +1042,14 @@ std::unique_ptr<ContentMainRunner> ContentMainRunner::Create() { ...@@ -1041,4 +1042,14 @@ std::unique_ptr<ContentMainRunner> ContentMainRunner::Create() {
return ContentMainRunnerImpl::Create(); return ContentMainRunnerImpl::Create();
} }
ContentClient* GetContentClientForTesting() {
return GetContentClient();
}
#if defined(OS_ANDROID)
ContentMainDelegate* GetContentMainDelegateForTesting() {
return GetContentMainDelegate();
}
#endif
} // namespace content } // namespace content
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/threading/hang_watcher.h" #include "base/threading/hang_watcher.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "content/browser/startup_data_impl.h" #include "content/browser/startup_data_impl.h"
#include "content/common/content_export.h"
#include "content/public/app/content_main.h" #include "content/public/app/content_main.h"
#include "content/public/app/content_main_runner.h" #include "content/public/app/content_main_runner.h"
#include "content/public/common/main_function_params.h" #include "content/public/common/main_function_params.h"
...@@ -33,9 +34,10 @@ class DiscardableSharedMemoryManager; ...@@ -33,9 +34,10 @@ class DiscardableSharedMemoryManager;
} }
namespace content { namespace content {
class ContentClient;
class ContentMainDelegate; class ContentMainDelegate;
struct ContentMainParams;
class ServiceManagerEnvironment; class ServiceManagerEnvironment;
struct ContentMainParams;
class ContentMainRunnerImpl : public ContentMainRunner { class ContentMainRunnerImpl : public ContentMainRunner {
public: public:
...@@ -92,6 +94,16 @@ class ContentMainRunnerImpl : public ContentMainRunner { ...@@ -92,6 +94,16 @@ class ContentMainRunnerImpl : public ContentMainRunner {
DISALLOW_COPY_AND_ASSIGN(ContentMainRunnerImpl); DISALLOW_COPY_AND_ASSIGN(ContentMainRunnerImpl);
}; };
// The BrowserTestBase on Android does not call ContentMain(). It tries instead
// to reproduce it more or less accurately. This requires to use
// GetContentMainDelegateForTesting() and GetContentClientForTesting().
// BrowserTestBase is implemented in content/public and GetContentClient() is
// only available to the implementation of content. Hence these functions.
CONTENT_EXPORT ContentClient* GetContentClientForTesting();
#if defined(OS_ANDROID)
CONTENT_EXPORT ContentMainDelegate* GetContentMainDelegateForTesting();
#endif
} // namespace content } // namespace content
#endif // CONTENT_APP_CONTENT_MAIN_RUNNER_IMPL_H_ #endif // CONTENT_APP_CONTENT_MAIN_RUNNER_IMPL_H_
...@@ -4,7 +4,6 @@ ...@@ -4,7 +4,6 @@
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "components/network_session_configurator/common/network_switches.h" #include "components/network_session_configurator/common/network_switches.h"
#include "content/browser/renderer_host/navigation_request.h" #include "content/browser/renderer_host/navigation_request.h"
#include "content/browser/renderer_host/render_frame_host_impl.h" #include "content/browser/renderer_host/render_frame_host_impl.h"
...@@ -2429,39 +2428,6 @@ class CoopReportingOriginTrialBrowserTest : public ContentBrowserTest { ...@@ -2429,39 +2428,6 @@ class CoopReportingOriginTrialBrowserTest : public ContentBrowserTest {
net::EmbeddedTestServer* https_server() { return &https_server_; } net::EmbeddedTestServer* https_server() { return &https_server_; }
// On every platforms, except on Android, ContainMain is called for
// browsertest. This calls SetOriginTrialPolicyGetter.
// On Android, a meager re-implementation of ContentMainRunnerImpl is made by
// BrowserTestBase. This doesn't call SetOriginTrialPolicyGetter.
//
// So on Android + BrowserTestBase + browser process, the OriginTrial policy
// isn't setup. This is tracked by https://crbug.com/1123953
//
// To fix this we could:
//
// 1) Fix https://crbug.com/1123953. Call SetOriginTrialPolicyGetter using
// GetContentClient()->GetOriginTrialPolicy() from BrowserTestBase. This
// doesn't work, because GetContentClient() is private to the
// implementation of content/, unreachable from the test.
//
// 2) Setup our own blink::OriginTrialPolicy here, based on
// embedder_support::OriginTrialPolicy. This doesn't work, because this
// violate the DEPS rules.
//
// 3) Copy-paste the implementation of embedder_support::OriginTrialPolicy
// here. This doesn't really worth the cost.
//
// Instead we abandon testing the OriginTrial on the Android platform :-(
//
// TODO(https://crbug.com/1123953). Remove this once fixed.
bool IsOriginTrialPolicySetup() {
#if defined(OS_ANDROID)
return false;
#else
return true;
#endif
}
private: private:
void SetUpOnMainThread() final { void SetUpOnMainThread() final {
ContentBrowserTest::TearDownOnMainThread(); ContentBrowserTest::TearDownOnMainThread();
...@@ -2519,10 +2485,6 @@ IN_PROC_BROWSER_TEST_F(CoopReportingOriginTrialBrowserTest, ...@@ -2519,10 +2485,6 @@ IN_PROC_BROWSER_TEST_F(CoopReportingOriginTrialBrowserTest,
IN_PROC_BROWSER_TEST_F(CoopReportingOriginTrialBrowserTest, IN_PROC_BROWSER_TEST_F(CoopReportingOriginTrialBrowserTest,
CoopStateWithToken) { CoopStateWithToken) {
// TODO(https://crbug.com/1123953). Remove this once fixed.
if (!IsOriginTrialPolicySetup())
return;
URLLoaderInterceptor interceptor(base::BindLambdaForTesting( URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
[&](URLLoaderInterceptor::RequestParams* params) { [&](URLLoaderInterceptor::RequestParams* params) {
if (params->url_request.url != OriginTrialURL()) if (params->url_request.url != OriginTrialURL())
...@@ -2591,9 +2553,6 @@ IN_PROC_BROWSER_TEST_F(CoopReportingOriginTrialBrowserTest, ...@@ -2591,9 +2553,6 @@ IN_PROC_BROWSER_TEST_F(CoopReportingOriginTrialBrowserTest,
IN_PROC_BROWSER_TEST_F(CoopReportingOriginTrialBrowserTest, IN_PROC_BROWSER_TEST_F(CoopReportingOriginTrialBrowserTest,
AccessReportingWithToken) { AccessReportingWithToken) {
// TODO(https://crbug.com/1123953). Remove this once fixed.
if (!IsOriginTrialPolicySetup())
return;
URLLoaderInterceptor interceptor(base::BindLambdaForTesting( URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
[&](URLLoaderInterceptor::RequestParams* params) { [&](URLLoaderInterceptor::RequestParams* params) {
if (params->url_request.url != OriginTrialURL()) if (params->url_request.url != OriginTrialURL())
......
...@@ -79,10 +79,13 @@ CONTENT_EXPORT int RunContentProcess(const ContentMainParams& params, ...@@ -79,10 +79,13 @@ CONTENT_EXPORT int RunContentProcess(const ContentMainParams& params,
// The ownership of |delegate| is transferred. // The ownership of |delegate| is transferred.
CONTENT_EXPORT void SetContentMainDelegate(ContentMainDelegate* delegate); CONTENT_EXPORT void SetContentMainDelegate(ContentMainDelegate* delegate);
#if defined(CONTENT_IMPLEMENTATION)
// In browser tests, ContentMain.java is not run either, and the browser test // In browser tests, ContentMain.java is not run either, and the browser test
// harness does not run ContentMain() at all. It does need to make use of the // harness does not run ContentMain() at all. It does need to make use of the
// delegate though while replacing ContentMain(). // delegate though while replacing ContentMain().
CONTENT_EXPORT ContentMainDelegate* GetContentMainDelegateForTesting(); ContentMainDelegate* GetContentMainDelegate();
#endif
#else #else
// ContentMain should be called from the embedder's main() function to do the // ContentMain should be called from the embedder's main() function to do the
// initial setup for every process. The embedder has a chance to customize // initial setup for every process. The embedder has a chance to customize
......
...@@ -56,7 +56,7 @@ class ContentUtilityClient; ...@@ -56,7 +56,7 @@ class ContentUtilityClient;
struct CdmInfo; struct CdmInfo;
struct PepperPluginInfo; struct PepperPluginInfo;
// Setter and getter for the client. The client should be set early, before any // Setter and getter for the client. The client should be set early, before any
// content code is called. // content code is called.
CONTENT_EXPORT void SetContentClient(ContentClient* client); CONTENT_EXPORT void SetContentClient(ContentClient* client);
......
...@@ -86,6 +86,7 @@ ...@@ -86,6 +86,7 @@
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
#include "base/android/task_scheduler/post_task_android.h" #include "base/android/task_scheduler/post_task_android.h"
#include "components/discardable_memory/service/discardable_shared_memory_manager.h" // nogncheck #include "components/discardable_memory/service/discardable_shared_memory_manager.h" // nogncheck
#include "content/app/content_main_runner_impl.h"
#include "content/app/mojo/mojo_init.h" #include "content/app/mojo/mojo_init.h"
#include "content/app/service_manager_environment.h" #include "content/app/service_manager_environment.h"
#include "content/public/app/content_main_delegate.h" #include "content/public/app/content_main_delegate.h"
...@@ -523,6 +524,12 @@ void BrowserTestBase::SetUp() { ...@@ -523,6 +524,12 @@ void BrowserTestBase::SetUp() {
InitializeBrowserMemoryInstrumentationClient(); InitializeBrowserMemoryInstrumentationClient();
} }
blink::TrialTokenValidator::SetOriginTrialPolicyGetter(
base::BindRepeating([]() -> blink::OriginTrialPolicy* {
ContentClient* client = GetContentClientForTesting();
return client ? client->GetOriginTrialPolicy() : nullptr;
}));
// All FeatureList overrides should have been registered prior to browser test // All FeatureList overrides should have been registered prior to browser test
// SetUp(). // SetUp().
base::FeatureList::ScopedDisallowOverrides disallow_feature_overrides( base::FeatureList::ScopedDisallowOverrides disallow_feature_overrides(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment