Forward OnAfterLastPart through nsDocumentOpenInfo so that we don't leak draft
authorMatt Woodrow <mwoodrow@mozilla.com>
Thu, 05 Dec 2019 15:04:18 +1300
changeset 2517419 e19cf79e46cded18e373aeaceed4ed8aaaf4e3d5
parent 2517053 68a5800cb97522afca84b106e06a9aeb91cb1784
child 2517420 dcb9d0517d82e54ceece41ddb5cf383dd97112a1
push id460521
push usermwoodrow@mozilla.com
push dateThu, 05 Dec 2019 03:16:46 +0000
treeherdertry@340b11aac1bc [default view] [failures only]
milestone73.0a1
Forward OnAfterLastPart through nsDocumentOpenInfo so that we don't leak
netwerk/ipc/DocumentLoadListener.cpp
uriloader/base/nsURILoader.cpp
uriloader/base/nsURILoader.h
--- a/netwerk/ipc/DocumentLoadListener.cpp
+++ b/netwerk/ipc/DocumentLoadListener.cpp
@@ -889,16 +889,23 @@ DocumentLoadListener::OnDataAvailable(ns
 }
 
 //-----------------------------------------------------------------------------
 // DoucmentLoadListener::nsIMultiPartChannelListener
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 DocumentLoadListener::OnAfterLastPart(nsresult aStatus) {
+  if (!mInitiatedRedirectToRealChannel) {
+    // if we get here, and we haven't initiated a redirect to a real
+    // channel, then it means we never got OnStartRequest (maybe a problem?)
+    // and we retargeted everything.
+    DisconnectChildListeners(NS_BINDING_RETARGETED, NS_OK);
+    return NS_OK;
+  }
   mStreamListenerFunctions.AppendElement(StreamListenerFunction{
       VariantIndex<3>{}, OnAfterLastPartParams{aStatus}});
   mIsFinished = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DocumentLoadListener::SetParentListener(
--- a/uriloader/base/nsURILoader.cpp
+++ b/uriloader/base/nsURILoader.cpp
@@ -73,16 +73,17 @@ static bool InitPreferences() {
 
 NS_IMPL_ADDREF(nsDocumentOpenInfo)
 NS_IMPL_RELEASE(nsDocumentOpenInfo)
 
 NS_INTERFACE_MAP_BEGIN(nsDocumentOpenInfo)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIRequestObserver)
   NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
   NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
+  NS_INTERFACE_MAP_ENTRY(nsIMultiPartChannelListener)
   NS_INTERFACE_MAP_ENTRY(nsIThreadRetargetableStreamListener)
 NS_INTERFACE_MAP_END
 
 nsDocumentOpenInfo::nsDocumentOpenInfo(nsIInterfaceRequestor* aWindowContext,
                                        uint32_t aFlags, nsURILoader* aURILoader,
                                        BrowsingContext* aBrowsingContext)
     : m_originalContext(aWindowContext),
       mFlags(aFlags),
@@ -98,31 +99,35 @@ nsresult nsDocumentOpenInfo::Prepare() {
   nsresult rv;
 
   // ask our window context if it has a uri content listener...
   m_contentListener = do_GetInterface(m_originalContext, &rv);
   return rv;
 }
 
 NS_IMETHODIMP nsDocumentOpenInfo::OnStartRequest(nsIRequest* request) {
+  nsCOMPtr<nsIMultiPartChannel> multiPartChannel = do_QueryInterface(request);
+  if (multiPartChannel) {
+    mExpectingOnAfterLastPart = true;
+  }
+
   nsresult rv = DoOnStartRequest(request);
 
   // When running normally, we don't do anything to notify the original
   // listener if we retargeted, since the channel will remove itself from
   // the load group, and that triggers OnStart/StopRequest to be sent to
   // the docshell.
   // When running in the parent process on behalf of DocumentChannelParent,
   // we don't have a load group (and implementing one just for this seems like
   // a lot of boilerplate), we can just notify manually that it shouldn't
   // expect any further results.
   if (RefPtr<mozilla::net::DocumentLoadListener> doc =
           do_GetInterface(m_originalContext)) {
     if (mRetargeted) {
       printf("Disconnecting child!  retargeted %d rv %x\n", mRetargeted, rv);
-      doc->DisconnectChildListeners(NS_BINDING_RETARGETED, NS_OK);
     } else if (!m_targetStreamListener) {
       // This is an enormous hack. If DoOnStartRequest returned an
       // error then we just want to forward directly on to our listener
       // and let the existing handling take place (usually switching
       // out PDocumentChannel with PHttpChannel, so that the docshell
       // can access the security error data etc).
       // We trust that this impl of DoContent is infallible, and entirely
       // ignores all parameters, so we're just grabbing the listener
@@ -290,23 +295,62 @@ NS_IMETHODIMP nsDocumentOpenInfo::OnStop
 
     // If this is a multipart stream, we could get another
     // OnStartRequest after this... reset state.
     m_targetStreamListener = nullptr;
     mContentType.Truncate();
     listener->OnStopRequest(request, aStatus);
   }
 
+  // If we're not a multipart channel and we got retargeted, then
+  // we never sent any OnStart/StopRequest to our original listener.
+  // Send OnAfterLastPart as a hack so that it can know to clean up.
+  if (!mExpectingOnAfterLastPart && mRetargeted) {
+    nsCOMPtr<nsIStreamListener> listener;
+    bool abort;
+    m_contentListener->DoContent(mContentType, true, nullptr,
+                                 getter_AddRefs(listener), &abort);
+    MOZ_ASSERT(listener);
+
+    nsCOMPtr<nsIMultiPartChannelListener> multiListener =
+        do_QueryInterface(listener);
+    if (multiListener) {
+      multiListener->OnAfterLastPart(aStatus);
+    }
+  }
+
+  mRetargeted = false;
+
   // Remember...
   // In the case of multiplexed streams (such as multipart/x-mixed-replace)
   // these stream listener methods could be called again :-)
   //
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsDocumentOpenInfo::OnAfterLastPart(nsresult aStatus) {
+  MOZ_ASSERT(mExpectingOnAfterLastPart);
+  // Maybe assert that we haven't forwarded one too?
+
+  nsCOMPtr<nsIStreamListener> listener;
+  bool abort;
+  m_contentListener->DoContent(mContentType, true, nullptr,
+                               getter_AddRefs(listener), &abort);
+  MOZ_ASSERT(listener);
+
+  nsCOMPtr<nsIMultiPartChannelListener> multiListener =
+      do_QueryInterface(listener);
+  if (multiListener) {
+    multiListener->OnAfterLastPart(aStatus);
+  }
+
+  return NS_OK;
+}
+
 nsresult nsDocumentOpenInfo::DispatchContent(nsIRequest* request,
                                              nsISupports* aCtxt) {
   LOG(("[0x%p] nsDocumentOpenInfo::DispatchContent for type '%s'", this,
        mContentType.get()));
 
   MOZ_ASSERT(!m_targetStreamListener,
              "Why do we already have a target stream listener?");
 
--- a/uriloader/base/nsURILoader.h
+++ b/uriloader/base/nsURILoader.h
@@ -12,16 +12,17 @@
 #include "nsCOMPtr.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsString.h"
 #include "nsIWeakReference.h"
 #include "mozilla/Attributes.h"
 #include "nsIStreamListener.h"
 #include "nsIThreadRetargetableStreamListener.h"
+#include "nsIMultiPartChannel.h"
 
 #include "mozilla/Logging.h"
 
 class nsDocumentOpenInfo;
 
 class nsURILoader final : public nsIURILoader {
  public:
   NS_DECL_NSIURILOADER
@@ -58,16 +59,17 @@ class nsURILoader final : public nsIURIL
 
 /**
  * The nsDocumentOpenInfo contains the state required when a single
  * document is being opened in order to discover the content type...
  * Each instance remains alive until its target URL has been loaded
  * (or aborted).
  */
 class nsDocumentOpenInfo final : public nsIStreamListener,
+                                 public nsIMultiPartChannelListener,
                                  public nsIThreadRetargetableStreamListener {
  public:
   // Real constructor
   // aFlags is a combination of the flags on nsIURILoader
   nsDocumentOpenInfo(nsIInterfaceRequestor* aWindowContext, uint32_t aFlags,
                      nsURILoader* aURILoader,
                      mozilla::dom::BrowsingContext* aBrowsingContext = nullptr);
 
@@ -105,16 +107,18 @@ class nsDocumentOpenInfo final : public 
   // nsIRequestObserver methods:
   NS_DECL_NSIREQUESTOBSERVER
 
   // nsIStreamListener methods:
   NS_DECL_NSISTREAMLISTENER
 
   // nsIThreadRetargetableStreamListener
   NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
+
+  NS_DECL_NSIMULTIPARTCHANNELLISTENER
  protected:
   ~nsDocumentOpenInfo();
 
  protected:
   /**
    * The first content listener to try dispatching data to.  Typically
    * the listener associated with the entity that originated the load.
    */
@@ -148,16 +152,18 @@ class nsDocumentOpenInfo final : public 
   /**
    * Reference to the URILoader service so we can access its list of
    * nsIURIContentListeners.
    */
   RefPtr<nsURILoader> mURILoader;
 
   RefPtr<mozilla::dom::BrowsingContext> mBrowsingContext;
 
+  bool mExpectingOnAfterLastPart = false;
+
   /**
    * Limit of data conversion depth to prevent infinite conversion loops
    */
   uint32_t mDataConversionDepthLimit;
 
   bool mRetargeted = false;
 };