From db2e8ee11d22cd9149a2df469fbcc33af1f7895a Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 23 Mar 2016 10:32:34 -0700 Subject: [PATCH] Block compositable updates from stale layers. (bug 1256517 part 4, r=mattwoodrow,nical) --- gfx/layers/Compositor.cpp | 12 ++++++++++++ gfx/layers/Compositor.h | 5 +++++ gfx/layers/basic/BasicCompositor.cpp | 5 +++-- gfx/layers/basic/BasicCompositor.h | 2 +- gfx/layers/basic/X11BasicCompositor.h | 5 +++-- gfx/layers/d3d11/CompositorD3D11.cpp | 5 +++-- gfx/layers/d3d11/CompositorD3D11.h | 2 +- gfx/layers/ipc/CompositableTransactionParent.cpp | 10 +++++++++- gfx/layers/ipc/CompositorBridgeParent.cpp | 12 ++++++++---- gfx/layers/ipc/LayerTransactionParent.cpp | 10 ++++++++++ gfx/layers/opengl/CompositorOGL.cpp | 6 ++++-- gfx/layers/opengl/CompositorOGL.h | 3 ++- gfx/tests/gtest/TestCompositor.cpp | 5 +++-- 13 files changed, 64 insertions(+), 18 deletions(-) diff --git a/gfx/layers/Compositor.cpp b/gfx/layers/Compositor.cpp index b04c8c3df138..56ecb704f089 100644 --- a/gfx/layers/Compositor.cpp +++ b/gfx/layers/Compositor.cpp @@ -398,6 +398,18 @@ Compositor::ComputeBackdropCopyRect(const gfx::Rect& aRect, return result; } +void +Compositor::SetInvalid() +{ + mParent = nullptr; +} + +bool +Compositor::IsValid() const +{ + return !mParent; +} + #if defined(MOZ_WIDGET_GONK) && ANDROID_VERSION >= 17 void Compositor::SetDispAcquireFence(Layer* aLayer, nsIWidget* aWidget) diff --git a/gfx/layers/Compositor.h b/gfx/layers/Compositor.h index b2a333ff94ae..a37ad75bc94e 100644 --- a/gfx/layers/Compositor.h +++ b/gfx/layers/Compositor.h @@ -517,6 +517,11 @@ public: return mCompositeUntilTime; } + // A stale Compositor has no CompositorBridgeParent; it will not process + // frames and should not be used. + void SetInvalid(); + bool IsValid() const; + protected: void DrawDiagnosticsInternal(DiagnosticFlags aFlags, const gfx::Rect& aVisibleRect, diff --git a/gfx/layers/basic/BasicCompositor.cpp b/gfx/layers/basic/BasicCompositor.cpp index 6f37396e58df..81809b5681b7 100644 --- a/gfx/layers/basic/BasicCompositor.cpp +++ b/gfx/layers/basic/BasicCompositor.cpp @@ -78,8 +78,9 @@ public: bool mWrappingExistingData; }; -BasicCompositor::BasicCompositor(nsIWidget *aWidget) - : mWidget(aWidget) +BasicCompositor::BasicCompositor(CompositorBridgeParent* aParent, nsIWidget *aWidget) + : Compositor(aParent) + , mWidget(aWidget) , mDidExternalComposition(false) { MOZ_COUNT_CTOR(BasicCompositor); diff --git a/gfx/layers/basic/BasicCompositor.h b/gfx/layers/basic/BasicCompositor.h index e4696e1b3e45..ae4e38f82dad 100644 --- a/gfx/layers/basic/BasicCompositor.h +++ b/gfx/layers/basic/BasicCompositor.h @@ -42,7 +42,7 @@ public: class BasicCompositor : public Compositor { public: - explicit BasicCompositor(nsIWidget *aWidget); + explicit BasicCompositor(CompositorBridgeParent* aParent, nsIWidget *aWidget); protected: virtual ~BasicCompositor(); diff --git a/gfx/layers/basic/X11BasicCompositor.h b/gfx/layers/basic/X11BasicCompositor.h index 5f7574d42087..d9efc568d79e 100644 --- a/gfx/layers/basic/X11BasicCompositor.h +++ b/gfx/layers/basic/X11BasicCompositor.h @@ -46,8 +46,9 @@ private: class X11BasicCompositor : public BasicCompositor { public: - - explicit X11BasicCompositor(nsIWidget *aWidget) : BasicCompositor(aWidget) {} + explicit X11BasicCompositor(CompositorBridgeParent* aParent, nsIWidget *aWidget) + : BasicCompositor(aParent, aWidget) + {} virtual already_AddRefed CreateDataTextureSource(TextureFlags aFlags = TextureFlags::NO_FLAGS) override; diff --git a/gfx/layers/d3d11/CompositorD3D11.cpp b/gfx/layers/d3d11/CompositorD3D11.cpp index 7ee7fbc80486..6a647e761844 100644 --- a/gfx/layers/d3d11/CompositorD3D11.cpp +++ b/gfx/layers/d3d11/CompositorD3D11.cpp @@ -158,8 +158,9 @@ private: bool mInitOkay; }; -CompositorD3D11::CompositorD3D11(nsIWidget* aWidget) - : mAttachments(nullptr) +CompositorD3D11::CompositorD3D11(CompositorBridgeParent* aParent, nsIWidget* aWidget) + : Compositor(aParent) + , mAttachments(nullptr) , mWidget(aWidget) , mHwnd(nullptr) , mDisableSequenceForNextFrame(false) diff --git a/gfx/layers/d3d11/CompositorD3D11.h b/gfx/layers/d3d11/CompositorD3D11.h index 5b1953596e33..aa6ea341a218 100644 --- a/gfx/layers/d3d11/CompositorD3D11.h +++ b/gfx/layers/d3d11/CompositorD3D11.h @@ -42,7 +42,7 @@ struct DeviceAttachmentsD3D11; class CompositorD3D11 : public Compositor { public: - CompositorD3D11(nsIWidget* aWidget); + CompositorD3D11(CompositorBridgeParent* aParent, nsIWidget* aWidget); ~CompositorD3D11(); virtual bool Initialize() override; diff --git a/gfx/layers/ipc/CompositableTransactionParent.cpp b/gfx/layers/ipc/CompositableTransactionParent.cpp index 58c79039500b..2236e27dbd40 100644 --- a/gfx/layers/ipc/CompositableTransactionParent.cpp +++ b/gfx/layers/ipc/CompositableTransactionParent.cpp @@ -74,7 +74,12 @@ bool CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation& aEdit, EditReplyVector& replyv) { + // Ignore all operations on compositables created on stale compositors. We + // return true because the child is unable to handle errors. CompositableHost* compositable = CompositableHost::FromIPDLActor(aEdit.compositableParent()); + if (compositable->GetCompositor() && compositable->GetCompositor()->IsValid()) { + return true; + } switch (aEdit.detail().type()) { case CompositableOperationDetail::TOpPaintTextureRegion: { @@ -121,6 +126,7 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation } case CompositableOperationDetail::TOpRemoveTexture: { const OpRemoveTexture& op = aEdit.detail().get_OpRemoveTexture(); + RefPtr tex = TextureHost::AsTextureHost(op.textureParent()); MOZ_ASSERT(tex.get()); @@ -183,7 +189,9 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation } } } - compositable->UseTextureHost(textures); + if (textures.Length() > 0) { + compositable->UseTextureHost(textures); + } if (UsesImageBridge() && compositable->GetLayer()) { ScheduleComposition(compositable); diff --git a/gfx/layers/ipc/CompositorBridgeParent.cpp b/gfx/layers/ipc/CompositorBridgeParent.cpp index ad71f8ef40ae..925e821b68d4 100644 --- a/gfx/layers/ipc/CompositorBridgeParent.cpp +++ b/gfx/layers/ipc/CompositorBridgeParent.cpp @@ -1618,22 +1618,23 @@ CompositorBridgeParent::NewCompositor(const nsTArray& aBackendHin for (size_t i = 0; i < aBackendHints.Length(); ++i) { RefPtr compositor; if (aBackendHints[i] == LayersBackend::LAYERS_OPENGL) { - compositor = new CompositorOGL(mWidget, + compositor = new CompositorOGL(this, + mWidget, mEGLSurfaceSize.width, mEGLSurfaceSize.height, mUseExternalSurfaceSize); } else if (aBackendHints[i] == LayersBackend::LAYERS_BASIC) { #ifdef MOZ_WIDGET_GTK if (gfxPlatformGtk::GetPlatform()->UseXRender()) { - compositor = new X11BasicCompositor(mWidget); + compositor = new X11BasicCompositor(this, mWidget); } else #endif { - compositor = new BasicCompositor(mWidget); + compositor = new BasicCompositor(this, mWidget); } #ifdef XP_WIN } else if (aBackendHints[i] == LayersBackend::LAYERS_D3D11) { - compositor = new CompositorD3D11(mWidget); + compositor = new CompositorD3D11(this, mWidget); } else if (aBackendHints[i] == LayersBackend::LAYERS_D3D9) { compositor = new CompositorD3D9(this, mWidget); #endif @@ -2212,6 +2213,9 @@ CompositorBridgeParent::ResetCompositorImpl(const nsTArray& aBack return Nothing(); } + if (mCompositor) { + mCompositor->SetInvalid(); + } mCompositor = compositor; mLayerManager->ChangeCompositor(compositor); diff --git a/gfx/layers/ipc/LayerTransactionParent.cpp b/gfx/layers/ipc/LayerTransactionParent.cpp index 229bb5113ca5..30ffa9f2715e 100644 --- a/gfx/layers/ipc/LayerTransactionParent.cpp +++ b/gfx/layers/ipc/LayerTransactionParent.cpp @@ -587,6 +587,11 @@ LayerTransactionParent::RecvUpdate(InfallibleTArray&& cset, case Edit::TOpAttachCompositable: { const OpAttachCompositable& op = edit.get_OpAttachCompositable(); CompositableHost* host = CompositableHost::FromIPDLActor(op.compositableParent()); + if (mPendingCompositorUpdates) { + // Do not attach compositables from old layer trees. Return true since + // content cannot handle errors. + return true; + } if (!Attach(cast(op.layerParent()), host, false)) { return false; } @@ -600,6 +605,11 @@ LayerTransactionParent::RecvUpdate(InfallibleTArray&& cset, NS_ERROR("CompositableParent not found in the map"); return false; } + if (mPendingCompositorUpdates) { + // Do not attach compositables from old layer trees. Return true since + // content cannot handle errors. + return true; + } CompositableHost* host = CompositableHost::FromIPDLActor(compositableParent); if (!Attach(cast(op.layerParent()), host, true)) { return false; diff --git a/gfx/layers/opengl/CompositorOGL.cpp b/gfx/layers/opengl/CompositorOGL.cpp index 812c9e17e638..bcc685b7b2be 100644 --- a/gfx/layers/opengl/CompositorOGL.cpp +++ b/gfx/layers/opengl/CompositorOGL.cpp @@ -83,9 +83,11 @@ CompositorOGL::BindBackdrop(ShaderProgramOGL* aProgram, GLuint aBackdrop, GLenum aProgram->SetBackdropTextureUnit(aTexUnit - LOCAL_GL_TEXTURE0); } -CompositorOGL::CompositorOGL(nsIWidget *aWidget, int aSurfaceWidth, +CompositorOGL::CompositorOGL(CompositorBridgeParent* aParent, + nsIWidget *aWidget, int aSurfaceWidth, int aSurfaceHeight, bool aUseExternalSurfaceSize) - : mWidget(aWidget) + : Compositor(aParent) + , mWidget(aWidget) , mWidgetSize(-1, -1) , mSurfaceSize(aSurfaceWidth, aSurfaceHeight) , mHasBGRA(0) diff --git a/gfx/layers/opengl/CompositorOGL.h b/gfx/layers/opengl/CompositorOGL.h index 845c16ff6dd2..343a691d7e39 100644 --- a/gfx/layers/opengl/CompositorOGL.h +++ b/gfx/layers/opengl/CompositorOGL.h @@ -193,7 +193,8 @@ class CompositorOGL final : public Compositor std::map mPrograms; public: - explicit CompositorOGL(nsIWidget *aWidget, int aSurfaceWidth = -1, int aSurfaceHeight = -1, + explicit CompositorOGL(CompositorBridgeParent* aParent, + nsIWidget *aWidget, int aSurfaceWidth = -1, int aSurfaceHeight = -1, bool aUseExternalSurfaceSize = false); protected: diff --git a/gfx/tests/gtest/TestCompositor.cpp b/gfx/tests/gtest/TestCompositor.cpp index ee0a2a807825..b2a184450aef 100644 --- a/gfx/tests/gtest/TestCompositor.cpp +++ b/gfx/tests/gtest/TestCompositor.cpp @@ -112,13 +112,14 @@ static already_AddRefed CreateTestCompositor(LayersBackend backend, RefPtr compositor; if (backend == LayersBackend::LAYERS_OPENGL) { - compositor = new CompositorOGL(widget, + compositor = new CompositorOGL(nullptr, + widget, gCompWidth, gCompHeight, true); compositor->SetDestinationSurfaceSize(IntSize(gCompWidth, gCompHeight)); } else if (backend == LayersBackend::LAYERS_BASIC) { - compositor = new BasicCompositor(widget); + compositor = new BasicCompositor(nullptr, widget); #ifdef XP_WIN } else if (backend == LayersBackend::LAYERS_D3D11) { //compositor = new CompositorD3D11();