Bug 964462 - Simplify IPC offline resource refcounting. r=mayhemer, a=sledru
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Thu, 06 Feb 2014 16:16:41 +0200
changeset 176249 1208fe0f787646f9e03afb76234cb1c6ebe5bc6b
parent 176248 959db82272fedee45a4375e9077b7bd5d48236aa
child 176250 c6718f474d3fd2eea6392f8b6b8b3eb1e60f214b
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer, sledru
bugs964462
milestone28.0
Bug 964462 - Simplify IPC offline resource refcounting. r=mayhemer, a=sledru
dom/ipc/TabChild.cpp
uriloader/prefetch/OfflineCacheUpdateChild.cpp
uriloader/prefetch/OfflineCacheUpdateChild.h
uriloader/prefetch/nsOfflineCacheUpdate.cpp
uriloader/prefetch/nsOfflineCacheUpdate.h
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -2092,17 +2092,17 @@ TabChild::AllocPOfflineCacheUpdateChild(
   NS_RUNTIMEABORT("unused");
   return nullptr;
 }
 
 bool
 TabChild::DeallocPOfflineCacheUpdateChild(POfflineCacheUpdateChild* actor)
 {
   OfflineCacheUpdateChild* offlineCacheUpdate = static_cast<OfflineCacheUpdateChild*>(actor);
-  delete offlineCacheUpdate;
+  NS_RELEASE(offlineCacheUpdate);
   return true;
 }
 
 bool
 TabChild::RecvLoadRemoteScript(const nsString& aURL)
 {
   if (!mGlobal && !InitTabChildGlobal())
     // This can happen if we're half-destroyed.  It's not a fatal
--- a/uriloader/prefetch/OfflineCacheUpdateChild.cpp
+++ b/uriloader/prefetch/OfflineCacheUpdateChild.cpp
@@ -62,37 +62,25 @@ namespace docshell {
 //-----------------------------------------------------------------------------
 
 NS_INTERFACE_MAP_BEGIN(OfflineCacheUpdateChild)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
   NS_INTERFACE_MAP_ENTRY(nsIOfflineCacheUpdate)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_ADDREF(OfflineCacheUpdateChild)
-NS_IMPL_RELEASE_WITH_DESTROY(OfflineCacheUpdateChild, RefcountHitZero())
-
-void
-OfflineCacheUpdateChild::RefcountHitZero()
-{
-    if (mIPCActivated) {
-        // ContentChild::DeallocPOfflineCacheUpdate will delete this
-        OfflineCacheUpdateChild::Send__delete__(this);
-    } else {
-        delete this;    // we never opened IPDL channel
-    }
-}
+NS_IMPL_RELEASE(OfflineCacheUpdateChild)
 
 //-----------------------------------------------------------------------------
 // OfflineCacheUpdateChild <public>
 //-----------------------------------------------------------------------------
 
 OfflineCacheUpdateChild::OfflineCacheUpdateChild(nsIDOMWindow* aWindow)
     : mState(STATE_UNINITIALIZED)
     , mIsUpgrade(false)
-    , mIPCActivated(false)
     , mAppID(NECKO_NO_APP_ID)
     , mInBrowser(false)
     , mWindow(aWindow)
     , mByteProgress(0)
 {
 }
 
 OfflineCacheUpdateChild::~OfflineCacheUpdateChild()
@@ -443,18 +431,18 @@ OfflineCacheUpdateChild::Schedule()
     bool stickDocument = mDocument != nullptr; 
 
     // Need to addref ourself here, because the IPC stack doesn't hold
     // a reference to us. Will be released in RecvFinish() that identifies 
     // the work has been done.
     child->SendPOfflineCacheUpdateConstructor(this, manifestURI, documentURI,
                                               stickDocument);
 
-    mIPCActivated = true;
-    this->AddRef();
+    // TabChild::DeallocPOfflineCacheUpdate will release this.
+    NS_ADDREF_THIS();
 
     return NS_OK;
 }
 
 bool
 OfflineCacheUpdateChild::RecvAssociateDocuments(const nsCString &cacheGroupId,
                                                   const nsCString &cacheClientId)
 {
@@ -532,15 +520,16 @@ OfflineCacheUpdateChild::RecvFinish(cons
         observerService->NotifyObservers(static_cast<nsIOfflineCacheUpdate*>(this),
                                          "offline-cache-update-completed",
                                          nullptr);
         LOG(("Done offline-cache-update-completed"));
     }
 
     // This is by contract the last notification from the parent, release
     // us now. This is corresponding to AddRef in Schedule().
-    this->Release();
+    // TabChild::DeallocPOfflineCacheUpdate will call Release.
+    OfflineCacheUpdateChild::Send__delete__(this);
 
     return true;
 }
 
 }
 }
--- a/uriloader/prefetch/OfflineCacheUpdateChild.h
+++ b/uriloader/prefetch/OfflineCacheUpdateChild.h
@@ -48,30 +48,27 @@ public:
     void SetDocument(nsIDOMDocument *aDocument);
 
 private:
     nsresult AssociateDocument(nsIDOMDocument *aDocument,
                                nsIApplicationCache *aApplicationCache);
     void GatherObservers(nsCOMArray<nsIOfflineCacheUpdateObserver> &aObservers);
     nsresult Finish();
 
