Bug 1322460 - Don't addref/release on the return value of prohibited functions. r?ehsan, aklotz draft bug-1322460
authorTing-Yu Chou <janus926@gmail.com>
Wed, 14 Dec 2016 14:34:26 +0800
changeset 450192 07b901d115ff9251ba979f6ea202a7d6f1fbd1b4
parent 448717 f46f85dcfbc2b3098ea758825d18be6fab33cbc6
child 539686 8978d476553537147607c720de24adc0d7d57676
push id38782
push userbmo:janus926@gmail.com
push dateFri, 16 Dec 2016 03:39:20 +0000
reviewersehsan, aklotz
bugs1322460
milestone53.0a1
Bug 1322460 - Don't addref/release on the return value of prohibited functions. r?ehsan, aklotz MozReview-Commit-ID: B0mAMZp5sll
accessible/windows/uia/uiaRawElmProvider.cpp
gfx/layers/TextureDIB.cpp
gfx/layers/d3d9/TextureD3D9.cpp
ipc/mscom/DispatchForwarder.cpp
ipc/mscom/Interceptor.cpp
ipc/mscom/Interceptor.h
ipc/mscom/WeakRef.cpp
ipc/mscom/WeakRef.h
--- a/accessible/windows/uia/uiaRawElmProvider.cpp
+++ b/accessible/windows/uia/uiaRawElmProvider.cpp
@@ -51,18 +51,18 @@ uiaRawElmProvider::GetIAccessiblePair(__
 
   *aAcc = nullptr;
   *aIdChild = 0;
 
   if (mAcc->IsDefunct())
     return CO_E_OBJNOTCONNECTED;
 
   *aIdChild = CHILDID_SELF;
-  *aAcc = mAcc;
-  mAcc->AddRef();
+  RefPtr<AccessibleWrap> copy(mAcc);
+  copy.forget(aAcc);
 
   return S_OK;
 
   A11Y_TRYBLOCK_END
 }
 
 STDMETHODIMP
 uiaRawElmProvider::GetRuntimeId(__RPC__deref_out_opt SAFEARRAY** aRuntimeIds)
