From ddb7905e511f46265c51b10e32733a5f8af9c8e2 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Thu, 15 Feb 2018 09:26:05 -0500 Subject: [PATCH] Bug 1437886 - Prevent shared surfaces from being used without WebRender. r=nical Move the initialization of SharedSurfacesParent from the compositor thread creation to mirror the other WebRender-specific components, such as the render thread creation. Now it will only be created if WebRender is in use. Also prevent shared surfaces from being used by the image frame allocator, even if image.mem.shared is set -- there is no purpose in allowing this at present. It was causing startup crashes for users who requested image.mem.shared and/or WebRender via gfx.webrender.all but did not actually get WebRender at all. Surfaces would get allocated in the shared memory, try to register themselves with the WR render thread, and then crash since that thread was never created. --- gfx/ipc/GPUParent.cpp | 3 +++ gfx/layers/ipc/CompositorThread.cpp | 3 --- gfx/layers/ipc/SharedSurfacesChild.cpp | 13 +++++++++---- gfx/thebes/gfxPlatform.cpp | 3 +++ image/imgFrame.cpp | 2 +- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/gfx/ipc/GPUParent.cpp b/gfx/ipc/GPUParent.cpp index 9794f1a3645f..e239bf7b2a46 100644 --- a/gfx/ipc/GPUParent.cpp +++ b/gfx/ipc/GPUParent.cpp @@ -30,6 +30,7 @@ #include "mozilla/layers/LayerTreeOwnerTracker.h" #include "mozilla/layers/UiCompositorControllerParent.h" #include "mozilla/layers/MemoryReportingMLGPU.h" +#include "mozilla/layers/SharedSurfacesParent.h" #include "mozilla/webrender/RenderThread.h" #include "mozilla/webrender/WebRenderAPI.h" #include "mozilla/HangDetails.h" @@ -234,6 +235,7 @@ GPUParent::RecvInit(nsTArray&& prefs, wr::WebRenderAPI::InitExternalLogHandler(); wr::RenderThread::Start(); + SharedSurfacesParent::Initialize(); } VRManager::ManagerInit(); @@ -463,6 +465,7 @@ GPUParent::ActorDestroy(ActorDestroyReason aWhy) CompositorThreadHolder::Shutdown(); VRListenerThreadHolder::Shutdown(); if (gfxVars::UseWebRender()) { + SharedSurfacesParent::Shutdown(); wr::RenderThread::ShutDown(); wr::WebRenderAPI::ShutdownExternalLogHandler(); diff --git a/gfx/layers/ipc/CompositorThread.cpp b/gfx/layers/ipc/CompositorThread.cpp index c90293ab8b4b..a3b1a67fd190 100644 --- a/gfx/layers/ipc/CompositorThread.cpp +++ b/gfx/layers/ipc/CompositorThread.cpp @@ -8,7 +8,6 @@ #include "nsThreadUtils.h" #include "CompositorBridgeParent.h" #include "mozilla/layers/ImageBridgeParent.h" -#include "mozilla/layers/SharedSurfacesParent.h" #include "mozilla/media/MediaSystemResourceService.h" namespace mozilla { @@ -70,7 +69,6 @@ CompositorThreadHolder::DestroyCompositorThread(base::Thread* aCompositorThread) MOZ_ASSERT(!sCompositorThreadHolder, "We shouldn't be destroying the compositor thread yet."); CompositorBridgeParent::Shutdown(); - SharedSurfacesParent::Shutdown(); delete aCompositorThread; sFinishedCompositorShutDown = true; } @@ -105,7 +103,6 @@ CompositorThreadHolder::CreateCompositorThread() return nullptr; } - SharedSurfacesParent::Initialize(); CompositorBridgeParent::Setup(); ImageBridgeParent::Setup(); diff --git a/gfx/layers/ipc/SharedSurfacesChild.cpp b/gfx/layers/ipc/SharedSurfacesChild.cpp index d42f8d94d229..25da1a578ecf 100644 --- a/gfx/layers/ipc/SharedSurfacesChild.cpp +++ b/gfx/layers/ipc/SharedSurfacesChild.cpp @@ -7,6 +7,7 @@ #include "SharedSurfacesChild.h" #include "SharedSurfacesParent.h" #include "CompositorManagerChild.h" +#include "mozilla/gfx/gfxVars.h" #include "mozilla/layers/IpcResourceUpdateQueue.h" #include "mozilla/layers/SourceSurfaceSharedData.h" #include "mozilla/layers/WebRenderBridgeChild.h" @@ -192,7 +193,7 @@ SharedSurfacesChild::ShareInternal(SourceSurfaceSharedData* aSurface, MOZ_ASSERT(aUserData); CompositorManagerChild* manager = CompositorManagerChild::GetInstance(); - if (NS_WARN_IF(!manager || !manager->CanSend())) { + if (NS_WARN_IF(!manager || !manager->CanSend() || !gfxVars::UseWebRender())) { // We cannot try to share the surface, most likely because the GPU process // crashed. Ideally, we would retry when it is ready, but the handles may be // a scarce resource, which can cause much more serious problems if we run @@ -391,9 +392,13 @@ SharedSurfacesChild::Unshare(const wr::ExternalImageId& aId, if (manager->OtherPid() == base::GetCurrentProcId()) { // We are in the combined UI/GPU process. Call directly to it to remove its - // wrapper surface to free the underlying buffer. - MOZ_ASSERT(manager->OwnsExternalImageId(aId)); - SharedSurfacesParent::RemoveSameProcess(aId); + // wrapper surface to free the underlying buffer, but only if the external + // image ID is owned by the manager. It can be different if the surface was + // last shared with the GPU process, which crashed several times, and its + // job was moved into the parent process. + if (manager->OwnsExternalImageId(aId)) { + SharedSurfacesParent::RemoveSameProcess(aId); + } } else if (manager->OwnsExternalImageId(aId)) { // Only attempt to release current mappings in the GPU process. It is // possible we had a surface that was previously shared, the GPU process diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 998586a0feb3..bf8dbdadb6c5 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -7,6 +7,7 @@ #include "mozilla/layers/CompositorThread.h" #include "mozilla/layers/ImageBridgeChild.h" #include "mozilla/layers/ISurfaceAllocator.h" // for GfxMemoryImageReporter +#include "mozilla/layers/SharedSurfacesParent.h" #include "mozilla/webrender/RenderThread.h" #include "mozilla/webrender/WebRenderAPI.h" #include "mozilla/webrender/webrender_ffi.h" @@ -1019,6 +1020,7 @@ gfxPlatform::InitLayersIPC() } else if (XRE_IsParentProcess()) { if (gfxVars::UseWebRender()) { wr::RenderThread::Start(); + layers::SharedSurfacesParent::Initialize(); } layers::CompositorThreadHolder::Start(); @@ -1053,6 +1055,7 @@ gfxPlatform::ShutdownLayersIPC() layers::CompositorThreadHolder::Shutdown(); gfx::VRListenerThreadHolder::Shutdown(); if (gfxVars::UseWebRender()) { + layers::SharedSurfacesParent::Shutdown(); wr::RenderThread::ShutDown(); Preferences::UnregisterCallback(WebRenderDebugPrefChangeCallback, WR_DEBUG_PREF); diff --git a/image/imgFrame.cpp b/image/imgFrame.cpp index 13f66d494ca2..05b1751dbcdf 100644 --- a/image/imgFrame.cpp +++ b/image/imgFrame.cpp @@ -101,7 +101,7 @@ AllocateBufferForImage(const IntSize& size, } #endif - if (!aIsAnimated && gfxPrefs::ImageMemShared()) { + if (!aIsAnimated && gfxVars::UseWebRender() && gfxPrefs::ImageMemShared()) { RefPtr newSurf = new SourceSurfaceSharedData(); if (newSurf->Init(size, stride, format)) { return newSurf.forget();