Commit 34881ada authored by Mohsen Izadi's avatar Mohsen Izadi Committed by Commit Bot

Update OzonePlatform::AfterSandboxEntry() according to spec

OzonePlatform::AfterSandboxEntry() should only be called when GPU is run
as a separate process (whether it is sandboxed or not). If GPU is
in-process, the embedders should not call AfterSandboxEntry(). This CL
updates ozone embedders and ozone implementations to adhere to this
requirement. Comments are also updated to reflect this requirement
clearly.

BUG=998113

Change-Id: I86a479131df463137185fc3217ce12dab107159e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807700Reviewed-by: default avatarRobert Kroeger <rjkroege@chromium.org>
Reviewed-by: default avatarDaniele Castagna <dcastagna@chromium.org>
Reviewed-by: default avatarJonathan Backer <backer@chromium.org>
Commit-Queue: Mohsen Izadi <mohsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699788}
parent 816f83e7
...@@ -508,7 +508,6 @@ void GpuInit::InitializeInProcess(base::CommandLine* command_line, ...@@ -508,7 +508,6 @@ void GpuInit::InitializeInProcess(base::CommandLine* command_line,
ui::OzonePlatform::GetInstance() ui::OzonePlatform::GetInstance()
->GetSurfaceFactoryOzone() ->GetSurfaceFactoryOzone()
->GetSupportedFormatsForTexturing(); ->GetSupportedFormatsForTexturing();
ui::OzonePlatform::GetInstance()->AfterSandboxEntry();
#endif #endif
bool needs_more_info = true; bool needs_more_info = true;
#if !defined(IS_CHROMECAST) #if !defined(IS_CHROMECAST)
......
...@@ -75,7 +75,6 @@ void RenderingHelper::InitializeOneOff(bool use_gl, base::WaitableEvent* done) { ...@@ -75,7 +75,6 @@ void RenderingHelper::InitializeOneOff(bool use_gl, base::WaitableEvent* done) {
ui::OzonePlatform::InitParams params; ui::OzonePlatform::InitParams params;
params.single_process = true; params.single_process = true;
ui::OzonePlatform::InitializeForGPU(params); ui::OzonePlatform::InitializeForGPU(params);
ui::OzonePlatform::GetInstance()->AfterSandboxEntry();
#endif #endif
if (!use_gl_) { if (!use_gl_) {
......
...@@ -824,7 +824,6 @@ class VideoEncodeAcceleratorTestEnvironment : public ::testing::Environment { ...@@ -824,7 +824,6 @@ class VideoEncodeAcceleratorTestEnvironment : public ::testing::Environment {
ui::OzonePlatform::InitParams params; ui::OzonePlatform::InitParams params;
params.single_process = true; params.single_process = true;
ui::OzonePlatform::InitializeForGPU(params); ui::OzonePlatform::InitializeForGPU(params);
ui::OzonePlatform::GetInstance()->AfterSandboxEntry();
done->Signal(); done->Signal();
} }
#endif #endif
......
...@@ -40,7 +40,6 @@ void InitializeOneOffHelper(bool init_extensions) { ...@@ -40,7 +40,6 @@ void InitializeOneOffHelper(bool init_extensions) {
ui::OzonePlatform::InitParams params; ui::OzonePlatform::InitParams params;
params.single_process = true; params.single_process = true;
ui::OzonePlatform::InitializeForGPU(params); ui::OzonePlatform::InitializeForGPU(params);
ui::OzonePlatform::GetInstance()->AfterSandboxEntry();
#endif #endif
#if defined(OS_LINUX) #if defined(OS_LINUX)
...@@ -119,7 +118,6 @@ void GLSurfaceTestSupport::InitializeOneOffWithMockBindings() { ...@@ -119,7 +118,6 @@ void GLSurfaceTestSupport::InitializeOneOffWithMockBindings() {
ui::OzonePlatform::InitParams params; ui::OzonePlatform::InitParams params;
params.single_process = true; params.single_process = true;
ui::OzonePlatform::InitializeForGPU(params); ui::OzonePlatform::InitializeForGPU(params);
ui::OzonePlatform::GetInstance()->AfterSandboxEntry();
#endif #endif
InitializeOneOffImplementation(kGLImplementationMockGL, false); InitializeOneOffImplementation(kGLImplementationMockGL, false);
...@@ -131,7 +129,6 @@ void GLSurfaceTestSupport::InitializeOneOffWithStubBindings() { ...@@ -131,7 +129,6 @@ void GLSurfaceTestSupport::InitializeOneOffWithStubBindings() {
ui::OzonePlatform::InitParams params; ui::OzonePlatform::InitParams params;
params.single_process = true; params.single_process = true;
ui::OzonePlatform::InitializeForGPU(params); ui::OzonePlatform::InitializeForGPU(params);
ui::OzonePlatform::GetInstance()->AfterSandboxEntry();
#endif #endif
InitializeOneOffImplementation(kGLImplementationStubGL, false); InitializeOneOffImplementation(kGLImplementationStubGL, false);
......
...@@ -81,7 +81,6 @@ int main(int argc, char** argv) { ...@@ -81,7 +81,6 @@ int main(int argc, char** argv) {
->SetCurrentLayoutByName("us"); ->SetCurrentLayoutByName("us");
ui::OzonePlatform::InitializeForGPU(params); ui::OzonePlatform::InitializeForGPU(params);
ui::OzonePlatform::GetInstance()->AfterSandboxEntry();
std::unique_ptr<ui::OzoneGpuTestHelper> gpu_helper; std::unique_ptr<ui::OzoneGpuTestHelper> gpu_helper;
if (!ui::OzonePlatform::GetInstance() if (!ui::OzonePlatform::GetInstance()
......
...@@ -56,7 +56,6 @@ int main(int argc, char** argv) { ...@@ -56,7 +56,6 @@ int main(int argc, char** argv) {
->SetCurrentLayoutByName("us"); ->SetCurrentLayoutByName("us");
ui::OzonePlatform::InitializeForGPU(params); ui::OzonePlatform::InitializeForGPU(params);
ui::OzonePlatform::GetInstance()->AfterSandboxEntry();
std::unique_ptr<ui::OzoneGpuTestHelper> gpu_helper; std::unique_ptr<ui::OzoneGpuTestHelper> gpu_helper;
if (!ui::OzonePlatform::GetInstance() if (!ui::OzonePlatform::GetInstance()
......
...@@ -184,7 +184,6 @@ class OzonePlatformGbm : public OzonePlatform { ...@@ -184,7 +184,6 @@ class OzonePlatformGbm : public OzonePlatform {
// 3. multi-process mode where host and viz components communicate // 3. multi-process mode where host and viz components communicate
// via mojo IPC. // via mojo IPC.
single_process_ = args.single_process;
using_mojo_ = args.using_mojo; using_mojo_ = args.using_mojo;
host_thread_ = base::PlatformThread::CurrentRef(); host_thread_ = base::PlatformThread::CurrentRef();
...@@ -263,14 +262,19 @@ class OzonePlatformGbm : public OzonePlatform { ...@@ -263,14 +262,19 @@ class OzonePlatformGbm : public OzonePlatform {
std::make_unique<DrmOverlayManagerGpu>(drm_thread_proxy_.get()); std::make_unique<DrmOverlayManagerGpu>(drm_thread_proxy_.get());
} }
if (using_mojo_ && single_process_ && // If gpu is in a separate process, rest of the initialization happens after
host_thread_ == base::PlatformThread::CurrentRef()) { // entering the sandbox.
CHECK(host_drm_device_) << "Mojo single-thread mode requires " if (!single_process())
"InitializeUI to be called first."; return;
// One-thread execution does not permit use of the sandbox. // In single process/mojo mode we need to make sure DrainBindingRequest is
AfterSandboxEntry(); // executed on this thread before we start the drm device.
const bool block_for_drm_thread = using_mojo_;
StartDrmThread(block_for_drm_thread);
if (using_mojo_ && host_thread_ == base::PlatformThread::CurrentRef()) {
CHECK(has_initialized_ui()) << "Mojo single-thread mode requires "
"InitializeUI to be called first.";
// Connect host and gpu here since OnGpuServiceLaunched() is not called in // Connect host and gpu here since OnGpuServiceLaunched() is not called in
// the single-threaded mode. // the single-threaded mode.
ui::ozone::mojom::DrmDevicePtr drm_device_ptr; ui::ozone::mojom::DrmDevicePtr drm_device_ptr;
...@@ -286,21 +290,28 @@ class OzonePlatformGbm : public OzonePlatform { ...@@ -286,21 +290,28 @@ class OzonePlatformGbm : public OzonePlatform {
} }
// The DRM thread needs to be started late because we need to wait for the // The DRM thread needs to be started late because we need to wait for the
// sandbox to start. This entry point in the Ozne API gives platforms // sandbox to start. This entry point in the Ozone API gives platforms
// flexibility in handing this requirement. // flexibility in handing this requirement.
void AfterSandboxEntry() override { void AfterSandboxEntry() override {
CHECK(drm_thread_proxy_) << "AfterSandboxEntry before InitializeForGPU is " DCHECK(!single_process());
"invalid startup order.\n"; CHECK(has_initialized_gpu()) << "AfterSandboxEntry before InitializeForGPU "
if (using_mojo_ && single_process_) { "is invalid startup order.";
// In single process/mojo mode we need to make sure DrainBindingRequest
// is executed on this thread before we start the drm device. const bool block_for_drm_thread = false;
StartDrmThread(block_for_drm_thread);
}
private:
// Starts the DRM thread. |blocking| determines if the call should be blocked
// until the thread is started.
void StartDrmThread(bool blocking) {
if (blocking) {
base::WaitableEvent done_event; base::WaitableEvent done_event;
drm_thread_proxy_->StartDrmThread(base::BindOnce( drm_thread_proxy_->StartDrmThread(base::BindOnce(
&base::WaitableEvent::Signal, base::Unretained(&done_event))); &base::WaitableEvent::Signal, base::Unretained(&done_event)));
done_event.Wait(); done_event.Wait();
DrainBindingRequests(); DrainBindingRequests();
} else { } else {
// Defer the actual startup of the DRM thread to here.
auto safe_binding_resquest_drainer = CreateSafeOnceCallback( auto safe_binding_resquest_drainer = CreateSafeOnceCallback(
base::BindOnce(&OzonePlatformGbm::DrainBindingRequests, base::BindOnce(&OzonePlatformGbm::DrainBindingRequests,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
...@@ -309,9 +320,7 @@ class OzonePlatformGbm : public OzonePlatform { ...@@ -309,9 +320,7 @@ class OzonePlatformGbm : public OzonePlatform {
} }
} }
private:
bool using_mojo_ = false; bool using_mojo_ = false;
bool single_process_ = false;
// Objects in the GPU process. // Objects in the GPU process.
std::unique_ptr<DrmThreadProxy> drm_thread_proxy_; std::unique_ptr<DrmThreadProxy> drm_thread_proxy_;
......
...@@ -18,12 +18,7 @@ ...@@ -18,12 +18,7 @@
namespace ui { namespace ui {
namespace { namespace {
bool g_platform_initialized_ui = false;
bool g_platform_initialized_gpu = false;
OzonePlatform* g_instance = nullptr; OzonePlatform* g_instance = nullptr;
} // namespace } // namespace
OzonePlatform::OzonePlatform() { OzonePlatform::OzonePlatform() {
...@@ -35,10 +30,12 @@ OzonePlatform::~OzonePlatform() = default; ...@@ -35,10 +30,12 @@ OzonePlatform::~OzonePlatform() = default;
// static // static
void OzonePlatform::InitializeForUI(const InitParams& args) { void OzonePlatform::InitializeForUI(const InitParams& args) {
if (g_platform_initialized_ui) EnsureInstance();
if (g_instance->initialized_ui_)
return; return;
g_platform_initialized_ui = true; g_instance->initialized_ui_ = true;
EnsureInstance()->InitializeUI(args); g_instance->single_process_ = args.single_process;
g_instance->InitializeUI(args);
// This is deliberately created after initializing so that the platform can // This is deliberately created after initializing so that the platform can
// create its own version of DDM. // create its own version of DDM.
DeviceDataManager::CreateInstance(); DeviceDataManager::CreateInstance();
...@@ -46,10 +43,12 @@ void OzonePlatform::InitializeForUI(const InitParams& args) { ...@@ -46,10 +43,12 @@ void OzonePlatform::InitializeForUI(const InitParams& args) {
// static // static
void OzonePlatform::InitializeForGPU(const InitParams& args) { void OzonePlatform::InitializeForGPU(const InitParams& args) {
if (g_platform_initialized_gpu) EnsureInstance();
if (g_instance->initialized_gpu_)
return; return;
g_platform_initialized_gpu = true; g_instance->initialized_gpu_ = true;
EnsureInstance()->InitializeGPU(args); g_instance->single_process_ = args.single_process;
g_instance->InitializeGPU(args);
} }
// static // static
...@@ -103,7 +102,7 @@ OzonePlatform::GetPlatformProperties() { ...@@ -103,7 +102,7 @@ OzonePlatform::GetPlatformProperties() {
const OzonePlatform::InitializedHostProperties& const OzonePlatform::InitializedHostProperties&
OzonePlatform::GetInitializedHostProperties() { OzonePlatform::GetInitializedHostProperties() {
DCHECK(g_platform_initialized_ui); DCHECK(initialized_ui_);
static InitializedHostProperties host_properties; static InitializedHostProperties host_properties;
return host_properties; return host_properties;
...@@ -111,11 +110,9 @@ OzonePlatform::GetInitializedHostProperties() { ...@@ -111,11 +110,9 @@ OzonePlatform::GetInitializedHostProperties() {
void OzonePlatform::AddInterfaces(service_manager::BinderRegistry* registry) {} void OzonePlatform::AddInterfaces(service_manager::BinderRegistry* registry) {}
void OzonePlatform::AfterSandboxEntry() {} void OzonePlatform::AfterSandboxEntry() {
// This should not be called in single-process mode.
// static DCHECK(!single_process_);
bool OzonePlatform::has_initialized_ui() {
return g_platform_initialized_ui;
} }
} // namespace ui } // namespace ui
...@@ -186,20 +186,31 @@ class COMPONENT_EXPORT(OZONE) OzonePlatform { ...@@ -186,20 +186,31 @@ class COMPONENT_EXPORT(OZONE) OzonePlatform {
// The GPU-specific portion of Ozone would typically run in a sandboxed // The GPU-specific portion of Ozone would typically run in a sandboxed
// process for additional security. Some startup might need to wait until // process for additional security. Some startup might need to wait until
// after the sandbox has been configured. The embedder should use this method // after the sandbox has been configured.
// to specify that the sandbox is configured and that GPU-side setup should // When the GPU is in a separate process, the embedder should call this method
// complete. A default do-nothing implementation is provided to permit // after it has configured (or failed to configure) the sandbox so that the
// platform implementations to ignore sandboxing and any associated launch // GPU-side setup is completed. If the GPU is in-process, there is no
// ordering issues. // sandboxing and the embedder should not call this method.
// A default do-nothing implementation is provided to permit platform
// implementations to ignore sandboxing and any associated launch ordering
// issues.
virtual void AfterSandboxEntry(); virtual void AfterSandboxEntry();
protected: protected:
static bool has_initialized_ui(); bool has_initialized_ui() const { return initialized_ui_; }
bool has_initialized_gpu() const { return initialized_gpu_; }
bool single_process() const { return single_process_; }
private: private:
virtual void InitializeUI(const InitParams& params) = 0; virtual void InitializeUI(const InitParams& params) = 0;
virtual void InitializeGPU(const InitParams& params) = 0; virtual void InitializeGPU(const InitParams& params) = 0;
bool initialized_ui_ = false;
bool initialized_gpu_ = false;
bool single_process_ = false;
DISALLOW_COPY_AND_ASSIGN(OzonePlatform); DISALLOW_COPY_AND_ASSIGN(OzonePlatform);
}; };
......
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