Skip to content

Commit

Permalink
viz: Register HWNDs on Windows.
Browse files Browse the repository at this point in the history
Add calls in VizProcessTransportFactory to register HWNDs. This code is
the same as in GpuProcessTransportFactory.

The registration of child HWND can happen later with viz and the display
compositor in the GPU process. As a result, VizProcessTransportFactory
can request calling SetParent() too early. Remember that the browser
requested calling SetParent() and when the child is registered call it
then.

InProcessCommandBuffer assumed it was in the browser process and
directly called ::SetParent(), which isn't allowed by the GPU sandbox.
Use the same mechanism used by GpuCommandBufferStub to forward the
SetParent() request to the browser. This requires providing the
GpuChannelManager to InProcessCommandBuffer.

Bug: 787099
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I2ff351bc90a3f0871149608ad5b6457a1a6cebbc
Reviewed-on: https://chromium-review.googlesource.com/779244
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521524}
  • Loading branch information
kylechar authored and Commit Bot committed Dec 4, 2017
1 parent 794e5a5 commit 8e7c5e4
Show file tree
Hide file tree
Showing 20 changed files with 168 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ AwRenderThreadContextProvider::AwRenderThreadContextProvider(
context_ = gpu::GLInProcessContext::CreateWithoutInit();
context_->Initialize(service, surface, surface->IsOffscreen(),
gpu::kNullSurfaceHandle, nullptr /* share_context */,
attributes, limits, nullptr, nullptr, nullptr);
attributes, limits, nullptr, nullptr, nullptr, nullptr);

context_->GetImplementation()->SetLostContextCallback(base::Bind(
&AwRenderThreadContextProvider::OnLostContext, base::Unretained(this)));
Expand Down
2 changes: 1 addition & 1 deletion cc/paint/oop_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class OopPixelTest : public testing::Test {
auto result = context_->Initialize(
nullptr, nullptr, is_offscreen, gpu::kNullSurfaceHandle, nullptr,
attribs, gpu::SharedMemoryLimits(), &gpu_memory_buffer_manager_,
&image_factory_, base::ThreadTaskRunnerHandle::Get());
&image_factory_, nullptr, base::ThreadTaskRunnerHandle::Get());

ASSERT_EQ(result, gpu::ContextResult::kSuccess);
ASSERT_TRUE(context_->GetCapabilities().supports_oop_raster);
Expand Down
2 changes: 1 addition & 1 deletion cc/paint/transfer_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class TransferCacheTest : public testing::Test {
auto result = context_->Initialize(
nullptr, nullptr, is_offscreen, gpu::kNullSurfaceHandle, nullptr,
attribs, gpu::SharedMemoryLimits(), &gpu_memory_buffer_manager_,
&image_factory_, base::ThreadTaskRunnerHandle::Get());
&image_factory_, nullptr, base::ThreadTaskRunnerHandle::Get());

ASSERT_EQ(result, gpu::ContextResult::kSuccess);
ASSERT_TRUE(context_->GetCapabilities().supports_oop_raster);
Expand Down
2 changes: 1 addition & 1 deletion cc/test/test_in_process_context_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ std::unique_ptr<gpu::GLInProcessContext> CreateTestInProcessContext(
auto result = context->Initialize(
nullptr, nullptr, is_offscreen, gpu::kNullSurfaceHandle, shared_context,
attribs, gpu::SharedMemoryLimits(), gpu_memory_buffer_manager,
image_factory, std::move(task_runner));
image_factory, nullptr, std::move(task_runner));

