Skip to content

Commit

Permalink
CHR-6375: [Windows] Fixed crash on fallback from ANGLE to SwiftShader.
Browse files Browse the repository at this point in the history
During fallback from ANGLE to SwiftShader it is required to unload
ANGLE libraries, otherwise SwiftShader will fail to load its own
libGLESv2.dll.

Fixed ANGLE platform reset.

Fixed order in ShutdownGL.

Bug: 760063
Cq-Include-Trybots: 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: If8797b49f2b5d04cb610302f96c82011dee9b3a9
Reviewed-on: https://chromium-review.googlesource.com/774296
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520730}
  • Loading branch information
Michał Pichliński authored and Commit Bot committed Nov 30, 2017
1 parent b34d4f9 commit aeb6e40
Show file tree
Hide file tree
Showing 18 changed files with 40 additions and 23 deletions.
4 changes: 2 additions & 2 deletions gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ void GLES2DecoderTestBase::ResetDecoder() {
command_buffer_service_.reset();
::gl::MockGLInterface::SetGLInterface(NULL);
gl_.reset();
gl::init::ShutdownGL();
gl::init::ShutdownGL(false);
}

void GLES2DecoderTestBase::TearDown() {
Expand Down Expand Up @@ -2341,7 +2341,7 @@ void GLES2DecoderPassthroughTestBase::TearDown() {
decoder_.reset();
group_ = nullptr;
command_buffer_service_.reset();
gl::init::ShutdownGL();
gl::init::ShutdownGL(false);
}

void GLES2DecoderPassthroughTestBase::SetBucketData(uint32_t bucket_id,
Expand Down
2 changes: 1 addition & 1 deletion gpu/command_buffer/service/gpu_service_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ void GpuServiceTest::TearDown() {
surface_ = nullptr;
::gl::MockGLInterface::SetGLInterface(NULL);
gl_.reset();
gl::init::ShutdownGL();
gl::init::ShutdownGL(false);
ran_teardown_ = true;

testing::Test::TearDown();
Expand Down
2 changes: 1 addition & 1 deletion gpu/config/gpu_info_collector_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ class GPUInfoCollectorTest
void TearDown() override {
::gl::MockGLInterface::SetGLInterface(NULL);
gl_.reset();
gl::init::ShutdownGL();
gl::init::ShutdownGL(false);

testing::Test::TearDown();
}
Expand Down
2 changes: 1 addition & 1 deletion gpu/ipc/client/gpu_memory_buffer_impl_test_template.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class GpuMemoryBufferImplTest : public testing::Test {
#if defined(OS_WIN)
// Overridden from testing::Test:
void SetUp() override { gl::GLSurfaceTestSupport::InitializeOneOff(); }
void TearDown() override { gl::init::ShutdownGL(); }
void TearDown() override { gl::init::ShutdownGL(false); }
#endif

private:
Expand Down
2 changes: 1 addition & 1 deletion gpu/ipc/service/gpu_channel_test_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ GpuChannelTestCommon::~GpuChannelTestCommon() {
task_runner_->ClearPendingTasks();
io_task_runner_->ClearPendingTasks();

gl::init::ShutdownGL();
gl::init::ShutdownGL(false);
}

GpuChannel* GpuChannelTestCommon::CreateChannel(int32_t client_id,
Expand Down
2 changes: 1 addition & 1 deletion gpu/ipc/service/gpu_memory_buffer_factory_test_template.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class GpuMemoryBufferFactoryTest : public testing::Test {
switches::kUseGpuInTests));
gl::GLSurfaceTestSupport::InitializeOneOff();
}
void TearDown() override { gl::init::ShutdownGL(); }
void TearDown() override { gl::init::ShutdownGL(false); }
#endif

protected:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class AndroidVideoDecodeAcceleratorTest : public testing::Test {
codec_allocator_ = nullptr;
context_ = nullptr;
surface_ = nullptr;
gl::init::ShutdownGL();
gl::init::ShutdownGL(false);
}

// Create and initialize AVDA with |config_|, and return the result.
Expand Down
2 changes: 1 addition & 1 deletion media/gpu/android/codec_image_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class CodecImageTest : public testing::Test {
context_ = nullptr;
share_group_ = nullptr;
surface_ = nullptr;
gl::init::ShutdownGL();
gl::init::ShutdownGL(false);
wrapper_->TakeCodecSurfacePair();
}

Expand Down
2 changes: 1 addition & 1 deletion media/gpu/android/surface_texture_gl_owner_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class SurfaceTextureGLOwnerTest : public testing::Test {
context_ = nullptr;
share_group_ = nullptr;
surface_ = nullptr;
gl::init::ShutdownGL();
gl::init::ShutdownGL(false);
}

scoped_refptr<SurfaceTextureGLOwner> surface_texture_;
Expand Down
2 changes: 1 addition & 1 deletion services/ui/ws/gpu_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ void GpuHostTest::SetUp() {

void GpuHostTest::TearDown() {
gpu_host_ = nullptr;
gl::init::ShutdownGL();
gl::init::ShutdownGL(false);
testing::Test::TearDown();
}

Expand Down
4 changes: 4 additions & 0 deletions ui/gl/angle_platform_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ void ResetPlatform(EGLDisplay display) {
base::UnsanitizedCfiCall(g_angle_reset_platform)(
static_cast<EGLDisplayType>(display));
ResetCacheProgramCallback();
{
auto writer = base::AutoWritableMemory::Create(g_angle_get_platform);
*g_angle_reset_platform = nullptr;
}
}

void SetCacheProgramCallback(CacheProgramCallback callback) {
Expand Down
19 changes: 16 additions & 3 deletions ui/gl/gl_implementation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,24 @@ LibraryArray* g_libraries;
PROTECTED_MEMORY_SECTION base::ProtectedMemory<GLGetProcAddressProc>
g_get_proc_address;

void CleanupNativeLibraries(void* unused) {
void CleanupNativeLibraries(void* due_to_fallback) {
if (g_libraries) {
// We do not call base::UnloadNativeLibrary() for these libraries as
// unloading libGL without closing X display is not allowed. See
// crbug.com/250813 for details.
bool unload_libraries = false;
#if defined(OS_WIN)
// However during fallback from ANGLE to SwiftShader ANGLE library needs to
// be unloaded, otherwise software SwiftShader loading will fail. See
// crbug.com/760063 for details.
unload_libraries = due_to_fallback &&
*static_cast<bool*>(due_to_fallback) &&
GetGLImplementation() == kGLImplementationEGLGLES2;
#endif
if (unload_libraries) {
for (auto* library : *g_libraries)
base::UnloadNativeLibrary(library);
}
delete g_libraries;
g_libraries = NULL;
}
Expand Down Expand Up @@ -151,8 +164,8 @@ void AddGLNativeLibrary(base::NativeLibrary library) {
g_libraries->push_back(library);
}

void UnloadGLNativeLibraries() {
CleanupNativeLibraries(NULL);
void UnloadGLNativeLibraries(bool due_to_fallback) {
CleanupNativeLibraries(&due_to_fallback);
}

void SetGLGetProcAddressProc(GLGetProcAddressProc proc) {
Expand Down
2 changes: 1 addition & 1 deletion ui/gl/gl_implementation.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ GL_EXPORT const char* GetGLImplementationName(GLImplementation implementation);
GL_EXPORT void AddGLNativeLibrary(base::NativeLibrary library);

// Unloads all native libraries.
GL_EXPORT void UnloadGLNativeLibraries();
GL_EXPORT void UnloadGLNativeLibraries(bool due_to_fallback);

// Set an additional function that will be called to find GL entry points.
// Exported so that tests may set the function used in the mock implementation.
Expand Down
2 changes: 1 addition & 1 deletion ui/gl/gpu_timing_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class GPUTimingTest : public testing::Test {
surface_ = nullptr;
if (setup_) {
MockGLInterface::SetGLInterface(NULL);
init::ShutdownGL();
init::ShutdownGL(false);
}
setup_ = false;
cpu_time_bounded_ = false;
Expand Down
8 changes: 4 additions & 4 deletions ui/gl/init/gl_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ bool InitializeGLOneOffImplementation(GLImplementation impl,
bool initialized =
InitializeStaticGLBindings(impl) && InitializeGLOneOffPlatform();
if (!initialized && fallback_to_software_gl) {
ShutdownGL();
ShutdownGL(true);
initialized = InitializeStaticGLBindings(GetSoftwareGLImplementation()) &&
InitializeGLOneOffPlatform();
}
Expand All @@ -89,7 +89,7 @@ bool InitializeGLOneOffImplementation(GLImplementation impl,
}

if (!initialized)
ShutdownGL();
ShutdownGL(false);

if (initialized) {
DVLOG(1) << "Using " << GetGLImplementationName(GetGLImplementation())
Expand All @@ -102,11 +102,11 @@ bool InitializeGLOneOffImplementation(GLImplementation impl,
return initialized;
}

void ShutdownGL() {
void ShutdownGL(bool due_to_fallback) {
ShutdownGLPlatform();

UnloadGLNativeLibraries(due_to_fallback);
SetGLImplementation(kGLImplementationNone);
UnloadGLNativeLibraries();
}

scoped_refptr<GLSurface> CreateOffscreenGLSurface(const gfx::Size& size) {
Expand Down
2 changes: 1 addition & 1 deletion ui/gl/init/gl_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ GL_INIT_EXPORT bool InitializeGLOneOffImplementation(
bool init_extensions);

// Clears GL bindings and resets GL implementation.
GL_INIT_EXPORT void ShutdownGL();
GL_INIT_EXPORT void ShutdownGL(bool due_to_fallback);

// Return information about the GL window system binding implementation (e.g.,
// EGL, GLX, WGL). Returns true if the information was retrieved successfully.
Expand Down
2 changes: 1 addition & 1 deletion ui/gl/test/gl_image_test_support.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void GLImageTestSupport::InitializeGL() {

// static
void GLImageTestSupport::CleanupGL() {
init::ShutdownGL();
init::ShutdownGL(false);
}

// static
Expand Down
2 changes: 1 addition & 1 deletion ui/gl/test/gl_surface_test_support.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ void GLSurfaceTestSupport::InitializeOneOffImplementation(

// This method may be called multiple times in the same process to set up
// bindings in different ways.
init::ShutdownGL();
init::ShutdownGL(false);

bool gpu_service_logging = false;
bool disable_gl_drawing = false;
Expand Down

0 comments on commit aeb6e40

Please sign in to comment.