Bug 1402944: Part 3 - Move error checks into ChannelWrapper. r=mixedpuppy,ehsan draft
authorKris Maglione <maglione.k@gmail.com>
Tue, 26 Sep 2017 13:38:54 -0700
changeset 670802 3ab72773f74a7ef2ed6f9351d1490f89b023f21d
parent 670801 b7b80c928d8c9ce783cd0bcd3c36d98f0ca9aa26
child 670803 14027f62bd457ac283cb7ecefc349169ee1c9686
push id81714
push usermaglione.k@gmail.com
push dateTue, 26 Sep 2017 21:30:45 +0000
reviewersmixedpuppy, ehsan
bugs1402944
milestone58.0a1
Bug 1402944: Part 3 - Move error checks into ChannelWrapper. r=mixedpuppy,ehsan MozReview-Commit-ID: 7uLonYWnLcX
dom/webidl/ChannelWrapper.webidl
toolkit/components/extensions/webrequest/ChannelWrapper.cpp
toolkit/components/extensions/webrequest/ChannelWrapper.h
toolkit/modules/addons/WebRequest.jsm
--- a/dom/webidl/ChannelWrapper.webidl
+++ b/dom/webidl/ChannelWrapper.webidl
@@ -35,17 +35,17 @@ enum MozContentPolicyType {
   "other"
 };
 
 /**
  * A thin wrapper around nsIChannel and nsIHttpChannel that allows JS
  * callers to access them without XPConnect overhead.
  */
 [ChromeOnly, Exposed=System]
-interface ChannelWrapper {
+interface ChannelWrapper : EventTarget {
   /**
    * Returns the wrapper instance for the given channel. The same wrapper is
    * always returned for a given channel.
    */
   static ChannelWrapper get(MozChannel channel);
 
   /**
    * A unique ID for for the requests which remains constant throughout the
@@ -150,16 +150,28 @@ interface ChannelWrapper {
    * This string is used in the error message when notifying extension
    * webRequest listeners of failure. The documentation specifically states
    * that this value MUST NOT be parsed, and is only meant to be displayed to
    * humans, but we all know how that works in real life.
    */
   [Cached, Pure]
   readonly attribute DOMString? errorString;
 
+  /**
+   * Dispatched when the channel is closed with an error status. Check
+   * errorString for the error details.
+   */
+  attribute EventHandler onerror;
+
+  /**
+   * Checks the request's current status and dispatches an error event if the
+   * request has failed and one has not already been dispatched.
+   */
+  void errorCheck();
+
 
   /**
    * Information about the proxy server which is handling this request, or
    * null if the request is not proxied.
    */
   [Cached, Frozen, GetterThrows, Pure]
   readonly attribute MozProxyInfo? proxyInfo;
 
--- a/toolkit/components/extensions/webrequest/ChannelWrapper.cpp
+++ b/toolkit/components/extensions/webrequest/ChannelWrapper.cpp
@@ -14,16 +14,17 @@
 
 #include "NSSErrorsService.h"
 #include "nsITransportSecurityInfo.h"
 
 #include "mozilla/AddonManagerWebAPI.h"
 #include "mozilla/ErrorNames.h"
 #include "mozilla/ResultExtensions.h"
 #include "mozilla/Unused.h"
+#include "mozilla/dom/Event.h"
 #include "nsIContentPolicy.h"
 #include "nsIHttpChannelInternal.h"
 #include "nsIHttpHeaderVisitor.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsILoadContext.h"
 #include "nsILoadGroup.h"
 #include "nsIProxiedChannel.h"
@@ -85,28 +86,32 @@ void
 ChannelWrapper::ClearCachedAttributes()
 {
   ChannelWrapperBinding::ClearCachedFinalURIValue(this);
   ChannelWrapperBinding::ClearCachedFinalURLValue(this);
   ChannelWrapperBinding::ClearCachedProxyInfoValue(this);
   ChannelWrapperBinding::ClearCachedRemoteAddressValue(this);
   ChannelWrapperBinding::ClearCachedStatusCodeValue(this);
   ChannelWrapperBinding::ClearCachedStatusLineValue(this);
+  if (!mFiredErrorEvent) {
+    ChannelWrapperBinding::ClearCachedErrorStringValue(this);
+  }
 }
 
 /*****************************************************************************
  * ...
  *****************************************************************************/
 
 void
 ChannelWrapper::Cancel(uint32_t aResult, ErrorResult& aRv)
 {
   nsresult rv = NS_ERROR_UNEXPECTED;
   if (nsCOMPtr<nsIChannel> chan = MaybeChannel()) {
     rv = chan->Cancel(nsresult(aResult));
+    ErrorCheck();
   }
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);
   }
 }
 
 void
 ChannelWrapper::RedirectTo(nsIURI* aURI, ErrorResult& aRv)
