Skip to content

Commit

Permalink
Report fatal vs transient errors when initializing contexts
Browse files Browse the repository at this point in the history
Currently context creation and initialization reports back just true/false
for if the process succeeded. This expands that information to a tri-state
of success, fatal error, transient error. In the latter case, the client
should retry making the context as it may succeed next time. This happens
when, for instance, another client crashes the gpu process at the same
time the context is being constructed. In the case of a fatal error, the
context can not be made given the current inputs so either the gpu is not
usable, or the client has a bug and is requesting an invalid context of
some sort, but it will not make progress by retrying. So in this case the
client should not retry and just fail back to a non-gpu mode.

This information will allow us to remove the retry count when making the
compositor context for the display compositor, removing the state machine
in that code (as retry becomes a local decision from the result).

TBR=raymes

Bug: 772574
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: Ic3574de9fccae42ac34ff8a5755d4cfa5f0dd2b0
Reviewed-on: https://chromium-review.googlesource.com/717548
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509836}
  • Loading branch information
danakj authored and Commit Bot committed Oct 18, 2017
1 parent 44aafc1 commit 45cfd23
Show file tree
Hide file tree
Showing 80 changed files with 840 additions and 766 deletions.
20 changes: 10 additions & 10 deletions android_webview/browser/aw_render_thread_context_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ AwRenderThreadContextProvider::AwRenderThreadContextProvider(
limits.start_transfer_buffer_size = 64 * 1024;
limits.min_transfer_buffer_size = 64 * 1024;

context_.reset(gpu::GLInProcessContext::Create(
service, surface, surface->IsOffscreen(), gpu::kNullSurfaceHandle,
nullptr /* share_context */, attributes, limits, nullptr, nullptr,
nullptr));
context_ = gpu::GLInProcessContext::CreateWithoutInit();
context_->Initialize(service, surface, surface->IsOffscreen(),
gpu::kNullSurfaceHandle, nullptr /* share_context */,
attributes, limits, nullptr, nullptr, nullptr);

context_->GetImplementation()->SetLostContextCallback(base::Bind(
&AwRenderThreadContextProvider::OnLostContext, base::Unretained(this)));
Expand All @@ -76,12 +76,12 @@ AwRenderThreadContextProvider::AwRenderThreadContextProvider(
switches::kEnableGpuClientTracing)) {
// This wraps the real GLES2Implementation and we should always use this
// instead when it's present.
trace_impl_.reset(new gpu::gles2::GLES2TraceImplementation(
context_->GetImplementation()));
trace_impl_ = std::make_unique<gpu::gles2::GLES2TraceImplementation>(
context_->GetImplementation());
}

cache_controller_.reset(
new viz::ContextCacheController(context_->GetImplementation(), nullptr));
cache_controller_ = std::make_unique<viz::ContextCacheController>(
context_->GetImplementation(), nullptr);
}

