Bug 1553400 - XMLHttpRequestDoneNotifier should own XHR, not vice-versa. r=baku a=jcristau
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Wed, 22 May 2019 15:40:10 +0000
changeset 533396 22734638bdcb7d90cdcce00aacaffad82cb8e3b7
parent 533395 37268a8937c5afac3c47fbb0428d3d2f751e3891
child 533397 5234a4220bae688d165edcd04567936f9d8ee823
push id11308
push usernbeleuzu@mozilla.com
push dateFri, 24 May 2019 19:41:29 +0000
treeherdermozilla-beta@298f7d8bb0c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku, jcristau
bugs1553400
milestone68.0
Bug 1553400 - XMLHttpRequestDoneNotifier should own XHR, not vice-versa. r=baku a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D32142
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -226,22 +226,25 @@ XMLHttpRequestMainThread::XMLHttpRequest
       mIsAnon(false),
       mFirstStartRequestSeen(false),
       mInLoadProgressEvent(false),
       mResultJSON(JS::UndefinedValue()),
       mResultArrayBuffer(nullptr),
       mIsMappedArrayBuffer(false),
       mXPCOMifier(nullptr),
       mEventDispatchingSuspended(false),
-      mEofDecoded(false) {
+      mEofDecoded(false),
+      mDelayedDoneNotifier(nullptr) {
   mozilla::HoldJSObjects(this);
 }
 
 XMLHttpRequestMainThread::~XMLHttpRequestMainThread() {
-  DisconnectDoneNotifier();
+  MOZ_ASSERT(
+      !mDelayedDoneNotifier,
+      "How can we have mDelayedDoneNotifier, which owns us, in destructor?");
 
   mFlagDeleted = true;
 
   if ((mState == XMLHttpRequest_Binding::OPENED && mFlagSend) ||
       mState == XMLHttpRequest_Binding::LOADING) {
     Abort();
   }
 
@@ -2229,16 +2232,18 @@ void XMLHttpRequestMainThread::MatchChar
     TruncateResponseText();
     mResponseBodyDecodedPos = 0;
     mEofDecoded = false;
     mDecoder = mResponseXML->GetDocumentCharacterSet()->NewDecoder();
   }
 }
 void XMLHttpRequestMainThread::DisconnectDoneNotifier() {
   if (mDelayedDoneNotifier) {
+    // Disconnect may release the last reference to 'this'.
+    RefPtr<XMLHttpRequestMainThread> kungfuDeathGrip = this;
     mDelayedDoneNotifier->Disconnect();
     mDelayedDoneNotifier = nullptr;
   }
 }
 
 void XMLHttpRequestMainThread::ChangeStateToDone(bool aWasSync) {
   DisconnectDoneNotifier();
 
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -719,17 +719,17 @@ class XMLHttpRequestMainThread final : p
   // Used in lazy decoding to distinguish between having
   // processed all the bytes but not the EOF and having
   // processed all the bytes and the EOF.
   bool mEofDecoded;
 
   // Our parse-end listener, if we are parsing.
   RefPtr<nsXHRParseEndListener> mParseEndListener;
 
-  RefPtr<XMLHttpRequestDoneNotifier> mDelayedDoneNotifier;
+  XMLHttpRequestDoneNotifier* mDelayedDoneNotifier;
   void DisconnectDoneNotifier();
 
   // Any stack information for the point the XHR was opened. This is deleted
   // after the XHR is opened, to avoid retaining references to the worker.
   UniquePtr<SerializedStackHolder> mOriginStack;
 
   static bool sDontWarnAboutSyncXHR;
 };
@@ -790,26 +790,27 @@ class nsXMLHttpRequestXPCOMifier final :
 class XMLHttpRequestDoneNotifier : public Runnable {
  public:
   explicit XMLHttpRequestDoneNotifier(XMLHttpRequestMainThread* aXHR)
       : Runnable("XMLHttpRequestDoneNotifier"), mXHR(aXHR) {}
 
   NS_IMETHOD Run() override {
     if (mXHR) {
       RefPtr<XMLHttpRequestMainThread> xhr = mXHR;
-      mXHR = nullptr;
+      // ChangeStateToDoneInternal ends up calling Disconnect();
       xhr->ChangeStateToDoneInternal();
+      MOZ_ASSERT(!mXHR);
     }
     return NS_OK;
   }
 
   void Disconnect() { mXHR = nullptr; }
 
  private:
-  XMLHttpRequestMainThread* mXHR;
+  RefPtr<XMLHttpRequestMainThread> mXHR;
 };
 
 class nsXHRParseEndListener : public nsIDOMEventListener {
  public:
   NS_DECL_ISUPPORTS
   NS_IMETHOD HandleEvent(Event* event) override {
     if (mXHR) {
       mXHR->OnBodyParseEnd();