@@ -694,43 +699,73 @@ ChannelWrapper::GetErrorString(nsString&
     } else {
       aRetVal.SetIsVoid(true);
     }
   } else {
     aRetVal.AssignLiteral("NS_ERROR_UNEXPECTED");
   }
 }
 
+void
+ChannelWrapper::ErrorCheck()
+{
+  if (!mFiredErrorEvent) {
+    nsAutoString error;
+    GetErrorString(error);
+    if (error.Length()) {
+      mFiredErrorEvent = true;
+      ChannelWrapperBinding::ClearCachedErrorStringValue(this);
+      FireEvent(NS_LITERAL_STRING("error"));
+    }
+  }
+}
+
+/*****************************************************************************
+ * Event dispatching
+ *****************************************************************************/
+
+void
+ChannelWrapper::FireEvent(const nsAString& aType)
+{
+  EventInit init;
+  init.mBubbles = false;
+  init.mCancelable = false;
+
+  RefPtr<Event> event = Event::Constructor(this, aType, init);
+  event->SetTrusted(true);
+
+  bool defaultPrevented;
+  DispatchEvent(event, &defaultPrevented);
+}
+
 /*****************************************************************************
  * Glue
  *****************************************************************************/
 
 JSObject*
 ChannelWrapper::WrapObject(JSContext* aCx, HandleObject aGivenProto)
 {
   return ChannelWrapperBinding::Wrap(aCx, this, aGivenProto);
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(ChannelWrapper)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ChannelWrapper)
-  NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(ChannelWrapper)
-  NS_INTERFACE_MAP_ENTRY(nsISupports)
-NS_INTERFACE_MAP_END
+NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)
 
-NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ChannelWrapper)
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(ChannelWrapper, DOMEventTargetHelper)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mParent)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
-NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(ChannelWrapper)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(ChannelWrapper, DOMEventTargetHelper)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParent)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
-NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(ChannelWrapper)
+NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(ChannelWrapper, DOMEventTargetHelper)
+NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
-NS_IMPL_CYCLE_COLLECTING_ADDREF(ChannelWrapper)
-NS_IMPL_CYCLE_COLLECTING_RELEASE(ChannelWrapper)
+NS_IMPL_ADDREF_INHERITED(ChannelWrapper, DOMEventTargetHelper)
+NS_IMPL_RELEASE_INHERITED(ChannelWrapper, DOMEventTargetHelper)
 
 } // namespace extensions
 } // namespace mozilla
 
--- a/toolkit/components/extensions/webrequest/ChannelWrapper.h
+++ b/toolkit/components/extensions/webrequest/ChannelWrapper.h
@@ -8,16 +8,17 @@
 #define mozilla_extensions_ChannelWrapper_h
 
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/ChannelWrapperBinding.h"
 
 #include "mozilla/Attributes.h"
 #include "mozilla/Maybe.h"
 
+#include "mozilla/DOMEventTargetHelper.h"
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsIChannel.h"
 #include "nsIHttpChannel.h"
 #include "nsWeakPtr.h"
 #include "nsWrapperCache.h"
 
 #define NS_CHANNELWRAPPER_IID \
@@ -86,23 +87,22 @@ namespace detail {
   private:
     nsWeakPtr mChannel;
 
     mutable nsIChannel* MOZ_NON_OWNING_REF mWeakChannel;
     mutable Maybe<nsIHttpChannel*> MOZ_NON_OWNING_REF mWeakHttpChannel;
   };
 }
 
-class ChannelWrapper final : public nsISupports
-                           , public nsWrapperCache
+class ChannelWrapper final : public DOMEventTargetHelper
                            , private detail::ChannelHolder
 {
 public:
-  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
-  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(ChannelWrapper)
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(ChannelWrapper, DOMEventTargetHelper)
 
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_CHANNELWRAPPER_IID)
 
   static already_AddRefed<extensions::ChannelWrapper> Get(const dom::GlobalObject& global, nsIChannel* channel);
 
 
   uint64_t Id() const { return mId; }
 
