Bug 893301. Reviewer changes. r=nical
authorNicholas Cameron <ncameron@mozilla.com>
Thu, 28 Nov 2013 10:16:34 +1300
changeset 158512 7f3f0bab5d091e9e60f232cb489be40f9264d05e
parent 158511 e3ab7774da1aa70bbf0746bfd2788e6cadc52912
child 158513 49788093a6e736f88dba92a1f088c71e7c6fd48b
push id25749
push userryanvm@gmail.com
push dateTue, 03 Dec 2013 21:45:20 +0000
treeherdermozilla-central@85694fd9b17c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs893301
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 893301. Reviewer changes. r=nical
gfx/layers/composite/CompositableHost.cpp
gfx/layers/composite/CompositableHost.h
gfx/layers/composite/ContentHost.cpp
gfx/layers/composite/ImageHost.cpp
gfx/layers/composite/ImageHost.h
gfx/layers/ipc/CompositableTransactionParent.cpp
--- a/gfx/layers/composite/CompositableHost.cpp
+++ b/gfx/layers/composite/CompositableHost.cpp
@@ -54,30 +54,29 @@ CompositableHost::AddTextureHost(Texture
              "A texture is already present with this ID");
   RefPtr<TextureHost> second = mFirstTexture;
   mFirstTexture = aTexture;
   aTexture->SetNextSibling(second);
   aTexture->SetCompositableBackendSpecificData(GetCompositableBackendSpecificData());
 }
 
 void
-CompositableHost::RemoveTextureHost(uint64_t aTextureID)
+CompositableHost::RemoveTextureHost(TextureHost* aTexture)
 {
-  if (mFirstTexture && mFirstTexture->GetID() == aTextureID) {
-    RefPtr<TextureHost> toRemove = mFirstTexture;
+  uint64_t textureID = aTexture->GetID();
+  if (mFirstTexture && mFirstTexture->GetID() == textureID) {
     mFirstTexture = mFirstTexture->GetNextSibling();
-    toRemove->SetNextSibling(nullptr);
+    aTexture->SetNextSibling(nullptr);
   }
   RefPtr<TextureHost> it = mFirstTexture;
   while (it) {
     if (it->GetNextSibling() &&
-        it->GetNextSibling()->GetID() == aTextureID) {
-      RefPtr<TextureHost> toRemove = it->GetNextSibling();
+        it->GetNextSibling()->GetID() == textureID) {
       it->SetNextSibling(it->GetNextSibling()->GetNextSibling());
-      toRemove->SetNextSibling(nullptr);
+      aTexture->SetNextSibling(nullptr);
     }
     it = it->GetNextSibling();
   }
   if (!mFirstTexture && mBackendData) {
     mBackendData->ClearData();
   }
 }
 
--- a/gfx/layers/composite/CompositableHost.h
+++ b/gfx/layers/composite/CompositableHost.h
@@ -291,28 +291,23 @@ public:
 
   virtual TemporaryRef<gfx::DataSourceSurface> GetAsSurface() { return nullptr; }
 #endif
 
   virtual void PrintInfo(nsACString& aTo, const char* aPrefix) { }
 
   void AddTextureHost(TextureHost* aTexture);
   virtual void UseTextureHost(TextureHost* aTexture) {}
-  virtual void RemoveTextureHost(uint64_t aTextureID);
   // If a texture host is flagged for deferred removal, the compositable will
   // get an option to run any cleanup code early, that is when it would have
-  // been run if the texture host was not marked deferred. That is, this method
-  // is called _before_ RemoveTextureHost for deferred removal texture hosts.
+  // been run if the texture host was not marked deferred.
   // If the compositable does not cleanup the texture host now, it is the
   // compositable's responsibility to cleanup the texture host before the
   // texture host dies.
-  virtual void RemoveTextureHostDeferred(TextureHost* aTexture)
-  {
-    MOZ_CRASH("Compositable was not expecting to handle deferred removal of texture hosts");
-  }
+  virtual void RemoveTextureHost(TextureHost* aTexture);
   TextureHost* GetTextureHost(uint64_t aTextureID);
 
 protected:
   TextureInfo mTextureInfo;
   Compositor* mCompositor;
   Layer* mLayer;
   RefPtr<CompositableBackendSpecificData> mBackendData;
   RefPtr<TextureHost> mFirstTexture;
--- a/gfx/layers/composite/ContentHost.cpp
+++ b/gfx/layers/composite/ContentHost.cpp
@@ -43,17 +43,17 @@ ContentHostBaseNew::GetAsTextureHost()
 }
 
 void
 ContentHostBaseNew::DestroyTextureHost()
 {
   // The third clause in the if statement checks that we are in fact done with
   // this texture. We don't want to prematurely deallocate a texture we might
   // use again or double deallocate. Deallocation will happen in
-  // RemoveTextureHostDeferred.
+  // RemoveTextureHost.
   // Note that GetTextureHost is linear in the number of texture hosts, but as
   // long as that number is small (I expect a maximum of 6 for now) then it
   // should be ok.
   if (mTextureHost &&
       mTextureHost->GetFlags() & TEXTURE_DEALLOCATE_DEFERRED &&
       !GetTextureHost(mTextureHost->GetID())) {
     MOZ_ASSERT(!(mTextureHost->GetFlags() & TEXTURE_DEALLOCATE_CLIENT));
     mTextureHost->DeallocateSharedData();
@@ -69,23 +69,26 @@ ContentHostBaseNew::DestroyTextureHostOn
       !GetTextureHost(mTextureHostOnWhite->GetID())) {
     MOZ_ASSERT(!(mTextureHostOnWhite->GetFlags() & TEXTURE_DEALLOCATE_CLIENT));
     mTextureHostOnWhite->DeallocateSharedData();
   }
   mTextureHostOnWhite = nullptr;
 }
 
 void