-    void RefcountHitZero();
-
     enum {
         STATE_UNINITIALIZED,
         STATE_INITIALIZED,
         STATE_CHECKING,
         STATE_DOWNLOADING,
         STATE_CANCELLED,
         STATE_FINISHED
     } mState;
 
     bool mIsUpgrade;
     bool mSucceeded;
-    bool mIPCActivated;
 
     nsCString mUpdateDomain;
     nsCOMPtr<nsIURI> mManifestURI;
     nsCOMPtr<nsIURI> mDocumentURI;
 
     nsCOMPtr<nsIObserverService> mObserverService;
 
     uint32_t mAppID;
--- a/uriloader/prefetch/nsOfflineCacheUpdate.cpp
+++ b/uriloader/prefetch/nsOfflineCacheUpdate.cpp
@@ -1204,17 +1204,16 @@ NS_IMPL_ISUPPORTS3(nsOfflineCacheUpdate,
                    nsIRunnable)
 
 //-----------------------------------------------------------------------------
 // nsOfflineCacheUpdate <public>
 //-----------------------------------------------------------------------------
 
 nsOfflineCacheUpdate::nsOfflineCacheUpdate()
     : mState(STATE_UNINITIALIZED)
-    , mOwner(nullptr)
     , mAddedItems(false)
     , mPartialUpdate(false)
     , mOnlyCheckUpdate(false)
     , mSucceeded(true)
     , mObsolete(false)
     , mAppID(NECKO_NO_APP_ID)
     , mInBrowser(false)
     , mItemsInProgress(0)
@@ -2031,17 +2030,17 @@ nsOfflineCacheUpdate::StickDocument(nsIU
 
     mDocumentURIs.AppendObject(aDocumentURI);
 }
 
 void
 nsOfflineCacheUpdate::SetOwner(nsOfflineCacheUpdateOwner *aOwner)
 {
     NS_ASSERTION(!mOwner, "Tried to set cache update owner twice.");
-    mOwner = aOwner;
+    mOwner = aOwner->asWeakPtr();
 }
 
 bool
 nsOfflineCacheUpdate::IsForGroupID(const nsCSubstring &groupID)
 {
     return mGroupID == groupID;
 }
 
@@ -2152,17 +2151,19 @@ nsOfflineCacheUpdate::FinishNoNotify()
             mApplicationCache->Discard();
         }
     }
 
     nsresult rv = NS_OK;
 
     if (mOwner) {
         rv = mOwner->UpdateFinished(this);
-        mOwner = nullptr;
+        // mozilla::WeakPtr is missing some key features, like setting it to
+        // null explicitly.
+        mOwner = mozilla::WeakPtr<nsOfflineCacheUpdateOwner>();
     }
 
     return rv;
 }
 
 nsresult
 nsOfflineCacheUpdate::Finish()
 {
--- a/uriloader/prefetch/nsOfflineCacheUpdate.h
+++ b/uriloader/prefetch/nsOfflineCacheUpdate.h
@@ -27,16 +27,17 @@
 #include "nsIURI.h"
 #include "nsIWebProgressListener.h"
 #include "nsClassHashtable.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "nsWeakReference.h"
 #include "nsICryptoHash.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/WeakPtr.h"
 #include "nsTHashtable.h"
 #include "nsHashKeys.h"
 
 class nsOfflineCacheUpdate;
 
 class nsICacheEntryDescriptor;
 class nsIUTF8StringEnumerator;
 class nsILoadContext;
@@ -174,18 +175,20 @@ private:
     // manifest hash data
     nsCOMPtr<nsICryptoHash> mManifestHash;
     bool mManifestHashInitialized;
     nsCString mManifestHashValue;
     nsCString mOldManifestHashValue;
 };
 
 class nsOfflineCacheUpdateOwner
+  : public mozilla::SupportsWeakPtr<nsOfflineCacheUpdateOwner>
 {
 public:
+    virtual ~nsOfflineCacheUpdateOwner() {}
     virtual nsresult UpdateFinished(nsOfflineCacheUpdate *aUpdate) = 0;
 };
 
 class nsOfflineCacheUpdate MOZ_FINAL : public nsIOfflineCacheUpdate
                                      , public nsIOfflineCacheUpdateObserver
                                      , public nsIRunnable
                                      , public nsOfflineCacheUpdateOwner
 {
@@ -250,17 +253,17 @@ private:
         STATE_UNINITIALIZED,
         STATE_INITIALIZED,
         STATE_CHECKING,
         STATE_DOWNLOADING,
         STATE_CANCELLED,
         STATE_FINISHED
     } mState;
 
-    nsOfflineCacheUpdateOwner *mOwner;
+    mozilla::WeakPtr<nsOfflineCacheUpdateOwner> mOwner;
 
     bool mAddedItems;
     bool mPartialUpdate;
     bool mOnlyCheckUpdate;
     bool mSucceeded;
     bool mObsolete;
 
     nsCString mUpdateDomain;