@@ -131,16 +131,20 @@ public:
 
 
   uint32_t StatusCode() const;
 
   void GetStatusLine(nsCString& aRetVal) const;
 
   void GetErrorString(nsString& aRetVal) const;
 
+  void ErrorCheck();
+
+  IMPL_EVENT_HANDLER(error);
+
 
   already_AddRefed<nsIURI> GetFinalURI(ErrorResult& aRv) const;
 
   void GetFinalURL(nsCString& aRetVal, ErrorResult& aRv) const;
 
 
   already_AddRefed<nsILoadInfo> GetLoadInfo() const
   {
@@ -207,27 +211,31 @@ private:
   {
     if (!HaveChannel()) {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return false;
     }
     return true;
   }
 
+  void FireEvent(const nsAString& aType);
+
   uint64_t WindowId(nsILoadInfo* aLoadInfo) const;
 
   static uint64_t GetNextId()
   {
     static uint64_t sNextId = 1;
     return ++sNextId;
   }
 
+
   const uint64_t mId = GetNextId();
   nsCOMPtr<nsISupports> mParent;
 
+  bool mFiredErrorEvent = false;
   bool mSuspended = false;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(ChannelWrapper,
                               NS_CHANNELWRAPPER_IID)
 
 } // namespace extensions
 } // namespace mozilla
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -520,16 +520,30 @@ HttpObserverManager = {
     headersReceived: new Map(),
     authRequired: new Map(),
     onRedirect: new Map(),
     onStart: new Map(),
     onError: new Map(),
     onStop: new Map(),
   },
 
