Bug 1402944: Part 9 - Optimize request/response header handling. r=mixedpuppy,ehsan

We don't use the initial Map returned by ChannelWrapper as a map, so there's no
need for the overhead involved in creating it. We also don't need the header map
generated by HeaderChanger unless headers are actually being modified, which
for many listeners they never are, so there's no need for the map creation and
string lower-casing overhead prior to modification time.

MozReview-Commit-ID: K2uK93Oo542

--HG--
extra : rebase_source : f50574fb0eb32878aee3f68b3a73d46ba53987e5

dom/webidl/ChannelWrapper.webidl

@@ -300,24 +300,32 @@ interface ChannelWrapper : EventTarget {
   readonly attribute sequence<MozFrameAncestorInfo>? frameAncestors;
 
   /**
-   * For HTTP requests, returns a Map of request headers which will be, or
-   * have been, sent with this request. Each key is a non-case-normalized
-   * header name string, and each value is its string value.
+   * For HTTP requests, returns an array of request headers which will be, or
+   * have been, sent with this request.
    *
    * For non-HTTP requests, throws NS_ERROR_UNEXPECTED.
    */
   [Throws]
-  object getRequestHeaders();
+  sequence<MozHTTPHeader> getRequestHeaders();
 
   /**
-   * For HTTP requests, returns a Map of response headers which were received
-   * for this request, in the same format as returned by getRequestHeaders.
-   *
+   * For HTTP requests, returns an array of response headers which were
+   * received for this request, in the same format as returned by
+   * getRequestHeaders.
+
    * Throws NS_ERROR_NOT_AVAILABLE if a response has not yet been received, or
    * NS_ERROR_UNEXPECTED if the channel is not an HTTP channel.
+   *
+   * Note: The Content-Type header is handled specially. That header is
+   * usually not mutable after the request has been received, and the content
+   * type must instead be changed via the contentType attribute. If a caller
+   * attempts to set the Content-Type header via setRequestHeader, however,
+   * that value is assigned to the contentType attribute and its original
+   * string value is cached. That original value is returned in place of the
+   * actual Content-Type header.
    */
   [Throws]
-  object getResponseHeaders();
+  sequence<MozHTTPHeader> getResponseHeaders();
 
   /**
    * Sets the given request header to the given value, overwriting any
@@ -335,6 +343,9 @@ interface ChannelWrapper : EventTarget {
    * removing it.
    *
    * For non-HTTP requests, throws NS_ERROR_UNEXPECTED.
+   *
+   * Note: The content type header is handled specially by this function. See
+   * getResponseHeaders() for details.
    */
   [Throws]
   void setResponseHeader(ByteString header, ByteString value);
@@ -389,6 +400,20 @@ dictionary MozFrameAncestorInfo {
   required unsigned long long frameId;
 };
 
+/**
+ * Represents an HTTP request or response header.
+ */
+dictionary MozHTTPHeader {
+  /**
+   * The case-insensitive, non-case-normalized header name.
+   */
+  required ByteString name;
+  /**
+   * The header value.
+   */
+  required ByteString value;
+};
+
 /**
  * An object used for filtering requests.
  */

toolkit/components/extensions/webrequest/ChannelWrapper.cpp

@@ -177,31 +177,24 @@ class MOZ_STACK_CLASS HeaderVisitor final : public nsIHttpHeaderVisitor
 public:
   NS_DECL_NSIHTTPHEADERVISITOR
 
-  explicit HeaderVisitor(JSContext* aCx)
-    : mCx(aCx)
-    , mMap(aCx)
+  explicit HeaderVisitor(nsTArray<dom::MozHTTPHeader>& aHeaders)
+    : mHeaders(aHeaders)
   {}
 
-  JSObject* VisitRequestHeaders(nsIHttpChannel* aChannel, ErrorResult& aRv)
+  HeaderVisitor(nsTArray<dom::MozHTTPHeader>& aHeaders,
+                const nsCString& aContentTypeHdr)
+    : mHeaders(aHeaders)
+    , mContentTypeHdr(aContentTypeHdr)
+  {}
+
+  void VisitRequestHeaders(nsIHttpChannel* aChannel, ErrorResult& aRv)
   {
-    if (!Init()) {
-      return nullptr;
-    }
-    if (!CheckResult(aChannel->VisitRequestHeaders(this), aRv)) {
-      return nullptr;
-    }
-    return mMap;
+    CheckResult(aChannel->VisitRequestHeaders(this), aRv);
   }
 
-  JSObject* VisitResponseHeaders(nsIHttpChannel* aChannel, ErrorResult& aRv)
+  void VisitResponseHeaders(nsIHttpChannel* aChannel, ErrorResult& aRv)
   {
-    if (!Init()) {
-      return nullptr;
-    }
-    if (!CheckResult(aChannel->VisitResponseHeaders(this), aRv)) {
-      return nullptr;
-    }
-    return mMap;
+    CheckResult(aChannel->VisitResponseHeaders(this), aRv);
   }
 
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
@@ -223,18 +216,8 @@ public:
   }
 
 private:
