Bug 1402944: Part 4 - Fold start/stop listener into ChannelWrapper. r=mixedpuppy,ehsan

MozReview-Commit-ID: 52zZNjgaCEj

--HG--
extra : rebase_source : c608625921d6cbf35dd7c4719df2438a9b0e3905

dom/webidl/ChannelWrapper.webidl

@@ -168,6 +168,17 @@ interface ChannelWrapper : EventTarget {
   void errorCheck();
 
 
+  /**
+   * Dispatched when the channel begins receiving data.
+   */
+  attribute EventHandler onstart;
+
+  /**
+   * Dispatched when the channel has finished receiving data.
+   */
+  attribute EventHandler onstop;
+
+
   /**
    * Information about the proxy server which is handling this request, or
    * null if the request is not proxied.

toolkit/components/build/nsToolkitCompsModule.cpp

@@ -46,8 +46,6 @@
 #include "NativeFileWatcherNotSupported.h"
 #endif // (XP_WIN)
 
-#include "nsWebRequestListener.h"
-
 #if !defined(MOZ_WIDGET_ANDROID)
 #define MOZ_HAS_TERMINATOR
 #endif
@@ -131,8 +129,6 @@ NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(AddonManagerStartup, AddonManagerStartu
 NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(ExtensionPolicyService, ExtensionPolicyService::GetInstance)
 NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR(WebRequestService, WebRequestService::GetInstance)
 
-NS_GENERIC_FACTORY_CONSTRUCTOR(nsWebRequestListener)
-
 NS_DEFINE_NAMED_CID(NS_TOOLKIT_APPSTARTUP_CID);
 #if defined(MOZ_HAS_PERFSTATS)
 NS_DEFINE_NAMED_CID(NS_TOOLKIT_PERFORMANCESTATSSERVICE_CID);
@@ -168,7 +164,6 @@ NS_DEFINE_NAMED_CID(NS_ADDON_MANAGER_STARTUP_CID);
 NS_DEFINE_NAMED_CID(NS_ADDON_POLICY_SERVICE_CID);
 NS_DEFINE_NAMED_CID(NS_WEBREQUEST_SERVICE_CID);
 NS_DEFINE_NAMED_CID(NATIVE_FILEWATCHER_SERVICE_CID);
-NS_DEFINE_NAMED_CID(NS_WEBREQUESTLISTENER_CID);
 
 static const Module::CIDEntry kToolkitCIDs[] = {
   { &kNS_TOOLKIT_APPSTARTUP_CID, false, nullptr, nsAppStartupConstructor },
@@ -205,7 +200,6 @@ static const Module::CIDEntry kToolkitCIDs[] = {
   { &kNS_ADDON_POLICY_SERVICE_CID, false, nullptr, ExtensionPolicyServiceConstructor },
   { &kNS_WEBREQUEST_SERVICE_CID, false, nullptr, WebRequestServiceConstructor },
   { &kNATIVE_FILEWATCHER_SERVICE_CID, false, nullptr, NativeFileWatcherServiceConstructor },
-  { &kNS_WEBREQUESTLISTENER_CID, false, nullptr, nsWebRequestListenerConstructor },
   { nullptr }
 };
 
@@ -245,7 +239,6 @@ static const Module::ContractIDEntry kToolkitContracts[] = {
   { NS_ADDON_POLICY_SERVICE_CONTRACTID, &kNS_ADDON_POLICY_SERVICE_CID },
   { NS_WEBREQUESTSERVICE_CONTRACTID, &kNS_WEBREQUEST_SERVICE_CID },
   { NATIVE_FILEWATCHER_SERVICE_CONTRACTID, &kNATIVE_FILEWATCHER_SERVICE_CID },
-  { NS_WEBREQUESTLISTENER_CONTRACTID, &kNS_WEBREQUESTLISTENER_CID },
   { nullptr }
 };
 

toolkit/components/extensions/test/xpcshell/test_ext_redirects.js

@@ -5,12 +5,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "TestUtils",
                                   "resource://testing-common/TestUtils.jsm");
 const XMLHttpRequest = Components.Constructor("@mozilla.org/xmlextras/xmlhttprequest;1", "nsIXMLHttpRequest");
 
-// nsIWebRequestListener is a nsIThreadRetargetableStreamListener that handles
-// forwarding of nsIRequestObserver for JS consumers.  It does nothing more
-// than that.
-let WebRequestListener = Components.Constructor("@mozilla.org/webextensions/webRequestListener;1",
-                                                "nsIWebRequestListener", "init");
-
 const server = createHttpServer();
 const gServerUrl = `http://localhost:${server.identity.primaryPort}`;
 
@@ -27,20 +21,25 @@ server.registerPathHandler("/dummy", (request, response) => {
 
 function onStopListener(channel) {
   return new Promise(resolve => {
-    new WebRequestListener({
+    let orig = channel.QueryInterface(Ci.nsITraceableChannel).setNewListener({
       QueryInterface: XPCOMUtils.generateQI([Ci.nsIRequestObserver,
                                              Ci.nsIStreamListener]),
       getFinalURI(request) {
         let {loadInfo} = request;
         return (loadInfo && loadInfo.resultPrincipalURI) || request.originalURI;
       },
+      onDataAvailable(...args) {
+        orig.onDataAvailable(...args);
+      },
       onStartRequest(request, context) {
+        orig.onStartRequest(request, context);
       },
       onStopRequest(request, context, statusCode) {
+        orig.onStopRequest(request, context, statusCode);
         let URI = this.getFinalURI(request.QueryInterface(Ci.nsIChannel));
         resolve(URI && URI.spec);
       },
-    }, channel);
+    });
   });
 }
 

toolkit/components/extensions/webrequest/ChannelWrapper.cpp

@@ -29,8 +29,10 @@
 #include "nsILoadGroup.h"
 #include "nsIProxiedChannel.h"
 #include "nsIProxyInfo.h"
+#include "nsITraceableChannel.h"
 #include "nsIWritablePropertyBag2.h"
 #include "nsNetUtil.h"
+#include "nsProxyRelease.h"
 #include "nsPrintfCString.h"
 
 using namespace mozilla::dom;
@@ -772,6 +774,74 @@ ChannelWrapper::ErrorCheck()
   }
 }
 
+/*****************************************************************************
+ * nsIWebRequestListener
+ *****************************************************************************/
+
+NS_IMPL_ISUPPORTS(ChannelWrapper::RequestListener,
+                  nsIStreamListener,
+                  nsIRequestObserver,
+                  nsIThreadRetargetableStreamListener)
+
+ChannelWrapper::RequestListener::~RequestListener() {
+  NS_ReleaseOnMainThreadSystemGroup("RequestListener::mChannelWrapper",
+                                    mChannelWrapper.forget());
+}
+
+nsresult
+ChannelWrapper::RequestListener::Init()
+{
+  if (nsCOMPtr<nsITraceableChannel> chan = mChannelWrapper->QueryChannel()) {
+    return chan->SetNewListener(this, getter_AddRefs(mOrigStreamListener));
+  }
+  return NS_ERROR_UNEXPECTED;
+}
+
+NS_IMETHODIMP
+ChannelWrapper::RequestListener::OnStartRequest(nsIRequest *request, nsISupports * aCtxt)
+{
+  MOZ_ASSERT(mOrigStreamListener, "Should have mOrigStreamListener");
+
+  mChannelWrapper->ErrorCheck();
+  mChannelWrapper->FireEvent(NS_LITERAL_STRING("start"));
+
+  return mOrigStreamListener->OnStartRequest(request, aCtxt);
+}
+
+NS_IMETHODIMP
+ChannelWrapper::RequestListener::OnStopRequest(nsIRequest *request, nsISupports *aCtxt,
+                                               nsresult aStatus)
+{
+  MOZ_ASSERT(mOrigStreamListener, "Should have mOrigStreamListener");
+
+  mChannelWrapper->ErrorCheck();
+  mChannelWrapper->FireEvent(NS_LITERAL_STRING("stop"));
+
+  return mOrigStreamListener->OnStopRequest(request, aCtxt, aStatus);
+}
+
+NS_IMETHODIMP
+ChannelWrapper::RequestListener::OnDataAvailable(nsIRequest *request, nsISupports * aCtxt,
+                                             nsIInputStream * inStr,
+                                             uint64_t sourceOffset, uint32_t count)
+{
+  MOZ_ASSERT(mOrigStreamListener, "Should have mOrigStreamListener");
+  return mOrigStreamListener->OnDataAvailable(request, aCtxt, inStr, sourceOffset, count);
+}
+
+NS_IMETHODIMP
+ChannelWrapper::RequestListener::CheckListenerChain()
+{
+    MOZ_ASSERT(NS_IsMainThread(), "Should be on main thread!");
+    nsresult rv;
+    nsCOMPtr<nsIThreadRetargetableStreamListener> retargetableListener =
+        do_QueryInterface(mOrigStreamListener, &rv);
+    if (retargetableListener) {
+        return retargetableListener->CheckListenerChain();
+    }
+    return rv;
+}
+
 /*****************************************************************************
  * Event dispatching
  *****************************************************************************/
@@ -790,6 +860,31 @@ ChannelWrapper::FireEvent(const nsAString& aType)
   DispatchEvent(event, &defaultPrevented);
 }
 
