Bug 1658571 - Don't create a reference cycle when catching redirects of a preload. r=smaug

PreloaderBase -> RedirectSink -> PreloaderBase is a strong,
non-cycle-collected reference cycle, which in cases where we don't drop
the channel because we never get an NotifyStop notification, it can
cause leaks.

I'm investigating the root cause of the lack of NotifyStop, but this
should fix the leak and is correct anyhow.

Move the class to the cpp file to ease debugging and changes.

Differential Revision: https://phabricator.services.mozilla.com/D91259

uriloader/preload/PreloaderBase.cpp

@@ -22,11 +22,35 @@ PreloaderBase::UsageTimer::UsageTimer(PreloaderBase* aPreload,
                                       dom::Document* aDocument)
     : mDocument(aDocument), mPreload(aPreload) {}
 
+class PreloaderBase::RedirectSink final : public nsIInterfaceRequestor,
+                                          public nsIChannelEventSink,
+                                          public nsIRedirectResultListener {
+  RedirectSink() = delete;
+  virtual ~RedirectSink();
+
+ public:
+  NS_DECL_THREADSAFE_ISUPPORTS
+  NS_DECL_NSIINTERFACEREQUESTOR
+  NS_DECL_NSICHANNELEVENTSINK
+  NS_DECL_NSIREDIRECTRESULTLISTENER
+
+  RedirectSink(PreloaderBase* aPreloader, nsIInterfaceRequestor* aCallbacks);
+
+ private:
+  WeakPtr<PreloaderBase> mPreloader;
+  nsCOMPtr<nsIInterfaceRequestor> mCallbacks;
+  nsCOMPtr<nsIChannel> mRedirectChannel;
+};
+
 PreloaderBase::RedirectSink::RedirectSink(PreloaderBase* aPreloader,
                                           nsIInterfaceRequestor* aCallbacks)
-    : mPreloader(new nsMainThreadPtrHolder<PreloaderBase>(
+    : mPreloader(aPreloader), mCallbacks(aCallbacks) {}
-          "RedirectSink.mPreloader", aPreloader)),
+
-      mCallbacks(aCallbacks) {}
+PreloaderBase::RedirectSink::~RedirectSink() {
+  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread(),
+                        "Should figure out how to safely drop mPreloader "
+                        "otherwise");
+}
 
 NS_IMPL_ISUPPORTS(PreloaderBase::RedirectSink, nsIInterfaceRequestor,
                   nsIChannelEventSink, nsIRedirectResultListener)
@@ -34,13 +58,17 @@ NS_IMPL_ISUPPORTS(PreloaderBase::RedirectSink, nsIInterfaceRequestor,
 NS_IMETHODIMP PreloaderBase::RedirectSink::AsyncOnChannelRedirect(
     nsIChannel* aOldChannel, nsIChannel* aNewChannel, uint32_t aFlags,
     nsIAsyncVerifyRedirectCallback* aCallback) {
+  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
+
   mRedirectChannel = aNewChannel;
 
   // Deliberately adding this before confirmation.
   nsCOMPtr<nsIURI> uri;
   aNewChannel->GetOriginalURI(getter_AddRefs(uri));
-  mPreloader->mRedirectRecords.AppendElement(
+  if (mPreloader) {
-      RedirectRecord(aFlags, uri.forget()));
+    mPreloader->mRedirectRecords.AppendElement(
+        RedirectRecord(aFlags, uri.forget()));
+  }
 
   if (mCallbacks) {
     nsCOMPtr<nsIChannelEventSink> sink(do_GetInterface(mCallbacks));

uriloader/preload/PreloaderBase.h

@@ -150,25 +150,7 @@ class PreloaderBase : public SupportsWeakPtr, public nsISupports {
   // directly is to keep PreloaderBase as simple as possible so that derived
   // classes don't have to deal with calling super when implementing these
   // interfaces from some reason as well.
-  class RedirectSink final : public nsIInterfaceRequestor,
+  class RedirectSink;
-                             public nsIChannelEventSink,
-                             public nsIRedirectResultListener {
-    RedirectSink() = delete;
-    virtual ~RedirectSink() = default;
-
-   public:
-    NS_DECL_THREADSAFE_ISUPPORTS
-    NS_DECL_NSIINTERFACEREQUESTOR
-    NS_DECL_NSICHANNELEVENTSINK
-    NS_DECL_NSIREDIRECTRESULTLISTENER
-
-    RedirectSink(PreloaderBase* aPreloader, nsIInterfaceRequestor* aCallbacks);
-
-   private:
-    nsMainThreadPtrHandle<PreloaderBase> mPreloader;
-    nsCOMPtr<nsIInterfaceRequestor> mCallbacks;
-    nsCOMPtr<nsIChannel> mRedirectChannel;
-  };
 
   // A timer callback to trigger the unuse warning for this preload
   class UsageTimer final : public nsITimerCallback {