--- a/gfx/layers/TextureDIB.cpp
+++ b/gfx/layers/TextureDIB.cpp
@@ -168,17 +168,17 @@ MemoryDIBTextureData::CreateSimilar(Laye
 
 bool
 MemoryDIBTextureData::Serialize(SurfaceDescriptor& aOutDescriptor)
 {
   MOZ_ASSERT(mSurface);
   // The host will release this ref when it receives the surface descriptor.
   // We AddRef in case we die before the host receives the pointer.
   aOutDescriptor = SurfaceDescriptorDIB(reinterpret_cast<uintptr_t>(mSurface.get()));
-  mSurface->AddRef();
+  mSurface.get()->AddRef();
   return true;
 }
 
 DIBTextureData*
 MemoryDIBTextureData::Create(gfx::IntSize aSize, gfx::SurfaceFormat aFormat)
 {
   RefPtr<gfxWindowsSurface> surface
     = new gfxWindowsSurface(aSize, SurfaceFormatToImageFormat(aFormat));
--- a/gfx/layers/d3d9/TextureD3D9.cpp
+++ b/gfx/layers/d3d9/TextureD3D9.cpp
@@ -642,17 +642,17 @@ D3D9TextureData::Unlock()
     mD3D9Surface->UnlockRect();
     mLockRect = false;
   }
 }
 
 bool
 D3D9TextureData::Serialize(SurfaceDescriptor& aOutDescriptor)
 {
-  mTexture->AddRef(); // Release in TextureHostD3D9::TextureHostD3D9
+  mTexture.get()->AddRef(); // Release in TextureHostD3D9::TextureHostD3D9
   aOutDescriptor = SurfaceDescriptorD3D9(reinterpret_cast<uintptr_t>(mTexture.get()));
   return true;
 }
 
 already_AddRefed<gfx::DrawTarget>
 D3D9TextureData::BorrowDrawTarget()
 {
   MOZ_ASSERT(mD3D9Surface);
@@ -830,17 +830,17 @@ DXGID3D9TextureData::Serialize(SurfaceDe
 TextureHostD3D9::TextureHostD3D9(TextureFlags aFlags,
                                  const SurfaceDescriptorD3D9& aDescriptor)
   : TextureHost(aFlags)
   , mFormat(SurfaceFormat::UNKNOWN)
   , mIsLocked(false)
 {
   mTexture = reinterpret_cast<IDirect3DTexture9*>(aDescriptor.texture());
   MOZ_ASSERT(mTexture);
-  mTexture->Release(); // see AddRef in TextureClientD3D9::ToSurfaceDescriptor
+  mTexture.get()->Release(); // see AddRef in TextureClientD3D9::ToSurfaceDescriptor
   MOZ_ASSERT(mTexture);
   D3DSURFACE_DESC desc;
   HRESULT hr = mTexture->GetLevelDesc(0, &desc);
   if (!FAILED(hr)) {
     mFormat = D3D9FormatToSurfaceFormat(desc.Format);
     mSize.width = desc.Width;
     mSize.height = desc.Height;
   }
--- a/ipc/mscom/DispatchForwarder.cpp
+++ b/ipc/mscom/DispatchForwarder.cpp
@@ -95,18 +95,18 @@ DispatchForwarder::GetTypeInfoCount(UINT
 }
 
 HRESULT
 DispatchForwarder::GetTypeInfo(UINT iTInfo, LCID lcid, ITypeInfo **ppTInfo)
 {
   // ITypeInfo as implemented by COM is apartment-neutral, so we don't need
   // to wrap it (yay!)
   if (mTypeInfo) {
-    *ppTInfo = mTypeInfo.get();
-    mTypeInfo->AddRef();
+    RefPtr<ITypeInfo> copy(mTypeInfo);
+    copy.forget(ppTInfo);
     return S_OK;
   }
   HRESULT hr = E_UNEXPECTED;
   auto fn = [&]() -> void {
     hr = mTarget->GetTypeInfo(iTInfo, lcid, ppTInfo);
   };
   MainThreadInvoker invoker;
   if (!invoker.Invoke(NS_NewRunnableFunction(fn))) {
--- a/ipc/mscom/Interceptor.cpp
+++ b/ipc/mscom/Interceptor.cpp
@@ -56,17 +56,17 @@ Interceptor::Interceptor(STAUniquePtr<IU
 
 Interceptor::~Interceptor()
 {
   // This needs to run on the main thread because it releases target interface
   // reference counts which may not be thread-safe.
   MOZ_ASSERT(NS_IsMainThread());
   for (uint32_t index = 0, len = mInterceptorMap.Length(); index < len; ++index) {
     MapEntry& entry = mInterceptorMap[index];
-    entry.mInterceptor->Release();
+    entry.mInterceptor = nullptr;
     entry.mTargetInterface->Release();
   }
 }
 
 Interceptor::MapEntry*
 Interceptor::Lookup(REFIID aIid)
 {
   mMutex.AssertCurrentThreadOwns();
@@ -229,22 +229,19 @@ Interceptor::GetInterceptorForIID(REFIID
   { // Scope for lock
     MutexAutoLock lock(mMutex);
     // We might have raced with another thread, so first check that we don't
     // already have an entry for this
     MapEntry* entry = Lookup(aIid);
     if (entry && entry->mInterceptor) {
       unkInterceptor = entry->mInterceptor;
     } else {
-      // We're inserting unkInterceptor into the map but we still want to hang
-      // onto it locally so that we can QI it below.
-      unkInterceptor->AddRef();
-      // OTOH we must not touch the refcount for the target interface
-      // because we are just moving it into the map and its refcounting might
-      // not be thread-safe.
+      // MapEntry has a RefPtr to unkInterceptor, OTOH we must not touch the
+      // refcount for the target interface because we are just moving it into
+      // the map and its refcounting might not be thread-safe.
       IUnknown* rawTargetInterface = targetInterface.release();
       mInterceptorMap.AppendElement(MapEntry(aIid,
                                              unkInterceptor,
                                              rawTargetInterface));
     }
   }
 
   return unkInterceptor->QueryInterface(aIid, aOutInterceptor);
--- a/ipc/mscom/Interceptor.h
+++ b/ipc/mscom/Interceptor.h
@@ -76,17 +76,17 @@ private:
   struct MapEntry
   {
     MapEntry(REFIID aIid, IUnknown* aInterceptor, IUnknown* aTargetInterface)
       : mIID(aIid)
       , mInterceptor(aInterceptor)
       , mTargetInterface(aTargetInterface)
     {}
     IID               mIID;
-    IUnknown*         mInterceptor;
+    RefPtr<IUnknown>  mInterceptor;
     IUnknown*         mTargetInterface;
   };
 
 private:
   Interceptor(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink);
   ~Interceptor();
   MapEntry* Lookup(REFIID aIid);
   HRESULT QueryInterfaceTarget(REFIID aIid, void** aOutput);
--- a/ipc/mscom/WeakRef.cpp
+++ b/ipc/mscom/WeakRef.cpp
@@ -94,17 +94,17 @@ WeakReferenceSupport::Release()
   return newRefCnt;
 }
 
 void
 WeakReferenceSupport::ClearWeakRefs()
 {
   for (uint32_t i = 0, len = mWeakRefs.Length(); i < len; ++i) {
     mWeakRefs[i]->Clear();
-    mWeakRefs[i]->Release();
+    mWeakRefs[i] = nullptr;
   }
   mWeakRefs.Clear();
 }
 
 HRESULT
 WeakReferenceSupport::GetWeakReference(IWeakReference** aOutWeakRef)
 {
   if (!aOutWeakRef) {
@@ -115,18 +115,17 @@ WeakReferenceSupport::GetWeakReference(I
   AutoCriticalSection lock(&mCS);
   RefPtr<WeakRef> weakRef = MakeAndAddRef<WeakRef>(this);
 
   HRESULT hr = weakRef->QueryInterface(IID_IWeakReference, (void**)aOutWeakRef);
   if (FAILED(hr)) {
     return hr;
   }
 
-  mWeakRefs.AppendElement(weakRef.get());
-  weakRef->AddRef();
+  mWeakRefs.AppendElement(weakRef);
   return S_OK;
 }
 
 WeakRef::WeakRef(WeakReferenceSupport* aSupport)
   : mRefCnt(1)
   , mMutex("mozilla::mscom::WeakRef::mMutex")
   , mSupport(aSupport)
 {
--- a/ipc/mscom/WeakRef.h
+++ b/ipc/mscom/WeakRef.h
@@ -65,20 +65,20 @@ protected:
   virtual HRESULT ThreadSafeQueryInterface(REFIID aIid,
                                            IUnknown** aOutInterface) = 0;
 
 private:
   void ClearWeakRefs();
 
 private:
   // Using a raw CRITICAL_SECTION here because it can be reentered
-  CRITICAL_SECTION    mCS;
-  ULONG               mRefCnt;
-  nsTArray<WeakRef*>  mWeakRefs;
-  Flags               mFlags;
+  CRITICAL_SECTION           mCS;
+  ULONG                      mRefCnt;
+  nsTArray<RefPtr<WeakRef>>  mWeakRefs;
+  Flags                      mFlags;
 };
 
 class WeakRef : public IWeakReference
 {
 public:
   // IUnknown
   STDMETHODIMP QueryInterface(REFIID riid, void** ppv) override;
   STDMETHODIMP_(ULONG) AddRef() override;