+  getWrapper(nativeChannel) {
+    let wrapper = ChannelWrapper.get(nativeChannel);
+    if (!wrapper._addedListeners) {
+      /* eslint-disable mozilla/balanced-listeners */
+      if (this.listeners.onError.size) {
+        wrapper.addEventListener("error", this);
+      }
+      /* eslint-enable mozilla/balanced-listeners */
+
+      wrapper._addedListeners = true;
+    }
+    return wrapper;
+  },
+
   get activityDistributor() {
     return Cc["@mozilla.org/network/http-activity-distributor;1"].getService(Ci.nsIHttpActivityDistributor);
   },
 
   addOrRemove() {
     let needOpening = this.listeners.opening.size;
     let needModify = this.listeners.modify.size || this.listeners.afterModify.size;
     if (needOpening && !this.openingInitialized) {
@@ -597,17 +611,17 @@ HttpObserverManager = {
   },
 
   removeListener(kind, callback) {
     this.listeners[kind].delete(callback);
     this.addOrRemove();
   },
 
   observe(subject, topic, data) {
-    let channel = ChannelWrapper.get(subject);
+    let channel = this.getWrapper(subject);
     switch (topic) {
       case "http-on-modify-request":
         this.runChannelListener(channel, "opening");
         break;
       case "http-on-before-connect":
         this.runChannelListener(channel, "modify");
         break;
       case "http-on-examine-cached-response":
@@ -635,33 +649,39 @@ HttpObserverManager = {
   },
   GOOD_LAST_ACTIVITY: nsIHttpActivityObserver.ACTIVITY_SUBTYPE_RESPONSE_HEADER,
   observeActivity(nativeChannel, activityType, activitySubtype /* , aTimestamp, aExtraSizeData, aExtraStringData */) {
     // Sometimes we get a NullHttpChannel, which implements
     // nsIHttpChannel but not nsIChannel.
     if (!(nativeChannel instanceof Ci.nsIChannel)) {
       return;
     }
-    let channel = ChannelWrapper.get(nativeChannel);
+    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) {
-      if (!this.errorCheck(channel)) {
-        this.runChannelListener(channel, "onError",
-          {error: this.activityErrorsMap.get(lastActivity) ||
-                  `NS_ERROR_NET_UNKNOWN_${lastActivity}`});
-      }
+      // Make a trip through the event loop to make sure errors have a
+      // chance to be processed before we fall back to a generic error
+      // string.
+      Services.tm.dispatchToMainThread(() => {
+        channel.errorCheck();
+        if (!channel.errorString) {
+          this.runChannelListener(channel, "onError",
+            {error: this.activityErrorsMap.get(lastActivity) ||
+                    `NS_ERROR_NET_UNKNOWN_${lastActivity}`});
+        }
+      });
     } else if (lastActivity !== this.GOOD_LAST_ACTIVITY &&
                lastActivity !== nsIHttpActivityObserver.ACTIVITY_SUBTYPE_TRANSACTION_CLOSE) {
       channel.lastActivity = activitySubtype;
     }
   },
 
   shouldRunListener(policyType, uri, filter) {
     // force the protocol to be ws again.
@@ -671,25 +691,16 @@ HttpObserverManager = {
 
     if (filter.types && !filter.types.includes(policyType)) {
       return false;
     }
 
     return !filter.urls || filter.urls.matches(uri);
   },
 
-  errorCheck(channel) {
-    let error = channel.errorString;
-    if (error) {
-      this.runChannelListener(channel, "onError", {error});
-      return true;
-    }
-    return false;
-  },
-
   getRequestData(channel, extraData) {
     let data = {
       requestId: String(channel.id),
       url: channel.finalURL,
       URI: channel.finalURI,
       method: channel.method,
       browser: channel.browserElement,
       type: channel.type,
@@ -742,32 +753,34 @@ HttpObserverManager = {
   destroyFilters(channel) {
     let filters = channel.registeredFilters || new Map();
     for (let [key, filter] of filters.entries()) {
       filter.destruct();
       filters.delete(key);
     }
   },
 
+  handleEvent(event) {
+    let channel = event.currentTarget;
+    switch (event.type) {
+      case "error":
+        this.runChannelListener(
+          channel, "onError", {error: channel.errorString});
+        break;
+    }
+  },
+
   runChannelListener(channel, kind, extraData = null) {
     let handlerResults = [];
     let requestHeaders;
     let responseHeaders;
 
     try {
-      if (this.activityInitialized) {
-        if (kind === "onError") {
-          this.destroyFilters(channel);
-          if (channel.errorNotified) {
-            return;
-          }
-          channel.errorNotified = true;
-        } else if (this.errorCheck(channel)) {
-          return;
-        }
+      if (kind !== "onError" && channel.errorString) {
+        return;
       }
 
       let includeStatus = ["headersReceived", "authRequired", "onRedirect", "onStart", "onStop"].includes(kind);
       let registerFilter = ["opening", "modify", "afterModify", "headersReceived", "authRequired", "onRedirect"].includes(kind);
 
       let commonData = null;
       let uri = channel.finalURI;
       let requestBody;
@@ -837,18 +850,16 @@ HttpObserverManager = {
           if (!result || typeof result !== "object") {
             continue;
           }
         }
 
         if (result.cancel) {
           channel.suspended = false;
           channel.cancel(Cr.NS_ERROR_ABORT);
-
-          this.errorCheck(channel);
           return;
         }
 
         if (result.redirectUrl) {
           try {
             channel.suspended = false;
             channel.redirectTo(Services.io.newURI(result.redirectUrl));
             return;
@@ -873,17 +884,17 @@ HttpObserverManager = {
       // forward the auth request to the base handler.
       if (kind === "authRequired" && channel.authPromptForward) {
         channel.authPromptForward();
       }
 
       if (kind === "modify" && this.listeners.afterModify.size) {
         await this.runChannelListener(channel, "afterModify");
       } else if (kind !== "onError") {
-        this.errorCheck(channel);
+        channel.errorCheck();
       }
     } catch (e) {
       Cu.reportError(e);
     }
 
     // Only resume the channel if it was suspended by this call.
     if (shouldResume) {
       channel.suspended = false;
@@ -928,17 +939,17 @@ HttpObserverManager = {
 
     if (!channel.hasAuthRequestor && this.shouldHookListener(this.listeners.authRequired, channel)) {
       channel.channel.notificationCallbacks = new AuthRequestor(channel.channel, this);
       channel.hasAuthRequestor = true;
     }
   },
 
   onChannelReplaced(oldChannel, newChannel) {
-    let channel = ChannelWrapper.get(oldChannel);
+    let channel = this.getWrapper(oldChannel);
 
     // We want originalURI, this will provide a moz-ext rather than jar or file
     // uri on redirects.
     this.destroyFilters(channel);
     this.runChannelListener(channel, "onRedirect", {redirectUrl: newChannel.originalURI.spec});
     channel.channel = newChannel;
   },