+void
+ChannelWrapper::CheckEventListeners()
+{
+  if (!mAddedStreamListener && (HasListenersFor(nsGkAtoms::onerror) ||
+                                HasListenersFor(nsGkAtoms::onstart) ||
+                                HasListenersFor(nsGkAtoms::onstop))) {
+    auto listener = MakeRefPtr<RequestListener>(this);
+    if (!NS_WARN_IF(NS_FAILED(listener->Init()))) {
+      mAddedStreamListener = true;
+    }
+  }
+}
+
+void
+ChannelWrapper::EventListenerAdded(nsIAtom* aType)
+{
+  CheckEventListeners();
+}
+
+void
+ChannelWrapper::EventListenerRemoved(nsIAtom* aType)
+{
+  CheckEventListeners();
+}
+
 /*****************************************************************************
  * Glue
  *****************************************************************************/

toolkit/components/extensions/webrequest/ChannelWrapper.h

@@ -18,6 +18,8 @@
 #include "nsCycleCollectionParticipant.h"
 #include "nsIChannel.h"
 #include "nsIHttpChannel.h"
+#include "nsIStreamListener.h"
+#include "nsIThreadRetargetableStreamListener.h"
 #include "nsWeakPtr.h"
 #include "nsWrapperCache.h"
 
@@ -139,6 +141,8 @@ public:
   void ErrorCheck();
 
   IMPL_EVENT_HANDLER(error);