-ContentHostBaseNew::RemoveTextureHostDeferred(TextureHost* aTexture)
+ContentHostBaseNew::RemoveTextureHost(TextureHost* aTexture)
 {
-  if (!(mTextureHost && mTextureHost == aTexture) &&
+  if ((aTexture->GetFlags() & TEXTURE_DEALLOCATE_DEFERRED) &&
+      !(mTextureHost && mTextureHost == aTexture) &&
       !(mTextureHostOnWhite && mTextureHostOnWhite == aTexture)) {
-    MOZ_ASSERT(aTexture->GetFlags() & TEXTURE_DEALLOCATE_DEFERRED);
+    MOZ_ASSERT(!(aTexture->GetFlags() & TEXTURE_DEALLOCATE_CLIENT));
     aTexture->DeallocateSharedData();
   }
+
+  CompositableHost::RemoveTextureHost(aTexture);
 }
 
 class MOZ_STACK_CLASS AutoLockTextureHost
 {
 public:
   AutoLockTextureHost(TextureHost* aHost)
     : mHost(aHost)
   {
--- a/gfx/layers/composite/ImageHost.cpp
+++ b/gfx/layers/composite/ImageHost.cpp
@@ -37,20 +37,20 @@ ImageHost::~ImageHost() {}
 
 void
 ImageHost::UseTextureHost(TextureHost* aTexture)
 {
   mFrontBuffer = aTexture;
 }
 
 void
-ImageHost::RemoveTextureHost(uint64_t aTextureID)
+ImageHost::RemoveTextureHost(TextureHost* aTexture)
 {
-  CompositableHost::RemoveTextureHost(aTextureID);
-  if (mFrontBuffer && mFrontBuffer->GetID() == aTextureID) {
+  CompositableHost::RemoveTextureHost(aTexture);
+  if (mFrontBuffer && mFrontBuffer->GetID() == aTexture->GetID()) {
     mFrontBuffer = nullptr;
   }
 }
 
 TextureHost*
 ImageHost::GetAsTextureHost()
 {
   return mFrontBuffer;
--- a/gfx/layers/composite/ImageHost.h
+++ b/gfx/layers/composite/ImageHost.h
@@ -50,17 +50,17 @@ public:
                          const gfx::Matrix4x4& aTransform,
                          const gfx::Filter& aFilter,
                          const gfx::Rect& aClipRect,
                          const nsIntRegion* aVisibleRegion = nullptr,
                          TiledLayerProperties* aLayerProperties = nullptr) MOZ_OVERRIDE;
 
   virtual void UseTextureHost(TextureHost* aTexture) MOZ_OVERRIDE;
 
-  virtual void RemoveTextureHost(uint64_t aTextureID) MOZ_OVERRIDE;
+  virtual void RemoveTextureHost(TextureHost* aTexture) MOZ_OVERRIDE;
 
   virtual TextureHost* GetAsTextureHost() MOZ_OVERRIDE;
 
   virtual void SetPictureRect(const nsIntRect& aPictureRect) MOZ_OVERRIDE
   {
     mPictureRect = aPictureRect;
     mHasPictureRect = true;
   }
--- a/gfx/layers/ipc/CompositableTransactionParent.cpp
+++ b/gfx/layers/ipc/CompositableTransactionParent.cpp
@@ -266,25 +266,22 @@ CompositableParentManager::ReceiveCompos
       }
       CompositableHost* compositable = AsCompositable(op);
 
       RefPtr<TextureHost> texture = compositable->GetTextureHost(op.textureID());
       MOZ_ASSERT(texture);
 
       TextureFlags flags = texture->GetFlags();
 
-      if (flags & TEXTURE_DEALLOCATE_DEFERRED) {
-        MOZ_ASSERT(!(flags & TEXTURE_DEALLOCATE_CLIENT),
-                   "textures should not be marked for deferred removal and client-side removal");
-        compositable->RemoveTextureHostDeferred(texture);
-      } else if (!(flags & TEXTURE_DEALLOCATE_CLIENT)) {
+      if (!(flags & TEXTURE_DEALLOCATE_CLIENT) &&
+          !(flags & TEXTURE_DEALLOCATE_DEFERRED)) {
         texture->DeallocateSharedData();
       }
 
-      compositable->RemoveTextureHost(op.textureID());
+      compositable->RemoveTextureHost(texture);
 
       // if it is not the host that deallocates the shared data, then we need
       // to notfy the client side to tell when it is safe to deallocate or
       // reuse it.
       if (flags & TEXTURE_DEALLOCATE_CLIENT) {
         replyv.push_back(ReplyTextureRemoved(op.compositableParent(), nullptr,
                                              op.textureID()));
       }