Bug 1114999 - Part 2: Apply MOZ_NO_ADDREF_RELEASE_ON_RETURN to all smart pointer arrow operators that can return refcounted objects; r=jrmuizel
authorEhsan Akhgari <ehsan@mozilla.com>
Thu, 25 Dec 2014 15:18:38 -0500
changeset 248106 3d94c5baadb0effa4e2cdc0fdefc5e946edb3f64
parent 248105 524915a71a51139d2548fc71fde0ccc86e456a58
child 248107 53e1120d30487db30df22a90a6d23b5430a853a8
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1114999
milestone37.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 1114999 - Part 2: Apply MOZ_NO_ADDREF_RELEASE_ON_RETURN to all smart pointer arrow operators that can return refcounted objects; r=jrmuizel
dom/archivereader/ArchiveZipFile.cpp
dom/asmjscache/AsmJSCache.cpp
dom/base/nsGlobalWindow.cpp
dom/canvas/CanvasRenderingContext2D.cpp
dom/canvas/WebGLObjectModel.h
dom/indexedDB/ActorsParent.cpp
dom/media/MediaResource.h
dom/media/eme/CDMProxy.h
dom/mobilemessage/ipc/SmsIPCService.cpp
dom/xul/templates/nsResourceSet.h
embedding/components/printingui/unixshared/nsPrintingPromptService.cpp
embedding/components/printingui/win/nsPrintingPromptService.cpp
gfx/layers/ImageContainer.cpp
gfx/layers/client/TiledContentClient.cpp
gfx/layers/composite/TiledContentHost.cpp
gfx/thebes/gfxQuartzSurface.cpp
mfbt/Attributes.h
mfbt/RefPtr.h
mfbt/WeakPtr.h
netwerk/base/src/AutoClose.h
netwerk/cache/nsDiskCacheDeviceSQL.cpp
netwerk/sctp/datachannel/DataChannel.cpp
netwerk/sctp/datachannel/DataChannel.h
parser/html/nsHtml5RefPtr.h
widget/gtk/nsClipboard.cpp
xpcom/base/nsRefPtr.h
xpcom/glue/nsCOMPtr.h
xpcom/glue/nsProxyRelease.h
xpcom/glue/nsTWeakRef.h
xpcom/tests/TestTimers.cpp
--- a/dom/archivereader/ArchiveZipFile.cpp
+++ b/dom/archivereader/ArchiveZipFile.cpp
@@ -373,19 +373,18 @@ ArchiveZipFileImpl::GetInternalStream(ns
   }
 
   nsRefPtr<ArchiveInputStream> stream = new ArchiveInputStream(size,
                                                                inputStream,
                                                                mFilename,
                                                                mStart,
                                                                mLength,
                                                                mCentral);
-  NS_ADDREF(stream);
 
-  *aStream = stream;
+  stream.forget(aStream);
   return NS_OK;
 }
 
 void
 ArchiveZipFileImpl::Unlink()
 {
   ArchiveZipFileImpl* tmp = this;
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mArchiveReader);
--- a/dom/asmjscache/AsmJSCache.cpp
+++ b/dom/asmjscache/AsmJSCache.cpp
@@ -349,17 +349,17 @@ public:
     void
     Init(File* aFile)
     {
       MOZ_ASSERT(!mFile);
       mFile = aFile;
     }
 
     File*