AwRenderThreadContextProvider::~AwRenderThreadContextProvider() {
Expand All @@ -95,11 +95,11 @@ uint32_t AwRenderThreadContextProvider::GetCopyTextureInternalFormat() {
return GL_RGBA;
}

bool AwRenderThreadContextProvider::BindToCurrentThread() {
gpu::ContextResult AwRenderThreadContextProvider::BindToCurrentThread() {
// This is called on the thread the context will be used.
DCHECK(main_thread_checker_.CalledOnValidThread());

return true;
return gpu::ContextResult::kSuccess;
}

const gpu::Capabilities& AwRenderThreadContextProvider::ContextCapabilities()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class AwRenderThreadContextProvider : public viz::ContextProvider {
~AwRenderThreadContextProvider() override;

// viz::ContextProvider:
bool BindToCurrentThread() override;
gpu::ContextResult BindToCurrentThread() override;
const gpu::Capabilities& ContextCapabilities() const override;
const gpu::GpuFeatureInfo& GetGpuFeatureInfo() const override;
gpu::gles2::GLES2Interface* ContextGL() override;
Expand Down
4 changes: 3 additions & 1 deletion cc/raster/raster_buffer_provider_perftest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ class PerfContextProvider : public viz::ContextProvider {
capabilities_.sync_query = true;
}

bool BindToCurrentThread() override { return true; }
gpu::ContextResult BindToCurrentThread() override {
return gpu::ContextResult::kSuccess;
}
const gpu::Capabilities& ContextCapabilities() const override {
return capabilities_;
}
Expand Down
2 changes: 1 addition & 1 deletion cc/raster/scoped_gpu_raster_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class ScopedGpuRasterTest : public testing::Test {
// Releasing ScopedGpuRaster should restore GL_UNPACK_ALIGNMENT == 4.
TEST(ScopedGpuRasterTest, RestoresUnpackAlignment) {
scoped_refptr<TestContextProvider> provider = TestContextProvider::Create();
EXPECT_TRUE(provider->BindToCurrentThread());
ASSERT_EQ(provider->BindToCurrentThread(), gpu::ContextResult::kSuccess);
gpu::gles2::GLES2Interface* gl = provider->ContextGL();
GLint unpack_alignment = 0;
gl->GetIntegerv(GL_UNPACK_ALIGNMENT, &unpack_alignment);
Expand Down
9 changes: 6 additions & 3 deletions cc/resources/scoped_resource_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ namespace {
TEST(ScopedResourceTest, NewScopedResource) {
scoped_refptr<TestContextProvider> context_provider =
TestContextProvider::Create();
ASSERT_TRUE(context_provider->BindToCurrentThread());
ASSERT_EQ(context_provider->BindToCurrentThread(),
gpu::ContextResult::kSuccess);

std::unique_ptr<viz::SharedBitmapManager> shared_bitmap_manager(
new TestSharedBitmapManager());
Expand All @@ -38,7 +39,8 @@ TEST(ScopedResourceTest, NewScopedResource) {
TEST(ScopedResourceTest, CreateScopedResource) {
scoped_refptr<TestContextProvider> context_provider =
TestContextProvider::Create();
ASSERT_TRUE(context_provider->BindToCurrentThread());
ASSERT_EQ(context_provider->BindToCurrentThread(),
gpu::ContextResult::kSuccess);

std::unique_ptr<viz::SharedBitmapManager> shared_bitmap_manager(
new TestSharedBitmapManager());
Expand All @@ -62,7 +64,8 @@ TEST(ScopedResourceTest, CreateScopedResource) {
TEST(ScopedResourceTest, ScopedResourceIsDeleted) {
scoped_refptr<TestContextProvider> context_provider =
TestContextProvider::Create();
ASSERT_TRUE(context_provider->BindToCurrentThread());
ASSERT_EQ(context_provider->BindToCurrentThread(),
gpu::ContextResult::kSuccess);

std::unique_ptr<viz::SharedBitmapManager> shared_bitmap_manager(
new TestSharedBitmapManager());
Expand Down
21 changes: 9 additions & 12 deletions cc/test/test_context_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ scoped_refptr<TestContextProvider> TestContextProvider::CreateWorker() {
std::make_unique<TestGLES2InterfaceForContextProvider>(),
TestWebGraphicsContext3D::Create()));
// Worker contexts are bound to the thread they are created on.
if (!worker_context_provider->BindToCurrentThread())
auto result = worker_context_provider->BindToCurrentThread();
if (result != gpu::ContextResult::kSuccess)
return nullptr;
return worker_context_provider;
}
Expand Down Expand Up @@ -168,23 +169,19 @@ TestContextProvider::~TestContextProvider() {
context_thread_checker_.CalledOnValidThread());
}

bool TestContextProvider::BindToCurrentThread() {
gpu::ContextResult TestContextProvider::BindToCurrentThread() {
// This is called on the thread the context will be used.
DCHECK(context_thread_checker_.CalledOnValidThread());

if (bound_)
return true;
if (!bound_) {
if (context_gl_->GetGraphicsResetStatusKHR() != GL_NO_ERROR)
return gpu::ContextResult::kTransientFailure;

if (context_gl_->GetGraphicsResetStatusKHR() != GL_NO_ERROR) {
return false;
context3d_->set_context_lost_callback(base::Bind(
&TestContextProvider::OnLostContext, base::Unretained(this)));
}
bound_ = true;

context3d_->set_context_lost_callback(
base::Bind(&TestContextProvider::OnLostContext,
base::Unretained(this)));

return true;
return gpu::ContextResult::kSuccess;
}

void TestContextProvider::DetachFromThread() {
Expand Down
2 changes: 1 addition & 1 deletion cc/test/test_context_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class TestContextProvider : public viz::ContextProvider {
static scoped_refptr<TestContextProvider> Create(
std::unique_ptr<TestGLES2Interface> gl);

bool BindToCurrentThread() override;
gpu::ContextResult BindToCurrentThread() override;
void DetachFromThread() override;
const gpu::Capabilities& ContextCapabilities() const override;
const gpu::GpuFeatureInfo& GetGpuFeatureInfo() const override;
Expand Down
16 changes: 8 additions & 8 deletions cc/test/test_in_process_context_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ std::unique_ptr<gpu::GLInProcessContext> CreateTestInProcessContext(
attribs.fail_if_major_perf_caveat = false;
attribs.bind_generates_resource = false;

std::unique_ptr<gpu::GLInProcessContext> context =
base::WrapUnique(gpu::GLInProcessContext::Create(
nullptr, nullptr, is_offscreen, gpu::kNullSurfaceHandle,
shared_context, attribs, gpu::SharedMemoryLimits(),
gpu_memory_buffer_manager, image_factory, std::move(task_runner)));
auto context = gpu::GLInProcessContext::CreateWithoutInit();
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));

DCHECK(context);
DCHECK_EQ(result, gpu::ContextResult::kSuccess);
return context;
}

Expand Down Expand Up @@ -83,8 +83,8 @@ TestInProcessContextProvider::TestInProcessContextProvider(
TestInProcessContextProvider::~TestInProcessContextProvider() {
}

bool TestInProcessContextProvider::BindToCurrentThread() {
return true;
gpu::ContextResult TestInProcessContextProvider::BindToCurrentThread() {
return gpu::ContextResult::kSuccess;
}

gpu::gles2::GLES2Interface* TestInProcessContextProvider::ContextGL() {
Expand Down
2 changes: 1 addition & 1 deletion cc/test/test_in_process_context_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class TestInProcessContextProvider : public viz::ContextProvider {
explicit TestInProcessContextProvider(
TestInProcessContextProvider* shared_context);

bool BindToCurrentThread() override;
gpu::ContextResult BindToCurrentThread() override;
gpu::gles2::GLES2Interface* ContextGL() override;
gpu::ContextSupport* ContextSupport() override;
class GrContext* GrContext() override;
Expand Down
3 changes: 2 additions & 1 deletion cc/tiles/picture_layer_tiling_set_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ class PictureLayerTilingSetTestWithResources : public testing::Test {
float expected_scale) {
scoped_refptr<TestContextProvider> context_provider =
TestContextProvider::Create();
ASSERT_TRUE(context_provider->BindToCurrentThread());
ASSERT_EQ(context_provider->BindToCurrentThread(),
gpu::ContextResult::kSuccess);
auto shared_bitmap_manager = std::make_unique<TestSharedBitmapManager>();
std::unique_ptr<ResourceProvider> resource_provider =
FakeResourceProvider::CreateLayerTreeResourceProvider(
Expand Down
3 changes: 2 additions & 1 deletion cc/trees/layer_tree_frame_sink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ bool LayerTreeFrameSink::BindToClient(LayerTreeFrameSinkClient* client) {
bool success = true;

if (context_provider_.get()) {
success = context_provider_->BindToCurrentThread();
auto result = context_provider_->BindToCurrentThread();
success = result == gpu::ContextResult::kSuccess;
if (success) {
context_provider_->SetLostContextCallback(
base::Bind(&LayerTreeFrameSink::DidLoseLayerTreeFrameSink,
Expand Down
3 changes: 2 additions & 1 deletion cc/trees/layer_tree_host_unittest_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,8 @@ class LayerTreeHostContextTestDontUseLostResources
context_should_support_io_surface_ = true;

child_context_provider_ = TestContextProvider::Create();
CHECK(child_context_provider_->BindToCurrentThread());
auto result = child_context_provider_->BindToCurrentThread();
CHECK_EQ(result, gpu::ContextResult::kSuccess);
shared_bitmap_manager_.reset(new TestSharedBitmapManager);
child_resource_provider_ =
FakeResourceProvider::CreateLayerTreeResourceProvider(
Expand Down
3 changes: 2 additions & 1 deletion cc/trees/layer_tree_host_unittest_copyrequest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,8 @@ class LayerTreeHostCopyRequestTestProvideTexture
protected:
void BeginTest() override {
external_context_provider_ = TestContextProvider::Create();
EXPECT_TRUE(external_context_provider_->BindToCurrentThread());
EXPECT_EQ(external_context_provider_->BindToCurrentThread(),
gpu::ContextResult::kSuccess);
LayerTreeHostCopyRequestTestCountTextures::BeginTest();
}

Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/android/vr_shell/mailbox_to_surface_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ void MailboxToSurfaceBridge::OnContextAvailable(
// otherwise the GL context created from it becomes invalid.
context_provider_ = std::move(provider);

if (!context_provider_->BindToCurrentThread()) {
auto result = context_provider_->BindToCurrentThread();
if (result != gpu::ContextResult::kSuccess) {
DLOG(ERROR) << "Failed to init viz::ContextProvider";
return;
}
Expand Down
21 changes: 11 additions & 10 deletions components/viz/common/gl_helper_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,17 @@ class GLHelperBenchmark : public testing::Test {
attributes.bind_generates_resource = false;
attributes.gpu_preference = gl::PreferDiscreteGpu;

context_.reset(
gpu::GLInProcessContext::Create(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()));
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());
DCHECK_EQ(result, gpu::ContextResult::kSuccess);
gl_ = context_->GetImplementation();
gpu::ContextSupport* support = context_->GetImplementation();

Expand Down
21 changes: 11 additions & 10 deletions components/viz/common/gl_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,17 @@ class GLHelperTest : public testing::Test {
attributes.sample_buffers = 1;
attributes.bind_generates_resource = false;

context_.reset(
gpu::GLInProcessContext::Create(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()));
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());
DCHECK_EQ(result, gpu::ContextResult::kSuccess);
gl_ = context_->GetImplementation();
gpu::ContextSupport* support = context_->GetImplementation();

Expand Down
3 changes: 2 additions & 1 deletion components/viz/common/gpu/context_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "components/viz/common/gpu/context_cache_controller.h"
#include "components/viz/common/viz_common_export.h"
#include "gpu/command_buffer/common/capabilities.h"
#include "gpu/command_buffer/common/context_result.h"

class GrContext;

Expand Down Expand Up @@ -60,7 +61,7 @@ class VIZ_COMMON_EXPORT ContextProvider
// from the same thread unless the function has some explicitly specified
// rules for access on a different thread. See SetupLockOnMainThread(), which
// can be used to provide access from multiple threads.
virtual bool BindToCurrentThread() = 0;
virtual gpu::ContextResult BindToCurrentThread() = 0;

virtual gpu::gles2::GLES2Interface* ContextGL() = 0;
virtual gpu::ContextSupport* ContextSupport() = 0;
Expand Down
39 changes: 14 additions & 25 deletions components/viz/common/gpu/in_process_context_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,6 @@ gpu::gles2::ContextCreationAttribHelper CreateAttributes() {
return attributes;
}

std::unique_ptr<gpu::GLInProcessContext> CreateInProcessContext(
scoped_refptr<gpu::InProcessCommandBuffer::Service> service,
const gpu::gles2::ContextCreationAttribHelper& attributes,
gpu::SurfaceHandle widget,
gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
gpu::ImageFactory* image_factory,
const gpu::SharedMemoryLimits& limits,
gpu::GLInProcessContext* shared_context) {
const bool is_offscreen = widget == gpu::kNullSurfaceHandle;
return base::WrapUnique(gpu::GLInProcessContext::Create(
service, nullptr, is_offscreen, widget, shared_context, attributes,
limits, gpu_memory_buffer_manager, image_factory,
base::ThreadTaskRunnerHandle::Get()));
}

} // namespace

InProcessContextProvider::InProcessContextProvider(
Expand All @@ -73,22 +58,26 @@ InProcessContextProvider::InProcessContextProvider(
const gpu::SharedMemoryLimits& limits,
InProcessContextProvider* shared_context)
: attributes_(CreateAttributes()),
context_(CreateInProcessContext(
service,
attributes_,
context_(gpu::GLInProcessContext::CreateWithoutInit()),
context_result_(context_->Initialize(
std::move(service),
nullptr,
(widget == gpu::kNullSurfaceHandle),
widget,
(shared_context ? shared_context->context_.get() : nullptr),
attributes_,
limits,
gpu_memory_buffer_manager,
image_factory,
limits,
(shared_context ? shared_context->context_.get() : nullptr))) {
cache_controller_.reset(new ContextCacheController(
context_->GetImplementation(), base::ThreadTaskRunnerHandle::Get()));
}
base::ThreadTaskRunnerHandle::Get())),
cache_controller_(std::make_unique<ContextCacheController>(
context_->GetImplementation(),
base::ThreadTaskRunnerHandle::Get())) {}

InProcessContextProvider::~InProcessContextProvider() = default;

bool InProcessContextProvider::BindToCurrentThread() {
return !!context_;
gpu::ContextResult InProcessContextProvider::BindToCurrentThread() {
return context_result_;
}

gpu::gles2::GLES2Interface* InProcessContextProvider::ContextGL() {
Expand Down
3 changes: 2 additions & 1 deletion components/viz/common/gpu/in_process_context_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class VIZ_COMMON_EXPORT InProcessContextProvider : public ContextProvider {
const gpu::SharedMemoryLimits& limits,
InProcessContextProvider* shared_context);

bool BindToCurrentThread() override;
gpu::ContextResult BindToCurrentThread() override;
gpu::gles2::GLES2Interface* ContextGL() override;
gpu::ContextSupport* ContextSupport() override;
class GrContext* GrContext() override;
Expand Down Expand Up @@ -74,6 +74,7 @@ class VIZ_COMMON_EXPORT InProcessContextProvider : public ContextProvider {

base::Lock context_lock_;
std::unique_ptr<gpu::GLInProcessContext> context_;
gpu::ContextResult context_result_;
std::unique_ptr<skia_bindings::GrContextForGLES2Interface> gr_context_;
std::unique_ptr<ContextCacheController> cache_controller_;
};
Expand Down
Loading

0 comments on commit 45cfd23

Please sign in to comment.