+  IMPL_EVENT_HANDLER(start);
+  IMPL_EVENT_HANDLER(stop);
 
 
   already_AddRefed<nsIURI> GetFinalURI(ErrorResult& aRv) const;
@@ -193,6 +197,12 @@ public:
   void SetResponseHeader(const nsCString& header, const nsCString& value, ErrorResult& aRv);
 
 
+  using EventTarget::EventListenerAdded;
+  using EventTarget::EventListenerRemoved;
+  virtual void EventListenerAdded(nsIAtom* aType) override;
+  virtual void EventListenerRemoved(nsIAtom* aType) override;
+
+
   nsISupports* GetParentObject() const { return mParent; }
 
   JSObject* WrapObject(JSContext* aCx, JS::HandleObject aGivenProto) override;
@@ -229,12 +239,45 @@ private:
     return ++sNextId;
   }
 
+  void CheckEventListeners();
+
+  mutable Maybe<URLInfo> mFinalURLInfo;
+  mutable Maybe<URLInfo> mDocumentURLInfo;
+
+  UniquePtr<WebRequestChannelEntry> mChannelEntry;
+
+  // The overridden Content-Type header value.
+  nsCString mContentTypeHdr = VoidCString();
 
   const uint64_t mId = GetNextId();
   nsCOMPtr<nsISupports> mParent;
 