-  bool Init()
-  {
-    mMap = NewMapObject(mCx);
-    return mMap;
-  }
-
   bool CheckResult(nsresult aNSRv, ErrorResult& aRv)
   {
-    if (JS_IsExceptionPending(mCx)) {
-      aRv.NoteJSContextException(mCx);
-      return false;
-    }
     if (NS_FAILED(aNSRv)) {
       aRv.Throw(aNSRv);
       return false;
@@ -242,8 +225,8 @@ private:
     return true;
   }
 
-  JSContext* mCx;
-  RootedObject mMap;
+  nsTArray<dom::MozHTTPHeader>& mHeaders;
+  nsCString mContentTypeHdr = VoidCString();
 
   nsrefcnt mRefCnt = 0;
 };
@@ -251,14 +234,17 @@ private:
 NS_IMETHODIMP
 HeaderVisitor::VisitHeader(const nsACString& aHeader, const nsACString& aValue)
 {
-  RootedValue header(mCx);
-  RootedValue value(mCx);
-
-  if (!xpc::NonVoidStringToJsval(mCx, NS_ConvertUTF8toUTF16(aHeader), &header) ||
-      !xpc::NonVoidStringToJsval(mCx, NS_ConvertUTF8toUTF16(aValue), &value) ||
-      !MapSet(mCx, mMap, header, value)) {
+  auto dict = mHeaders.AppendElement(fallible);
+  if (!dict) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
+  dict->mName = aHeader;
+
+  if (!mContentTypeHdr.IsVoid() && aHeader.LowerCaseEqualsLiteral("content-type")) {
+    dict->mValue = mContentTypeHdr;
+  } else {
+    dict->mValue = aValue;
+  }
 
   return NS_OK;
 }
@@ -269,22 +255,22 @@ NS_IMPL_QUERY_INTERFACE(HeaderVisitor, nsIHttpHeaderVisitor)
 
 
 void
-ChannelWrapper::GetRequestHeaders(JSContext* cx, JS::MutableHandle<JSObject*> aRetVal, ErrorResult& aRv) const
+ChannelWrapper::GetRequestHeaders(nsTArray<dom::MozHTTPHeader>& aRetVal, ErrorResult& aRv) const
 {
   if (nsCOMPtr<nsIHttpChannel> chan = MaybeHttpChannel()) {
-    HeaderVisitor visitor(cx);
-    aRetVal.set(visitor.VisitRequestHeaders(chan, aRv));
+    HeaderVisitor visitor(aRetVal);
+    visitor.VisitRequestHeaders(chan, aRv);
   } else {
     aRv.Throw(NS_ERROR_UNEXPECTED);
   }
 }
 
 void
-ChannelWrapper::GetResponseHeaders(JSContext* cx, JS::MutableHandle<JSObject*> aRetVal, ErrorResult& aRv) const
+ChannelWrapper::GetResponseHeaders(nsTArray<dom::MozHTTPHeader>& aRetVal, ErrorResult& aRv) const
 {
   if (nsCOMPtr<nsIHttpChannel> chan = MaybeHttpChannel()) {
-    HeaderVisitor visitor(cx);
-    aRetVal.set(visitor.VisitResponseHeaders(chan, aRv));
+    HeaderVisitor visitor(aRetVal, mContentTypeHdr);
+    visitor.VisitResponseHeaders(chan, aRv);
   } else {
     aRv.Throw(NS_ERROR_UNEXPECTED);
   }
@@ -307,7 +293,14 @@ ChannelWrapper::SetResponseHeader(const nsCString& aHeader, const nsCString& aVa
 {
   nsresult rv = NS_ERROR_UNEXPECTED;
   if (nsCOMPtr<nsIHttpChannel> chan = MaybeHttpChannel()) {
-    rv = chan->SetResponseHeader(aHeader, aValue, false);
+    if (aHeader.LowerCaseEqualsLiteral("content-type")) {
+      rv = chan->SetContentType(aValue);
+      if (NS_SUCCEEDED(rv)) {
+        mContentTypeHdr = aValue;
+      }
+    } else {
+      rv = chan->SetResponseHeader(aHeader, aValue, false);
+    }
   }
   if (NS_FAILED(rv)) {
     aRv.Throw(rv);

toolkit/components/extensions/webrequest/ChannelWrapper.h

@@ -215,9 +215,9 @@ public:
 
   void GetRemoteAddress(nsCString& aRetVal) const;
 
-  void GetRequestHeaders(JSContext* cx, JS::MutableHandle<JSObject*> aRetVal, ErrorResult& aRv) const;
+  void GetRequestHeaders(nsTArray<dom::MozHTTPHeader>& aRetVal, ErrorResult& aRv) const;
 
-  void GetResponseHeaders(JSContext* cx, JS::MutableHandle<JSObject*> aRetVal, ErrorResult& aRv) const;
+  void GetResponseHeaders(nsTArray<dom::MozHTTPHeader>& aRetVal, ErrorResult& aRv) const;
 
   void SetRequestHeader(const nsCString& header, const nsCString& value, ErrorResult& aRv);
 

toolkit/modules/addons/WebRequest.jsm

@@ -68,14 +68,21 @@ class HeaderChanger {
   constructor(channel) {
     this.channel = channel;
 
-    this.originalHeaders = new Map();
-    for (let [name, value] of this.iterHeaders()) {
-      this.originalHeaders.set(name.toLowerCase(), {name, value});
+    this.array = this.readHeaders();
+  }
+
+  getMap() {
+    if (!this.map) {
+      this.map = new Map();
+      for (let header of this.array) {
+        this.map.set(header.name.toLowerCase(), header);
+      }
     }
+    return this.map;
   }
 
   toArray() {
-    return Array.from(this.originalHeaders.values());
+    return this.array;
   }
 
   validateHeaders(headers) {
@@ -110,7 +117,8 @@ class HeaderChanger {
       ({name}) => name.toLowerCase()));
 
     // Remove missing headers.
-    for (let name of this.originalHeaders.keys()) {
+    let origHeaders = this.getMap();
+    for (let name of origHeaders.keys()) {
       if (!newHeaders.has(name)) {
         this.setHeader(name, "");
       }
@@ -121,7 +129,7 @@ class HeaderChanger {
       if (binaryValue) {
         value = String.fromCharCode(...binaryValue);
       }
-      let original = this.originalHeaders.get(name.toLowerCase());
+      let original = origHeaders.get(name.toLowerCase());
       if (!original || value !== original.value) {
         this.setHeader(name, value);
       }
@@ -138,36 +146,22 @@ class RequestHeaderChanger extends HeaderChanger {
     }
   }
 
-  iterHeaders() {
-    return this.channel.getRequestHeaders().entries();
+  readHeaders() {
+    return this.channel.getRequestHeaders();
   }
 }
 
 class ResponseHeaderChanger extends HeaderChanger {
   setHeader(name, value) {
     try {
-      if (name.toLowerCase() === "content-type" && value) {
-        // The Content-Type header value can't be modified, so we
-        // set the channel's content type directly, instead, and
-        // record that we made the change for the sake of
-        // subsequent observers.
-        this.channel.contentType = value;
-        this.channel._contentType = value;
-      } else {
-        this.channel.setResponseHeader(name, value);
-      }
+      this.channel.setResponseHeader(name, value);
     } catch (e) {
       Cu.reportError(new Error(`Error setting response header ${name}: ${e}`));
     }
   }
 
-  * iterHeaders() {
-    for (let [name, value] of this.channel.getResponseHeaders()) {
-      if (name.toLowerCase() === "content-type") {
-        value = this.channel._contentType || value;
-      }
-      yield [name, value];
-    }
+  readHeaders() {
+    return this.channel.getResponseHeaders();
   }
 }