-    operator->() const
+    operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
     {
       MOZ_ASSERT(mFile);
       return mFile;
     }
 
     void
     Forget(File** aFile)
     {
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -12433,17 +12433,17 @@ nsGlobalWindow::RunTimeout(nsTimeout *aT
   // portion of the list that we are about to process now and those
   // timeouts that will be processed in a future call to
   // win_run_timeout(). This dummy timeout serves as the head of the
   // list for any timeouts inserted as a result of running a timeout.
   nsRefPtr<nsTimeout> dummy_timeout = new nsTimeout();
   dummy_timeout->mFiringDepth = firingDepth;
   dummy_timeout->mWhen = now;
   last_expired_timeout->setNext(dummy_timeout);
-  dummy_timeout->AddRef();
+  nsRefPtr<nsTimeout> timeoutExtraRef(dummy_timeout);
 
   last_insertion_point = mTimeoutInsertionPoint;
   // If we ever start setting mTimeoutInsertionPoint to a non-dummy timeout,
   // the logic in ResetTimersForNonBackgroundWindow will need to change.
   mTimeoutInsertionPoint = dummy_timeout;
 
   Telemetry::AutoCounter<Telemetry::DOM_TIMERS_FIRED_PER_NATIVE_TIMEOUT> timeoutsRan;
 
@@ -12484,16 +12484,17 @@ nsGlobalWindow::RunTimeout(nsTimeout *aT
     bool timeout_was_cleared = RunTimeoutHandler(timeout, scx);
 
     if (timeout_was_cleared) {
       // The running timeout's window was cleared, this means that
       // ClearAllTimeouts() was called from a *nested* call, possibly
       // through a timeout that fired while a modal (to this window)
       // dialog was open or through other non-obvious paths.
       MOZ_ASSERT(dummy_timeout->HasRefCntOne(), "dummy_timeout may leak");
+      unused << timeoutExtraRef.forget().take();
 
       mTimeoutInsertionPoint = last_insertion_point;
 
       return;
     }
 
     // If we have a regular interval timer, we re-schedule the
     // timeout, accounting for clock drift.
@@ -12512,17 +12513,17 @@ nsGlobalWindow::RunTimeout(nsTimeout *aT
     }
 
     // Release the timeout struct since it's possibly out of the list
     timeout->Release();
   }
 
   // Take the dummy timeout off the head of the list
   dummy_timeout->remove();
-  dummy_timeout->Release();
+  timeoutExtraRef = nullptr;
   MOZ_ASSERT(dummy_timeout->HasRefCntOne(), "dummy_timeout may leak");
 
   mTimeoutInsertionPoint = last_insertion_point;
 }
 
 void
 nsGlobalWindow::ClearTimeoutOrInterval(int32_t aTimerID, ErrorResult& aError)
 {
--- a/dom/canvas/CanvasRenderingContext2D.cpp
+++ b/dom/canvas/CanvasRenderingContext2D.cpp
@@ -573,17 +573,17 @@ public:
     mShadowTarget.reset();
   }
 
   operator DrawTarget*()
   {
     return mTarget;
   }
 
-  DrawTarget* operator->()
+  DrawTarget* operator->() MOZ_NO_ADDREF_RELEASE_ON_RETURN
   {
     return mTarget;
   }
 
 private:
 
   mgfx::Rect
   MaxSourceNeededBoundsForFilter(const mgfx::Rect& aDestBounds, CanvasRenderingContext2D *ctx)
--- a/dom/canvas/WebGLObjectModel.h
+++ b/dom/canvas/WebGLObjectModel.h
@@ -214,17 +214,17 @@ public:
     T* get() const {
         return static_cast<T*>(mRawPtr);
     }
 
     operator T*() const {
         return get();
     }
 
-    T* operator->() const {
+    T* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN {
         MOZ_ASSERT(mRawPtr != 0, "You can't dereference a nullptr WebGLRefPtr with operator->()!");
         return get();
     }
 
     T& operator*() const {
         MOZ_ASSERT(mRawPtr != 0, "You can't dereference a nullptr WebGLRefPtr with operator*()!");
         return *get();
     }
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -3662,17 +3662,17 @@ public:
   { }
 
   operator mozIStorageStatement*()
   {
     return mStatement;
   }
 
   mozIStorageStatement*
-  operator->()
+  operator->() MOZ_NO_ADDREF_RELEASE_ON_RETURN
   {
     MOZ_ASSERT(mStatement);
     return mStatement;
   }
 
   void
   Reset()
   {
@@ -4072,17 +4072,17 @@ struct FactoryOp::MaybeBlockedDatabaseIn
 
   bool
   operator<(const MaybeBlockedDatabaseInfo& aOther) const
   {
     return mDatabase < aOther.mDatabase;
   }
 
   Database*
-  operator->()
+  operator->() MOZ_NO_ADDREF_RELEASE_ON_RETURN
   {
     return mDatabase;
   }
 };
 
 class OpenDatabaseOp MOZ_FINAL
   : public FactoryOp
 {
--- a/dom/media/MediaResource.h
+++ b/dom/media/MediaResource.h
@@ -739,17 +739,17 @@ class MOZ_STACK_CLASS AutoPinned {
     mResource->Pin();
   }
 
   ~AutoPinned() {
     mResource->Unpin();
   }
 
   operator T*() const { return mResource; }
-  T* operator->() const { return mResource; }
+  T* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN { return mResource; }
 
 private:
   T* mResource;
   MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
 } // namespace mozilla
 
--- a/dom/media/eme/CDMProxy.h
+++ b/dom/media/eme/CDMProxy.h
@@ -275,17 +275,17 @@ private:
       return !mPtr;
     }
 
     void Clear() {
       MOZ_ASSERT(NS_IsMainThread());
       mPtr = nullptr;
     }
 
-    Type* operator->() const {
+    Type* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN {
       MOZ_ASSERT(NS_IsMainThread());
       return mPtr;
     }
   private:
     Type* mPtr;
   };
 
   // Our reference back to the MediaKeys object.
--- a/dom/mobilemessage/ipc/SmsIPCService.cpp
+++ b/dom/mobilemessage/ipc/SmsIPCService.cpp
@@ -9,16 +9,17 @@
 #include "mozilla/dom/mobilemessage/SmsChild.h"
 #include "SmsMessage.h"
 #include "nsJSUtils.h"
 #include "mozilla/dom/MozMobileMessageManagerBinding.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/Preferences.h"
 #include "nsString.h"
 #include "mozilla/dom/ipc/BlobChild.h"
+#include "mozilla/unused.h"
 
 using namespace mozilla::dom;
 using namespace mozilla::dom::mobilemessage;
 
 namespace {
 
 #define kPrefMmsDefaultServiceId "dom.mms.defaultServiceId"
 #define kPrefSmsDefaultServiceId "dom.sms.defaultServiceId"
@@ -65,17 +66,18 @@ SendCursorRequest(const IPCMobileMessage
   PSmsChild* smsChild = GetSmsChild();
   NS_ENSURE_TRUE(smsChild, NS_ERROR_FAILURE);
 
   nsRefPtr<MobileMessageCursorChild> actor =
     new MobileMessageCursorChild(aRequestReply);
 
   // Add an extra ref for IPDL. Will be released in
   // SmsChild::DeallocPMobileMessageCursor().
-  actor->AddRef();
+  nsRefPtr<MobileMessageCursorChild> actorCopy(actor);
+  mozilla::unused << actorCopy.forget().take();
 
   smsChild->SendPMobileMessageCursorConstructor(actor, aRequest);
 
   actor.forget(aResult);
   return NS_OK;
 }
 
 uint32_t
--- a/dom/xul/templates/nsResourceSet.h
+++ b/dom/xul/templates/nsResourceSet.h
@@ -55,17 +55,17 @@ public:
         ConstIterator operator++(int) {
             ConstIterator result(*this);
             ++mCurrent;
             return result; }
 
         /*const*/ nsIRDFResource* operator*() const {
             return *mCurrent; }
 
-        /*const*/ nsIRDFResource* operator->() const {
+        /*const*/ nsIRDFResource* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN {
             return *mCurrent; }
 
         bool operator==(const ConstIterator& aConstIterator) const {
             return mCurrent == aConstIterator.mCurrent; }
 
         bool operator!=(const ConstIterator& aConstIterator) const {
             return mCurrent != aConstIterator.mCurrent; }
 
--- a/embedding/components/printingui/unixshared/nsPrintingPromptService.cpp
+++ b/embedding/components/printingui/unixshared/nsPrintingPromptService.cpp
@@ -37,17 +37,17 @@ public:
     }
     ~ParamBlock() 
     {
         NS_IF_RELEASE(mBlock);
     }
     nsresult Init() {
       return CallCreateInstance(NS_DIALOGPARAMBLOCK_CONTRACTID, &mBlock);
     }
-    nsIDialogParamBlock * operator->() const { return mBlock; }
+    nsIDialogParamBlock * operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN { return mBlock; }
     operator nsIDialogParamBlock * const ()  { return mBlock; }
 
 private:
     nsIDialogParamBlock *mBlock;
 };
 
 /****************************************************************
  ***************** nsPrintingPromptService **********************
--- a/embedding/components/printingui/win/nsPrintingPromptService.cpp
+++ b/embedding/components/printingui/win/nsPrintingPromptService.cpp
@@ -54,17 +54,17 @@ public:
     }
     ~ParamBlock() 
     {
         NS_IF_RELEASE(mBlock);
     }
     nsresult Init() {
       return CallCreateInstance(NS_DIALOGPARAMBLOCK_CONTRACTID, &mBlock);
     }
-    nsIDialogParamBlock * operator->() const { return mBlock; }
+    nsIDialogParamBlock * operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN { return mBlock; }
     operator nsIDialogParamBlock * const ()  { return mBlock; }
 
 private:
     nsIDialogParamBlock *mBlock;
 };
 
 //*****************************************************************************   
 
--- a/gfx/layers/ImageContainer.cpp
+++ b/gfx/layers/ImageContainer.cpp
@@ -277,18 +277,18 @@ ImageContainer::LockCurrentImage()
 }
 
 TemporaryRef<gfx::SourceSurface>
 ImageContainer::LockCurrentAsSourceSurface(gfx::IntSize *aSize, Image** aCurrentImage)
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
 
   if (aCurrentImage) {
-    NS_IF_ADDREF(mActiveImage);
-    *aCurrentImage = mActiveImage.get();
+    nsRefPtr<Image> activeImage(mActiveImage);
+    activeImage.forget(aCurrentImage);
   }
 
   if (!mActiveImage) {
     return nullptr;
   }
 
   *aSize = mActiveImage->GetSize();
   return mActiveImage->GetAsSourceSurface();
--- a/gfx/layers/client/TiledContentClient.cpp
+++ b/gfx/layers/client/TiledContentClient.cpp
@@ -786,17 +786,17 @@ TileClient::GetTileDescriptor()
   if (IsPlaceholderTile()) {
     return PlaceholderTileDescriptor();
   }
   MOZ_ASSERT(mFrontLock);
   if (mFrontLock->GetType() == gfxSharedReadLock::TYPE_MEMORY) {
     // AddRef here and Release when receiving on the host side to make sure the
     // reference count doesn't go to zero before the host receives the message.
     // see TiledLayerBufferComposite::TiledLayerBufferComposite
-    mFrontLock->AddRef();
+    mFrontLock.get()->AddRef();
   }
 
   if (mFrontLock->GetType() == gfxSharedReadLock::TYPE_MEMORY) {
     return TexturedTileDescriptor(nullptr, mFrontBuffer->GetIPDLActor(),
                                   mFrontBufferOnWhite ? MaybeTexture(mFrontBufferOnWhite->GetIPDLActor()) : MaybeTexture(null_t()),
                                   TileLock(uintptr_t(mFrontLock.get())));
   } else {
     gfxShmSharedReadLock *lock = static_cast<gfxShmSharedReadLock*>(mFrontLock.get());
--- a/gfx/layers/composite/TiledContentHost.cpp
+++ b/gfx/layers/composite/TiledContentHost.cpp
@@ -86,17 +86,17 @@ TiledLayerBufferComposite::TiledLayerBuf
             mIsValid = false;
 
             mRetainedTiles.Clear();
             return;
           }
           sharedLock = reinterpret_cast<gfxMemorySharedReadLock*>(ipcLock.get_uintptr_t());
           if (sharedLock) {
             // The corresponding AddRef is in TiledClient::GetTileDescriptor
-            sharedLock->Release();
+            sharedLock.get()->Release();
           }
         }
 
         CompositableTextureSourceRef textureSource;
         CompositableTextureSourceRef textureSourceOnWhite;
         if (texture) {
           texture->SetCompositor(aCompositor);
           texture->PrepareTextureSource(textureSource);
--- a/gfx/thebes/gfxQuartzSurface.cpp
+++ b/gfx/thebes/gfxQuartzSurface.cpp
@@ -174,17 +174,17 @@ already_AddRefed<gfxImageSurface> gfxQua
         return nullptr;
 
     nsRefPtr<gfxASurface> img = Wrap(surface);
 
     // cairo_quartz_surface_get_image returns a referenced image, and thebes
     // shares the refcounts of Cairo surfaces. However, Wrap also adds a
     // reference to the image. We need to remove one of these references
     // explicitly so we don't leak.
-    img->Release();
+    img.get()->Release();
 
     img->SetOpaqueRect(GetOpaqueRect());
 
     return img.forget().downcast<gfxImageSurface>();
 }
 
 gfxQuartzSurface::~gfxQuartzSurface()
 {
--- a/mfbt/Attributes.h
+++ b/mfbt/Attributes.h
@@ -519,16 +519,21 @@
  * MOZ_NON_OWNING_REF: Applies to declarations of pointer types.  This attribute
  *   tells the compiler that the raw pointer is a weak reference, and that
  *   property is somehow enforced by the code.  This can make the compiler
  *   ignore these pointers when validating the usage of pointers otherwise.
  * MOZ_UNSAFE_REF: Applies to declarations of pointer types.  This attribute
  *   should be used for non-owning references that can be unsafe, and their
  *   safety needs to be validated through code inspection.  The string argument
  *   passed to this macro documents the safety conditions.
+ * MOZ_NO_ADDREF_RELEASE_ON_RETURN: Applies to function declarations.  Makes it
+ *   a compile time error to call AddRef or Release on the return value of a
+ *   function.  This is intended to be used with operator->() of our smart
+ *   pointer classes to ensure that the refcount of an object wrapped in a
+ *   smart pointer is not manipulated directly.
  */
 #ifdef MOZ_CLANG_PLUGIN
 #  define MOZ_MUST_OVERRIDE __attribute__((annotate("moz_must_override")))
 #  define MOZ_STACK_CLASS __attribute__((annotate("moz_stack_class")))
 #  define MOZ_NONHEAP_CLASS __attribute__((annotate("moz_nonheap_class")))
 #  define MOZ_TRIVIAL_CTOR_DTOR __attribute__((annotate("moz_trivial_ctor_dtor")))
 #  ifdef DEBUG
      /* in debug builds, these classes do have non-trivial constructors. */
@@ -537,16 +542,17 @@
 #    define MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS __attribute__((annotate("moz_global_class"))) \
             MOZ_TRIVIAL_CTOR_DTOR
 #  endif
 #  define MOZ_IMPLICIT __attribute__((annotate("moz_implicit")))
 #  define MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT __attribute__((annotate("moz_no_arith_expr_in_arg")))
 #  define MOZ_OWNING_REF __attribute__((annotate("moz_strong_ref")))
 #  define MOZ_NON_OWNING_REF __attribute__((annotate("moz_weak_ref")))
 #  define MOZ_UNSAFE_REF(reason) __attribute__((annotate("moz_strong_ref")))
+#  define MOZ_NO_ADDREF_RELEASE_ON_RETURN __attribute__((annotate("moz_no_addref_release_on_return")))
 /*
  * It turns out that clang doesn't like void func() __attribute__ {} without a
  * warning, so use pragmas to disable the warning. This code won't work on GCC
  * anyways, so the warning is safe to ignore.
  */
 #  define MOZ_HEAP_ALLOCATOR \
     _Pragma("clang diagnostic push") \
     _Pragma("clang diagnostic ignored \"-Wgcc-compat\"") \
@@ -559,16 +565,17 @@
 #  define MOZ_TRIVIAL_CTOR_DTOR /* nothing */
 #  define MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS /* nothing */
 #  define MOZ_IMPLICIT /* nothing */
 #  define MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT /* nothing */
 #  define MOZ_HEAP_ALLOCATOR /* nothing */
 #  define MOZ_OWNING_REF /* nothing */
 #  define MOZ_NON_OWNING_REF /* nothing */
 #  define MOZ_UNSAFE_REF(reason) /* nothing */
+#  define MOZ_NO_ADDREF_RELEASE_ON_RETURN /* nothing */
 #endif /* MOZ_CLANG_PLUGIN */
 
 /*
  * MOZ_THIS_IN_INITIALIZER_LIST is used to avoid a warning when we know that
  * it's safe to use 'this' in an initializer list.
  */
 #ifdef _MSC_VER
 #  define MOZ_THIS_IN_INITIALIZER_LIST() \
--- a/mfbt/RefPtr.h
+++ b/mfbt/RefPtr.h
@@ -270,17 +270,17 @@ public:
   {
     T* tmp = mPtr;
     mPtr = nullptr;
     return TemporaryRef<T>(tmp, DontRef());
   }
 
   T* get() const { return mPtr; }
   operator T*() const { return mPtr; }
-  T* operator->() const { return mPtr; }
+  T* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN { return mPtr; }
   T& operator*() const { return *mPtr; }
   template<typename U>
   operator TemporaryRef<U>() { return TemporaryRef<U>(mPtr); }
 
 private:
   void assign(T* aVal)
   {
     unref(mPtr);
--- a/mfbt/WeakPtr.h
+++ b/mfbt/WeakPtr.h
@@ -190,17 +190,17 @@ public:
   }
 
   // Ensure that mRef is dereferenceable in the uninitialized state.
   WeakPtr() : mRef(new WeakReference(nullptr)) {}
 
   operator T*() const { return mRef->get(); }
   T& operator*() const { return *mRef->get(); }
 
-  T* operator->() const { return mRef->get(); }
+  T* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN { return mRef->get(); }
 
   T* get() const { return mRef->get(); }
 
 private:
   friend class SupportsWeakPtr<T>;
 
   explicit WeakPtr(const RefPtr<WeakReference>& aOther) : mRef(aOther) {}
 
--- a/netwerk/base/src/AutoClose.h
+++ b/netwerk/base/src/AutoClose.h
@@ -46,17 +46,17 @@ public:
   }
 
   void CloseAndRelease()
   {
     Close();
     mPtr = nullptr;
   }
 