+  bool mAddedStreamListener = false;
   bool mFiredErrorEvent = false;
   bool mSuspended = false;
+
+
+  class RequestListener final : public nsIStreamListener
+                              , public nsIThreadRetargetableStreamListener
+  {
+  public:
+    NS_DECL_THREADSAFE_ISUPPORTS
+    NS_DECL_NSIREQUESTOBSERVER
+    NS_DECL_NSISTREAMLISTENER
+    NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
+
+    explicit RequestListener(ChannelWrapper* aWrapper)
+      : mChannelWrapper(aWrapper) {}
+
+    nsresult Init();
+
+  protected:
+    virtual ~RequestListener();
+
+  private:
+    RefPtr<ChannelWrapper> mChannelWrapper;
+    nsCOMPtr<nsIStreamListener> mOrigStreamListener;
+  };
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(ChannelWrapper,

toolkit/components/extensions/webrequest/moz.build

@@ -6,14 +6,12 @@
 
 XPIDL_SOURCES += [
     'mozIWebRequestService.idl',
-    'nsIWebRequestListener.idl',
 ]
 
 XPIDL_MODULE = 'webextensions'
 
 UNIFIED_SOURCES += [
     'ChannelWrapper.cpp',
-    'nsWebRequestListener.cpp',
     'StreamFilter.cpp',
     'StreamFilterChild.cpp',
     'StreamFilterEvents.cpp',
@@ -25,10 +23,6 @@ IPDL_SOURCES += [
     'PStreamFilter.ipdl',
 ]
 
-EXPORTS += [
-    'nsWebRequestListener.h',
-]
-
 EXPORTS.mozilla += [
     'WebRequestService.h',
 ]

toolkit/components/extensions/webrequest/nsIWebRequestListener.idl

@@ -1,28 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "nsISupports.idl"
-#include "nsIStreamListener.idl"
-#include "nsITraceableChannel.idl"
-
-/* nsIWebRequestListener is a nsIThreadRetargetableStreamListener that handles
- * forwarding of nsIRequestObserver for JS consumers. nsIWebRequestListener
- * is not cycle collected, JS consumers should not keep a reference to this.
- */
-
-[scriptable, uuid(699a50bb-1f18-2844-b9ea-9f216f62cb18)]
-interface nsIWebRequestListener : nsISupports
-{
-  void init(in nsIStreamListener aStreamListener,
-            in nsITraceableChannel aTraceableChannel);
-};
-
-%{C++
-/* ebea9901-e135-b546-82e2-052666992dbb */
-#define NS_WEBREQUESTLISTENER_CID                   \
- {0xebea9901, 0xe135, 0xb546,                       \
- {0x82, 0xe2, 0x05, 0x26, 0x66, 0x99, 0x2d, 0xbb} }
-#define NS_WEBREQUESTLISTENER_CONTRACTID "@mozilla.org/webextensions/webRequestListener;1"
-%}

toolkit/components/extensions/webrequest/nsWebRequestListener.cpp

@@ -1,72 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "mozilla/ModuleUtils.h"
-#include "nsWebRequestListener.h"
-
-#ifdef DEBUG
-#include "MainThreadUtils.h"
-#endif
-
-using namespace mozilla;
-
-NS_IMPL_ISUPPORTS(nsWebRequestListener,
-                  nsIWebRequestListener,
-                  nsIStreamListener,
-                  nsIRequestObserver,
-                  nsIThreadRetargetableStreamListener)
-
-NS_IMETHODIMP
-nsWebRequestListener::Init(nsIStreamListener *aStreamListener, nsITraceableChannel *aTraceableChannel)
-{
-  MOZ_ASSERT(aStreamListener, "Should have aStreamListener");
-  MOZ_ASSERT(aTraceableChannel, "Should have aTraceableChannel");
-  mTargetStreamListener = aStreamListener;
-  return aTraceableChannel->SetNewListener(this, getter_AddRefs(mOrigStreamListener));
-}
-
-NS_IMETHODIMP
-nsWebRequestListener::OnStartRequest(nsIRequest *request, nsISupports * aCtxt)
-{
-  MOZ_ASSERT(mTargetStreamListener, "Should have mTargetStreamListener");
-  MOZ_ASSERT(mOrigStreamListener, "Should have mOrigStreamListener");
-
-  mTargetStreamListener->OnStartRequest(request, aCtxt);
-
-  return mOrigStreamListener->OnStartRequest(request, aCtxt);
-}
-
-NS_IMETHODIMP
-nsWebRequestListener::OnStopRequest(nsIRequest *request, nsISupports *aCtxt,
-                                           nsresult aStatus)
-{
-  MOZ_ASSERT(mOrigStreamListener, "Should have mOrigStreamListener");
-  MOZ_ASSERT(mTargetStreamListener, "Should have mTargetStreamListener");
-
-  mOrigStreamListener->OnStopRequest(request, aCtxt, aStatus);
-
-  return mTargetStreamListener->OnStopRequest(request, aCtxt, aStatus);
-}
-
-NS_IMETHODIMP
-nsWebRequestListener::OnDataAvailable(nsIRequest *request, nsISupports * aCtxt,
-                                             nsIInputStream * inStr,
-                                             uint64_t sourceOffset, uint32_t count)
-{
-  MOZ_ASSERT(mOrigStreamListener, "Should have mOrigStreamListener");
-  return mOrigStreamListener->OnDataAvailable(request, aCtxt, inStr, sourceOffset, count);
-}
-
-NS_IMETHODIMP
-nsWebRequestListener::CheckListenerChain()
-{
-    MOZ_ASSERT(NS_IsMainThread(), "Should be on main thread!");
-    nsresult rv;
-    nsCOMPtr<nsIThreadRetargetableStreamListener> retargetableListener =
-        do_QueryInterface(mOrigStreamListener, &rv);
-    if (retargetableListener) {
-        return retargetableListener->CheckListenerChain();
-    }
-    return rv;
-}

toolkit/components/extensions/webrequest/nsWebRequestListener.h

@@ -1,40 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#ifndef nsWebRequestListener_h__
-#define nsWebRequestListener_h__
-
-#include "nsCOMPtr.h"
-#include "nsIWebRequestListener.h"
-#include "nsIRequestObserver.h"
-#include "nsIStreamListener.h"
-#include "nsITraceableChannel.h"
-#include "nsIThreadRetargetableStreamListener.h"
-#include "nsProxyRelease.h"
-#include "mozilla/Attributes.h"
-
-class nsWebRequestListener final : public nsIWebRequestListener
-                                 , public nsIStreamListener
-                                 , public nsIThreadRetargetableStreamListener
-{
-public:
-  NS_DECL_THREADSAFE_ISUPPORTS
-  NS_DECL_NSIWEBREQUESTLISTENER
-  NS_DECL_NSIREQUESTOBSERVER
-  NS_DECL_NSISTREAMLISTENER
-  NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
-
-  nsWebRequestListener() {}
-
-private:
-  ~nsWebRequestListener() {
-    NS_ReleaseOnMainThreadSystemGroup("nsWebRequestListener::mTargetStreamListener",
-                                      mTargetStreamListener.forget());
-  }
-  nsCOMPtr<nsIStreamListener> mOrigStreamListener;
-  nsCOMPtr<nsIStreamListener> mTargetStreamListener;
-};
-
-#endif // nsWebRequestListener_h__
-

toolkit/modules/addons/WebRequest.jsm

@@ -33,9 +33,6 @@ XPCOMUtils.defineLazyServiceGetter(this, "webReqService",
 
 XPCOMUtils.defineLazyGetter(this, "ExtensionError", () => ExtensionUtils.ExtensionError);
 
-let WebRequestListener = Components.Constructor("@mozilla.org/webextensions/webRequestListener;1",
-                                                "nsIWebRequestListener", "init");
-
 function runLater(job) {
   Services.tm.dispatchToMainThread(job);
 }
@@ -306,24 +303,6 @@ var ContentPolicyManager = {
 };
 ContentPolicyManager.init();
 
-function StartStopListener(manager, channel) {
-  this.manager = manager;
-  new WebRequestListener(this, channel.channel);
-}
-
-StartStopListener.prototype = {
-  QueryInterface: XPCOMUtils.generateQI([Ci.nsIRequestObserver,
-                                         Ci.nsIStreamListener]),
-
-  onStartRequest: function(request, context) {
-    this.manager.onStartRequest(ChannelWrapper.get(request));
-  },
-
-  onStopRequest(request, context, statusCode) {
-    this.manager.onStopRequest(ChannelWrapper.get(request));
-  },
-};
-
 var ChannelEventSink = {
   _classDescription: "WebRequest channel event sink",
   _classID: Components.ID("115062f8-92f1-11e5-8b7f-080027b0f7ec"),
@@ -532,6 +511,12 @@ HttpObserverManager = {
       if (this.listeners.onError.size) {
         wrapper.addEventListener("error", this);
       }
+      if (this.listeners.onStart.size) {
+        wrapper.addEventListener("start", this);
+      }
+      if (this.listeners.onStop.size) {
+        wrapper.addEventListener("stop", this);
+      }
       /* eslint-enable mozilla/balanced-listeners */
 
       wrapper._addedListeners = true;
@@ -656,13 +641,6 @@ HttpObserverManager = {
     }
     let channel = this.getWrapper(nativeChannel);
 
-    // StartStopListener has to be activated early in the request to catch
-    // SSL connection issues which do not get reported via nsIHttpActivityObserver.
-    if (activityType == nsIHttpActivityObserver.ACTIVITY_TYPE_HTTP_TRANSACTION &&
-        activitySubtype == nsIHttpActivityObserver.ACTIVITY_SUBTYPE_REQUEST_HEADER) {
-      this.attachStartStopListener(channel);
-    }
-
     let lastActivity = channel.lastActivity || 0;
     if (activitySubtype === nsIHttpActivityObserver.ACTIVITY_SUBTYPE_RESPONSE_COMPLETE &&
         lastActivity && lastActivity !== this.GOOD_LAST_ACTIVITY) {
@@ -772,6 +750,13 @@ HttpObserverManager = {
         this.runChannelListener(
           channel, "onError", {error: channel.errorString});
         break;
+      case "start":
+        this.destroyFilters(channel);
+        this.runChannelListener(channel, "onStart");
+        break;
+      case "stop":
+        this.runChannelListener(channel, "onStop");
+        break;
     }
   },
 
@@ -930,25 +915,7 @@ HttpObserverManager = {
     return false;
   },
 
-  attachStartStopListener(channel) {
-    // Check whether we've already added a listener to this channel,
-    // so we don't wind up chaining multiple listeners.
-    if (!this.needTracing || channel.hasListener ||
-        !(channel.channel instanceof Ci.nsITraceableChannel)) {
-      return;
-    }
-
-    // skip redirections, https://bugzilla.mozilla.org/show_bug.cgi?id=728901#c8
-    let {statusCode} = channel;
-    if (statusCode < 300 || statusCode >= 400) {
-      new StartStopListener(this, channel);
-      channel.hasListener = true;
-    }
-  },
-
   examine(channel, topic, data) {
-    this.attachStartStopListener(channel);
-
     if (this.listeners.headersReceived.size) {
       this.runChannelListener(channel, "headersReceived");
     }
@@ -968,15 +935,6 @@ HttpObserverManager = {
     this.runChannelListener(channel, "onRedirect", {redirectUrl: newChannel.originalURI.spec});
     channel.channel = newChannel;
   },
-
-  onStartRequest(channel) {
-    this.destroyFilters(channel);
-    this.runChannelListener(channel, "onStart");
-  },
-
-  onStopRequest(channel) {
-    this.runChannelListener(channel, "onStop");
-  },
 };
 
 var onBeforeRequest = {