Bug 1374922 - FetchConsumer should not continue if the window is destroyed. r=bkelly, a=lizzard
☠☠ backed out by 0a30a2b7945a ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 29 Jun 2017 15:42:00 -0400
changeset 414277 a98632e47afd571c1a850322d86e777783f8b747
parent 414276 bc1c85daa17a0b7bc85c9e0503bdeab005306148
child 414278 0a30a2b7945a2ea01f21a0910bcd72d48a66d8bc
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly, lizzard
bugs1374922
milestone55.0
Bug 1374922 - FetchConsumer should not continue if the window is destroyed. r=bkelly, a=lizzard
dom/fetch/Fetch.cpp
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -6,16 +6,18 @@
 
 #include "Fetch.h"
 
 #include "nsIDocument.h"
 #include "nsIGlobalObject.h"
 #include "nsIStreamLoader.h"
 #include "nsIThreadRetargetableRequest.h"
 #include "nsIUnicodeDecoder.h"
+#include "nsIObserver.h"
+#include "nsWeakReference.h"
 
 #include "nsCharSeparatedTokenizer.h"
 #include "nsDOMString.h"
 #include "nsJSUtils.h"
 #include "nsNetUtil.h"
 #include "nsReadableUtils.h"
 #include "nsStreamUtils.h"
 #include "nsStringStream.h"
@@ -853,62 +855,105 @@ public:
 
   bool Notify(workers::Status aStatus) override;
 };
 
 // FetchBody is not thread-safe but we need to move it around threads.
 // In order to keep it alive all the time, we use a WorkerHolder, if created on
 // workers, plus a wrapper.
 template <class Derived>
-class FetchBodyWrapper final
+class FetchBodyWrapper final : public nsIObserver
+                             , public nsSupportsWeakReference
 {
 public:
   friend class ReleaseObjectHelper;
 
-  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FetchBodyWrapper<Derived>)
+  NS_DECL_THREADSAFE_ISUPPORTS
 
   static already_AddRefed<FetchBodyWrapper<Derived>>
-  Create(FetchBody<Derived>* aBody)
+  Create(FetchBody<Derived>* aBody, nsIGlobalObject* aOwner)
   {
     MOZ_ASSERT(aBody);
 
     RefPtr<FetchBodyWrapper<Derived>> wrapper =
-      new FetchBodyWrapper<Derived>(aBody);
+      new FetchBodyWrapper<Derived>(aBody, aOwner);
 
     if (!NS_IsMainThread()) {
       WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
       MOZ_ASSERT(workerPrivate);
 
       if (!wrapper->RegisterWorkerHolder(workerPrivate)) {
         return nullptr;
       }
+    } else {
+      nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
+      if (NS_WARN_IF(!os)) {
+        return nullptr;
+      }
+
+      nsresult rv = os->AddObserver(wrapper, DOM_WINDOW_DESTROYED_TOPIC, true);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return nullptr;
+      }
+
+      rv = os->AddObserver(wrapper, DOM_WINDOW_FROZEN_TOPIC, true);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return nullptr;
+      }
     }
 
     return wrapper.forget();
   }
 
   void
   ReleaseObject()
   {
     AssertIsOnTargetThread();
 
+    if (NS_IsMainThread()) {
+      nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
+      if (os) {
+        os->RemoveObserver(this, DOM_WINDOW_DESTROYED_TOPIC);
+        os->RemoveObserver(this, DOM_WINDOW_FROZEN_TOPIC);
+      }
+    }
+
     mWorkerHolder = nullptr;
     mBody = nullptr;
   }
 
   FetchBody<Derived>*
   Body() const
   {
     return mBody;
   }
 
+  NS_IMETHOD
+  Observe(nsISupports* aSubject, const char* aTopic,
+          const char16_t* aData) override
+  {
+    AssertIsOnMainThread();
+
+    MOZ_ASSERT((strcmp(aTopic, DOM_WINDOW_FROZEN_TOPIC) == 0) ||
+               (strcmp(aTopic, DOM_WINDOW_DESTROYED_TOPIC) == 0));
+
+    nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(mOwner);
+    if (SameCOMIdentity(aSubject, window)) {
+      mBody->ContinueConsumeBody(this, NS_BINDING_ABORTED, 0, nullptr);
+    }
+
+    return NS_OK;
+  }
+
 private:
-  explicit FetchBodyWrapper(FetchBody<Derived>* aBody)
+  explicit FetchBodyWrapper(FetchBody<Derived>* aBody,
+                            nsIGlobalObject* aOwner)
     : mTargetThread(NS_GetCurrentThread())
     , mBody(aBody)
+    , mOwner(aOwner)
   {}
 
   ~FetchBodyWrapper()
   {
     NS_ProxyRelease(mTargetThread, mBody.forget());
   }
 
   void
@@ -932,24 +977,36 @@ private:
       return false;
     }
 
     return true;
   }
 
   nsCOMPtr<nsIThread> mTargetThread;
   RefPtr<FetchBody<Derived>> mBody;
+  nsCOMPtr<nsIGlobalObject> mOwner;
 
   // Set when consuming the body is attempted on a worker.
   // Unset when consumption is done/aborted.
   // This WorkerHolder keeps alive the wrapper via a cycle.
   UniquePtr<workers::WorkerHolder> mWorkerHolder;
 };
 
 template <class Derived>
+NS_IMPL_ADDREF(FetchBodyWrapper<Derived>)
+
+template <class Derived>
+NS_IMPL_RELEASE(FetchBodyWrapper<Derived>)
+
+template <class Derived>
+NS_IMPL_QUERY_INTERFACE(FetchBodyWrapper<Derived>,
+                        nsIObserver,
+                        nsISupportsWeakReference)
+
+template <class Derived>
 bool
 FetchBodyWorkerHolder<Derived>::Notify(workers::Status aStatus)
 {
   MOZ_ASSERT(aStatus > workers::Running);
   if (!mWasNotified) {
     mWasNotified = true;
     // This will probably cause the releasing of the wrapper.
     // The WorkerHolder will be released as well.
@@ -1293,17 +1350,17 @@ FetchBody<Derived>::BeginConsumeBody()
 {
   AssertIsOnTargetThread();
   MOZ_ASSERT(mConsumePromise);
 
   // The FetchBody is not thread-safe refcounted. We wrap it with a thread-safe
   // object able to keep the current worker alive (if we are running in a
   // worker).
   RefPtr<FetchBodyWrapper<Derived>> wrapper =
-    FetchBodyWrapper<Derived>::Create(this);
+    FetchBodyWrapper<Derived>::Create(this, mOwner);
   if (!wrapper) {
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIRunnable> r = new BeginConsumeBodyRunnable<Derived>(wrapper);
   nsresult rv = NS_OK;
   mMainThreadEventTarget->Dispatch(r.forget(), NS_DISPATCH_NORMAL);
   if (NS_WARN_IF(NS_FAILED(rv))) {