Bug 1254730 - patch 2 - Better comments and a better management of lifetime of ChannelEvents, r=michal, a=ritu
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 14 Mar 2016 18:46:22 +0100
changeset 323496 44d8ad3711809c296cd43844b376413f7e8e3b53
parent 323495 4ff247b3d6e8a5be7f79a387b82c7c0c2e183f89
child 323497 888d64f75c9d12755d9ab85dcaf61bdad454ce54
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmichal, ritu
bugs1254730
milestone47.0a2
Bug 1254730 - patch 2 - Better comments and a better management of lifetime of ChannelEvents, r=michal, a=ritu
netwerk/ipc/ChannelEventQueue.h
netwerk/protocol/ftp/FTPChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.cpp
--- a/netwerk/ipc/ChannelEventQueue.h
+++ b/netwerk/ipc/ChannelEventQueue.h
@@ -47,20 +47,20 @@ class ChannelEventQueue final
     , mOwner(owner)
     , mMutex("ChannelEventQueue::mMutex")
   {}
 
   // Puts IPDL-generated channel event into queue, to be run later
   // automatically when EndForcedQueueing and/or Resume is called.
   //
   // @param aCallback - the ChannelEvent
-  // @param aAssertWhenNotQueued - this optional param will be used in an
+  // @param aAssertionWhenNotQueued - this optional param will be used in an
   //   assertion when the event is executed directly.
   inline void RunOrEnqueue(ChannelEvent* aCallback,
-                           bool aAssertWhenNotQueued = true);
+                           bool aAssertionWhenNotQueued = false);
   inline nsresult PrependEvents(nsTArray<UniquePtr<ChannelEvent>>& aEvents);
 
   // After StartForcedQueueing is called, RunOrEnqueue() will start enqueuing
   // events that will be run/flushed when EndForcedQueueing is called.
   // - Note: queueing may still be required after EndForcedQueueing() (if the
   //   queue is suspended, etc):  always call RunOrEnqueue() to avoid race
   //   conditions.
   inline void StartForcedQueueing();
@@ -104,35 +104,38 @@ class ChannelEventQueue final
   // EventTarget for delivery of events to the correct thread.
   nsCOMPtr<nsIEventTarget> mTargetThread;
 
   friend class AutoEventEnqueuer;
 };
 
 inline void
 ChannelEventQueue::RunOrEnqueue(ChannelEvent* aCallback,
-                                bool aAssertWhenNotQueued)
+                                bool aAssertionWhenNotQueued)
 {
   MOZ_ASSERT(aCallback);
 
+  // To avoid leaks.
+  UniquePtr<ChannelEvent> event(aCallback);
+
   {
     MutexAutoLock lock(mMutex);
 
     bool enqueue =  mForced || mSuspended || mFlushing;
     MOZ_ASSERT(enqueue == true || mEventQueue.IsEmpty(),
                "Should always enqueue if ChannelEventQueue not empty");
 
     if (enqueue) {
-      mEventQueue.AppendElement(aCallback);
+      mEventQueue.AppendElement(Move(event));
       return;
     }
   }
 
-  MOZ_RELEASE_ASSERT(aAssertWhenNotQueued);
-  aCallback->Run();
+  MOZ_RELEASE_ASSERT(!aAssertionWhenNotQueued);
+  event->Run();
 }
 
 inline void
 ChannelEventQueue::StartForcedQueueing()
 {
   MutexAutoLock lock(mMutex);
   mForced = true;
 }
--- a/netwerk/protocol/ftp/FTPChannelChild.cpp
+++ b/netwerk/protocol/ftp/FTPChannelChild.cpp
@@ -372,17 +372,17 @@ FTPChannelChild::RecvOnDataAvailable(con
 {
   MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
                      "Should not be receiving any more callbacks from parent!");
 
   LOG(("FTPChannelChild::RecvOnDataAvailable [this=%p]\n", this));
 
   mEventQ->RunOrEnqueue(new FTPDataAvailableEvent(this, channelStatus, data,
                                                   offset, count),
-                        !mDivertingToParent);
+                        mDivertingToParent);
 
   return true;
 }
 
 class MaybeDivertOnDataFTPEvent : public ChannelEvent
 {
  public:
   MaybeDivertOnDataFTPEvent(FTPChannelChild* child,
@@ -627,17 +627,17 @@ class FTPFlushedForDiversionEvent : publ
 };
 
 bool
 FTPChannelChild::RecvFlushedForDiversion()
 {
   LOG(("FTPChannelChild::RecvFlushedForDiversion [this=%p]\n", this));
   MOZ_ASSERT(mDivertingToParent);
 
-  mEventQ->RunOrEnqueue(new FTPFlushedForDiversionEvent(this));
+  mEventQ->RunOrEnqueue(new FTPFlushedForDiversionEvent(this), true);
   return true;
 }
 
 void
 FTPChannelChild::FlushedForDiversion()
 {
   LOG(("FTPChannelChild::FlushedForDiversion [this=%p]\n", this));
   MOZ_RELEASE_ASSERT(mDivertingToParent);
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -606,17 +606,17 @@ HttpChannelChild::RecvOnTransportAndData
   LOG(("HttpChannelChild::RecvOnTransportAndData [this=%p]\n", this));
   MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
                      "Should not be receiving any more callbacks from parent!");
 
   mEventQ->RunOrEnqueue(new TransportAndDataEvent(this, channelStatus,
                                                   transportStatus, progress,
                                                   progressMax, data, offset,
                                                   count),
-                        !mDivertingToParent);
+                        mDivertingToParent);
   return true;
 }
 
 class MaybeDivertOnDataHttpEvent : public ChannelEvent
 {
  public:
   MaybeDivertOnDataHttpEvent(HttpChannelChild* child,
                              const nsCString& data,
@@ -805,17 +805,18 @@ class StopRequestEvent : public ChannelE
 bool
 HttpChannelChild::RecvOnStopRequest(const nsresult& channelStatus,
                                     const ResourceTimingStruct& timing)
 {
   LOG(("HttpChannelChild::RecvOnStopRequest [this=%p]\n", this));
   MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
     "Should not be receiving any more callbacks from parent!");
 
-  mEventQ->RunOrEnqueue(new StopRequestEvent(this, channelStatus, timing));
+  mEventQ->RunOrEnqueue(new StopRequestEvent(this, channelStatus, timing),
+                        mDivertingToParent);
   return true;
 }
 
 class MaybeDivertOnStopHttpEvent : public ChannelEvent
 {
  public:
   MaybeDivertOnStopHttpEvent(HttpChannelChild* child,
                              const nsresult& channelStatus)
@@ -1334,17 +1335,17 @@ class HttpFlushedForDiversionEvent : pub
 };
 
 bool
 HttpChannelChild::RecvFlushedForDiversion()
 {
   LOG(("HttpChannelChild::RecvFlushedForDiversion [this=%p]\n", this));
   MOZ_RELEASE_ASSERT(mDivertingToParent);
 
-  mEventQ->RunOrEnqueue(new HttpFlushedForDiversionEvent(this));
+  mEventQ->RunOrEnqueue(new HttpFlushedForDiversionEvent(this), true);
 
   return true;
 }
 
 bool
 HttpChannelChild::RecvNotifyTrackingProtectionDisabled()
 {
   nsChannelClassifier::NotifyTrackingProtectionDisabled(this);