DCHECK_EQ(result, gpu::ContextResult::kSuccess);
return context;
Expand Down
20 changes: 11 additions & 9 deletions components/viz/common/gl_helper_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,17 @@ class GLHelperBenchmark : public testing::Test {
attributes.gpu_preference = gl::PreferDiscreteGpu;

context_ = gpu::GLInProcessContext::CreateWithoutInit();
auto result = context_->Initialize(nullptr, /* service */
nullptr, /* surface */
true, /* offscreen */
gpu::kNullSurfaceHandle, /* window */
nullptr, /* share_context */
attributes, gpu::SharedMemoryLimits(),
nullptr, /* gpu_memory_buffer_manager */
nullptr, /* image_factory */
base::ThreadTaskRunnerHandle::Get());
auto result =
context_->Initialize(nullptr, /* service */
nullptr, /* surface */
true, /* offscreen */
gpu::kNullSurfaceHandle, /* window */
nullptr, /* share_context */
attributes, gpu::SharedMemoryLimits(),
nullptr, /* gpu_memory_buffer_manager */
nullptr, /* image_factory */
nullptr /* gpu_channel_manager_delegate */,
base::ThreadTaskRunnerHandle::Get());
DCHECK_EQ(result, gpu::ContextResult::kSuccess);
gl_ = context_->GetImplementation();
gpu::ContextSupport* support = context_->GetImplementation();
Expand Down
20 changes: 11 additions & 9 deletions components/viz/common/gl_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,17 @@ class GLHelperTest : public testing::Test {
attributes.bind_generates_resource = false;

context_ = gpu::GLInProcessContext::CreateWithoutInit();
auto result = context_->Initialize(nullptr, /* service */
nullptr, /* surface */
true, /* offscreen */
gpu::kNullSurfaceHandle, /* window */
nullptr, /* share_context */
attributes, gpu::SharedMemoryLimits(),
nullptr, /* gpu_memory_buffer_manager */
nullptr, /* image_factory */
base::ThreadTaskRunnerHandle::Get());
auto result =
context_->Initialize(nullptr, /* service */
nullptr, /* surface */
true, /* offscreen */
gpu::kNullSurfaceHandle, /* window */
nullptr, /* share_context */
attributes, gpu::SharedMemoryLimits(),
nullptr, /* gpu_memory_buffer_manager */
nullptr, /* image_factory */
nullptr /* gpu_channel_manager_delegate */,
base::ThreadTaskRunnerHandle::Get());
DCHECK_EQ(result, gpu::ContextResult::kSuccess);
gl_ = context_->GetImplementation();
gpu::ContextSupport* support = context_->GetImplementation();
Expand Down
2 changes: 2 additions & 0 deletions components/viz/common/gpu/in_process_context_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ InProcessContextProvider::InProcessContextProvider(
gpu::SurfaceHandle surface_handle,
gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
gpu::ImageFactory* image_factory,
gpu::GpuChannelManagerDelegate* gpu_channel_manager_delegate,
const gpu::SharedMemoryLimits& limits,
InProcessContextProvider* shared_context)
: attributes_(CreateAttributes()),
Expand All @@ -68,6 +69,7 @@ InProcessContextProvider::InProcessContextProvider(
limits,
gpu_memory_buffer_manager,
image_factory,
gpu_channel_manager_delegate,
base::ThreadTaskRunnerHandle::Get())),
cache_controller_(std::make_unique<ContextCacheController>(
context_->GetImplementation(),
Expand Down
6 changes: 6 additions & 0 deletions components/viz/common/gpu/in_process_context_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class GrContext;

namespace gpu {
class GLInProcessContext;
class GpuChannelManagerDelegate;
class GpuMemoryBufferManager;
class ImageFactory;
struct SharedMemoryLimits;
Expand All @@ -33,13 +34,18 @@ class GrContextForGLES2Interface;

namespace viz {

// A ContextProvider used in the viz process to setup command buffers between
// the compositor and gpu thread.
// TODO(kylechar): Rename VizProcessContextProvider and move to
// components/viz/service.
class VIZ_COMMON_EXPORT InProcessContextProvider : public ContextProvider {
public:
InProcessContextProvider(
scoped_refptr<gpu::InProcessCommandBuffer::Service> service,
gpu::SurfaceHandle surface_handle,
gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
gpu::ImageFactory* image_factory,
gpu::GpuChannelManagerDelegate* gpu_channel_manager_delegate,
const gpu::SharedMemoryLimits& limits,
InProcessContextProvider* shared_context);

Expand Down
20 changes: 11 additions & 9 deletions components/viz/common/yuv_readback_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,17 @@ class YUVReadbackTest : public testing::Test {
attributes.bind_generates_resource = false;

context_ = gpu::GLInProcessContext::CreateWithoutInit();
auto result = context_->Initialize(nullptr, /* service */
nullptr, /* surface */
true, /* offscreen */
gpu::kNullSurfaceHandle, /* window */
nullptr, /* share_context */
attributes, gpu::SharedMemoryLimits(),
nullptr, /* gpu_memory_buffer_manager */
nullptr, /* image_factory */
base::ThreadTaskRunnerHandle::Get());
auto result =
context_->Initialize(nullptr, /* service */
nullptr, /* surface */
true, /* offscreen */
gpu::kNullSurfaceHandle, /* window */
nullptr, /* share_context */
attributes, gpu::SharedMemoryLimits(),
nullptr, /* gpu_memory_buffer_manager */
nullptr, /* image_factory */
nullptr /* gpu_channel_manager_delegate */,
base::ThreadTaskRunnerHandle::Get());
DCHECK_EQ(result, gpu::ContextResult::kSuccess);
gl_ = context_->GetImplementation();
gpu::ContextSupport* support = context_->GetImplementation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "gpu/command_buffer/service/image_factory.h"
#include "gpu/ipc/common/surface_handle.h"
#include "gpu/ipc/service/gpu_channel_manager.h"
#include "gpu/ipc/service/gpu_channel_manager_delegate.h"
#include "gpu/ipc/service/gpu_memory_buffer_factory.h"
#include "ui/base/ui_base_switches.h"

Expand Down Expand Up @@ -66,6 +67,7 @@ GpuDisplayProvider::GpuDisplayProvider(
CompositingModeReporterImpl* compositing_mode_reporter)
: restart_id_(restart_id),
gpu_service_(std::move(gpu_service)),
gpu_channel_manager_delegate_(gpu_channel_manager->delegate()),
gpu_memory_buffer_manager_(
base::MakeUnique<InProcessGpuMemoryBufferManager>(
gpu_channel_manager)),
Expand Down Expand Up @@ -113,8 +115,8 @@ std::unique_ptr<Display> GpuDisplayProvider::CreateDisplay(
} else {
auto context_provider = base::MakeRefCounted<InProcessContextProvider>(
gpu_service_, surface_handle, gpu_memory_buffer_manager_.get(),
image_factory_, gpu::SharedMemoryLimits(),
nullptr /* shared_context */);
image_factory_, gpu_channel_manager_delegate_,
gpu::SharedMemoryLimits(), nullptr /* shared_context */);

// TODO(rjkroege): If there is something better to do than CHECK, add it.
// TODO(danakj): Should retry if the result is kTransientFailure.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

namespace gpu {
class GpuChannelManager;
class GpuChannelManagerDelegate;
class ImageFactory;
} // namespace gpu

Expand Down Expand Up @@ -54,6 +55,7 @@ class VIZ_SERVICE_EXPORT GpuDisplayProvider : public DisplayProvider {

const uint32_t restart_id_;
scoped_refptr<gpu::InProcessCommandBuffer::Service> gpu_service_;
gpu::GpuChannelManagerDelegate* const gpu_channel_manager_delegate_;
std::unique_ptr<gpu::GpuMemoryBufferManager> gpu_memory_buffer_manager_;
gpu::ImageFactory* const image_factory_;
CompositingModeReporterImpl* const compositing_mode_reporter_;
Expand Down
26 changes: 26 additions & 0 deletions content/browser/compositor/viz_process_transport_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
#include "services/ui/public/cpp/gpu/context_provider_command_buffer.h"
#include "ui/compositor/reflector.h"

#if defined(OS_WIN)
#include "ui/gfx/win/rendering_window_manager.h"
#endif

namespace content {
namespace {

Expand Down Expand Up @@ -149,6 +153,11 @@ void VizProcessTransportFactory::ConnectHostFrameSinkManager() {

void VizProcessTransportFactory::CreateLayerTreeFrameSink(
base::WeakPtr<ui::Compositor> compositor) {
#if defined(OS_WIN)
gfx::RenderingWindowManager::GetInstance()->UnregisterParent(
compositor->widget());
#endif

if (is_gpu_compositing_disabled_ || compositor->force_software_compositor()) {
OnEstablishedGpuChannel(compositor, nullptr);
return;
Expand All @@ -165,6 +174,13 @@ VizProcessTransportFactory::SharedMainThreadContextProvider() {
}

void VizProcessTransportFactory::RemoveCompositor(ui::Compositor* compositor) {
#if defined(OS_WIN)
// TODO(crbug.com/791660): Make sure that GpuProcessHost::SetChildSurface()
// doesn't crash the GPU process after parent is unregistered.
gfx::RenderingWindowManager::GetInstance()->UnregisterParent(
compositor->widget());
#endif

compositor_data_map_.erase(compositor);
}

Expand Down Expand Up @@ -381,6 +397,11 @@ void VizProcessTransportFactory::OnEstablishedGpuChannel(
}
}

#if defined(OS_WIN)
gfx::RenderingWindowManager::GetInstance()->RegisterParent(
compositor->widget());
#endif

// TODO(crbug.com/776050): Deal with context loss.

// Create interfaces for a root CompositorFrameSink.
Expand Down Expand Up @@ -429,6 +450,11 @@ void VizProcessTransportFactory::OnEstablishedGpuChannel(
compositor->SetLayerTreeFrameSink(
std::make_unique<viz::ClientLayerTreeFrameSink>(
std::move(compositor_context), std::move(worker_context), &params));

#if defined(OS_WIN)
gfx::RenderingWindowManager::GetInstance()->DoSetParentOnChild(
compositor->widget());
#endif
}

bool VizProcessTransportFactory::CreateContextProviders(
Expand Down
19 changes: 10 additions & 9 deletions gpu/ipc/client/gpu_in_process_context_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,16 @@ class ContextTestBase : public testing::Test {
attributes.bind_generates_resource = false;

auto context = gpu::GLInProcessContext::CreateWithoutInit();
auto result = context->Initialize(nullptr, /* service */
nullptr, /* surface */
true, /* offscreen */
gpu::kNullSurfaceHandle, /* window */
nullptr, /* share_context */
attributes, gpu::SharedMemoryLimits(),
gpu_memory_buffer_manager_.get(),
nullptr, /* image_factory */
base::ThreadTaskRunnerHandle::Get());
auto result = context->Initialize(
nullptr, /* service */
nullptr, /* surface */
true, /* offscreen */
gpu::kNullSurfaceHandle, /* window */
nullptr, /* share_context */
attributes, gpu::SharedMemoryLimits(), gpu_memory_buffer_manager_.get(),
nullptr, /* image_factory */
nullptr /* gpu_channel_manager_delegate */,
base::ThreadTaskRunnerHandle::Get());
DCHECK_EQ(result, gpu::ContextResult::kSuccess);
return context;
}
Expand Down
5 changes: 4 additions & 1 deletion gpu/ipc/gl_in_process_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class GLInProcessContextImpl
const SharedMemoryLimits& mem_limits,
GpuMemoryBufferManager* gpu_memory_buffer_manager,
ImageFactory* image_factory,
GpuChannelManagerDelegate* gpu_channel_manager_delegate,
scoped_refptr<base::SingleThreadTaskRunner> task_runner) override;
const gpu::Capabilities& GetCapabilities() const override;
const gpu::GpuFeatureInfo& GetGpuFeatureInfo() const override;
Expand Down Expand Up @@ -157,6 +158,7 @@ gpu::ContextResult GLInProcessContextImpl::Initialize(
const SharedMemoryLimits& mem_limits,
GpuMemoryBufferManager* gpu_memory_buffer_manager,
ImageFactory* image_factory,
GpuChannelManagerDelegate* gpu_channel_manager_delegate,
scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
// If a surface is provided, we are running in a webview and should not have
// a task runner. We must have a task runner in all other cases.
Expand All @@ -183,7 +185,8 @@ gpu::ContextResult GLInProcessContextImpl::Initialize(

auto result = command_buffer_->Initialize(
surface, is_offscreen, window, attribs, share_command_buffer,
gpu_memory_buffer_manager, image_factory, std::move(task_runner));
gpu_memory_buffer_manager, image_factory, gpu_channel_manager_delegate,
std::move(task_runner));
if (result != gpu::ContextResult::kSuccess) {
DLOG(ERROR) << "Failed to initialize InProcessCommmandBuffer";
return result;
Expand Down
10 changes: 6 additions & 4 deletions gpu/ipc/gl_in_process_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
#include "base/callback.h"
#include "base/compiler_specific.h"
#include "base/single_thread_task_runner.h"
#include "gl_in_process_context_export.h"
#include "gpu/command_buffer/common/gles2_cmd_utils.h"
#include "gpu/ipc/gl_in_process_context_export.h"
#include "gpu/ipc/in_process_command_buffer.h"
#include "ui/gfx/native_widget_types.h"
#include "ui/gl/gl_surface.h"
Expand All @@ -36,13 +36,14 @@ class GL_IN_PROCESS_CONTEXT_EXPORT GLInProcessContext {
static std::unique_ptr<GLInProcessContext> CreateWithoutInit();

// Initialize the GLInProcessContext, if |is_offscreen| is true, renders to an
// offscreen context. |attrib_list| must be NULL or a NONE-terminated list
// offscreen context. |attrib_list| must be null or a NONE-terminated list
// of attribute/value pairs.
// If |surface| is not NULL, then it must match |is_offscreen|,
// If |surface| is not null, then it must match |is_offscreen|,
// |window| must be gfx::kNullAcceleratedWidget, and the command buffer
// service must run on the same thread as this client because GLSurface is
// not thread safe. If |surface| is NULL, then the other parameters are used
// not thread safe. If |surface| is null, then the other parameters are used
// to correctly create a surface.
// |gpu_channel_manager| should be non-null when used in the GPU process.
virtual gpu::ContextResult Initialize(
scoped_refptr<gpu::InProcessCommandBuffer::Service> service,
scoped_refptr<gl::GLSurface> surface,
Expand All @@ -53,6 +54,7 @@ class GL_IN_PROCESS_CONTEXT_EXPORT GLInProcessContext {
const SharedMemoryLimits& memory_limits,
GpuMemoryBufferManager* gpu_memory_buffer_manager,
ImageFactory* image_factory,
GpuChannelManagerDelegate* gpu_channel_manager_delegate,
scoped_refptr<base::SingleThreadTaskRunner> task_runner) = 0;

virtual const gpu::Capabilities& GetCapabilities() const = 0;
Expand Down
Loading

0 comments on commit 8e7c5e4

Please sign in to comment.