-  T* operator->() const
+  T* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
   {
     return mPtr.operator->();
   }
 
 private:
   void Close()
   {
     if (mPtr) {
--- a/netwerk/cache/nsDiskCacheDeviceSQL.cpp
+++ b/netwerk/cache/nsDiskCacheDeviceSQL.cpp
@@ -89,17 +89,17 @@ DecomposeCacheEntryKey(const nsCString *
 }
 
 class AutoResetStatement
 {
   public:
     explicit AutoResetStatement(mozIStorageStatement *s)
       : mStatement(s) {}
     ~AutoResetStatement() { mStatement->Reset(); }
-    mozIStorageStatement *operator->() { return mStatement; }
+    mozIStorageStatement *operator->() MOZ_NO_ADDREF_RELEASE_ON_RETURN { return mStatement; }
   private:
     mozIStorageStatement *mStatement;
 };
 
 class EvictionObserver
 {
   public:
   EvictionObserver(mozIStorageConnection *db,
--- a/netwerk/sctp/datachannel/DataChannel.cpp
+++ b/netwerk/sctp/datachannel/DataChannel.cpp
@@ -2100,18 +2100,20 @@ DataChannelConnection::OpenFinish(alread
     // else if state is CONNECTING, we'll just re-negotiate when OpenFinish
     // is called, if needed
     queue = true;
   }
   if (queue) {
     LOG(("Queuing channel %p (%u) to finish open", channel.get(), stream));
     // Also serves to mark we told the app
     channel->mFlags |= DATA_CHANNEL_FLAGS_FINISH_OPEN;
-    channel->AddRef(); // we need a ref for the nsDeQue and one to return
-    mPending.Push(channel);
+    // we need a ref for the nsDeQue and one to return
+    DataChannel* rawChannel = channel;
+    rawChannel->AddRef();
+    mPending.Push(rawChannel);
     return channel.forget();
   }
 
   MOZ_ASSERT(stream != INVALID_STREAM);
   // just allocated (& OPEN), or externally negotiated
   mStreams[stream] = channel; // holds a reference
   channel->mStream = stream;
 
--- a/netwerk/sctp/datachannel/DataChannel.h
+++ b/netwerk/sctp/datachannel/DataChannel.h
@@ -259,17 +259,17 @@ private:
   // Exists solely for proxying release of the TransportFlow to the STS thread
   static void ReleaseTransportFlow(nsRefPtr<TransportFlow> aFlow) {}
 
   // Data:
   // NOTE: while this array will auto-expand, increases in the number of
   // channels available from the stack must be negotiated!
   bool mAllocateEven;
   nsAutoTArray<nsRefPtr<DataChannel>,16> mStreams;
-  nsDeque mPending; // Holds already_AddRefed<DataChannel>s -- careful!
+  nsDeque mPending; // Holds addref'ed DataChannel's -- careful!
   // holds data that's come in before a channel is open
   nsTArray<nsAutoPtr<QueuedDataMessage> > mQueuedData;
 
   // Streams pending reset
   nsAutoTArray<uint16_t,4> mStreamsResetting;
 
   struct socket *mMasterSocket; // accessed from STS thread
   struct socket *mSocket; // cloned from mMasterSocket on successful Connect on STS thread
--- a/parser/html/nsHtml5RefPtr.h
+++ b/parser/html/nsHtml5RefPtr.h
@@ -199,17 +199,17 @@ class nsHtml5RefPtr
             Prefer the implicit use of this operator to calling |get()|, except where
             necessary to resolve ambiguity.
           */
         {
           return get();
         }
 
       T*
-      operator->() const
+      operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
         {
           NS_PRECONDITION(mRawPtr != 0, "You can't dereference a NULL nsHtml5RefPtr with operator->().");
           return get();
         }
 
       nsHtml5RefPtr<T>*
       get_address()
           // This is not intended to be used by clients.  See |address_of|
--- a/widget/gtk/nsClipboard.cpp
+++ b/widget/gtk/nsClipboard.cpp
@@ -974,17 +974,17 @@ clipboard_contents_received(GtkClipboard
     context->Release();
 }
 
 static GtkSelectionData *
 wait_for_contents(GtkClipboard *clipboard, GdkAtom target)
 {
     RefPtr<RetrievalContext> context = new RetrievalContext();
     // Balanced by Release in clipboard_contents_received
-    context->AddRef();
+    context.get()->AddRef();
     gtk_clipboard_request_contents(clipboard, target,
                                    clipboard_contents_received,
                                    context.get());
     return static_cast<GtkSelectionData*>(context->Wait());
 }
 
 static void
 clipboard_text_received(GtkClipboard *clipboard,
@@ -996,12 +996,12 @@ clipboard_text_received(GtkClipboard *cl
     context->Release();
 }
 
 static gchar *
 wait_for_text(GtkClipboard *clipboard)
 {
     RefPtr<RetrievalContext> context = new RetrievalContext();
     // Balanced by Release in clipboard_text_received
-    context->AddRef();
+    context.get()->AddRef();
     gtk_clipboard_request_text(clipboard, clipboard_text_received, context.get());
     return static_cast<gchar*>(context->Wait());
 }
--- a/xpcom/base/nsRefPtr.h
+++ b/xpcom/base/nsRefPtr.h
@@ -2,16 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsRefPtr_h
 #define nsRefPtr_h
 
+#include "mozilla/Attributes.h"
 #include "AlreadyAddRefed.h"
 #include "nsDebug.h"
 #include "nsISupportsUtils.h"
 
 /*****************************************************************************/
 
 // template <class T> class nsRefPtrGetterAddRefs;
 
@@ -217,17 +218,17 @@ public:
     Prefer the implicit use of this operator to calling |get()|, except where
     necessary to resolve ambiguity.
   */
   {
     return get();
   }
 
   T*
-  operator->() const
+  operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
   {
     NS_PRECONDITION(mRawPtr != 0,
                     "You can't dereference a NULL nsRefPtr with operator->().");
     return get();
   }
 
   // This operator is needed for gcc <= 4.0.* and for Sun Studio; it
   // causes internal compiler errors for some MSVC versions.  (It's not
--- a/xpcom/glue/nsCOMPtr.h
+++ b/xpcom/glue/nsCOMPtr.h
@@ -688,17 +688,17 @@ public:
   // Makes an nsCOMPtr act like its underlying raw pointer type whenever it is
   // used in a context where a raw pointer is expected. It is this operator
   // that makes an nsCOMPtr substitutable for a raw pointer.
   //
   // Prefer the implicit use of this operator to calling |get()|, except where
   // necessary to resolve ambiguity.
   operator T*() const { return get(); }
 
-  T* operator->() const
+  T* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
   {
     NS_ABORT_IF_FALSE(mRawPtr != 0,
                       "You can't dereference a NULL nsCOMPtr with operator->().");
     return get();
   }
 
   // These are not intended to be used by clients. See |address_of| below.
   nsCOMPtr<T>* get_address() { return this; }
@@ -967,17 +967,17 @@ public:
   // Makes an nsCOMPtr act like its underlying raw pointer type whenever it is
   // used in a context where a raw pointer is expected. It is this operator
   // that makes an nsCOMPtr substitutable for a raw pointer.
   //
   // Prefer the implicit use of this operator to calling |get()|, except where
   // necessary to resolve ambiguity/
   operator nsISupports* () const { return get(); }
 
-  nsISupports* operator->() const
+  nsISupports* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
   {
     NS_ABORT_IF_FALSE(mRawPtr != 0,
                       "You can't dereference a NULL nsCOMPtr with operator->().");
     return get();
   }
 
   // These are not intended to be used by clients. See |address_of| below.
   nsCOMPtr<nsISupports>* get_address() { return this; }
--- a/xpcom/glue/nsProxyRelease.h
+++ b/xpcom/glue/nsProxyRelease.h
@@ -214,17 +214,17 @@ public:
   {
     if (mPtr) {
       return mPtr.get()->get();
     }
     return nullptr;
   }
 
   operator T*() { return get(); }
-  T* operator->() { return get(); }
+  T* operator->() MOZ_NO_ADDREF_RELEASE_ON_RETURN { return get(); }
 
   // These are safe to call on other threads with appropriate external locking.
   bool operator==(const nsMainThreadPtrHandle<T>& aOther) const
   {
     if (!mPtr || !aOther.mPtr) {
       return mPtr == aOther.mPtr;
     }
     return *mPtr == *aOther.mPtr;
--- a/xpcom/glue/nsTWeakRef.h
+++ b/xpcom/glue/nsTWeakRef.h
@@ -152,17 +152,17 @@ public:
    * Allow |*this| to be treated as a |Type*| for convenience.
    */
   operator Type*() const { return get(); }
 
   /**
    * Allow |*this| to be treated as a |Type*| for convenience.  Use with
    * caution since this method will crash if the referenced object is null.
    */
-  Type* operator->() const
+  Type* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN
   {
     NS_ASSERTION(mRef && mRef->mObj,
                  "You can't dereference a null weak reference with operator->().");
     return get();
   }
 
 private:
 
--- a/xpcom/tests/TestTimers.cpp
+++ b/xpcom/tests/TestTimers.cpp
@@ -37,17 +37,17 @@ public:
   ~AutoTestThread() {
     mThread->Shutdown();
   }
 
   operator nsIThread*() const {
     return mThread;
   }
 
-  nsIThread* operator->() const {
+  nsIThread* operator->() const MOZ_NO_ADDREF_RELEASE_ON_RETURN {
     return mThread;
   }
 
 private:
   nsCOMPtr<nsIThread> mThread;
 };
 
 class AutoCreateAndDestroyReentrantMonitor