Bug 1397536: Avoid newURI overhead for MatchPattern. r=ehsan,mixedpuppy
authorKris Maglione <maglione.k@gmail.com>
Wed, 06 Sep 2017 21:56:45 -0700
changeset 428907 7282bbabab15133686eef40434da0940a9244cea
parent 428906 30fe3c84b60ee8ae1ab865733041173902892e8f
child 428908 60ac0fadec3127895fa37fe392f4981f4fa75105
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan, mixedpuppy
bugs1397536
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1397536: Avoid newURI overhead for MatchPattern. r=ehsan,mixedpuppy Ehsan, can you please review the (trivial) WebIDL changes, and Shane the WebRequest logic? The change to allow strings in MatchPattern arguments removes a huge amount of XPConnect overhead that accumulates when creating nsIURI objects for WebRequest processing. The change to re-use existing URI objects removes a huge amount of URI creation overhead. MozReview-Commit-ID: 3DJjAKJK1Sa
dom/webidl/ChannelWrapper.webidl
dom/webidl/MatchPattern.webidl
toolkit/components/extensions/MatchPattern.cpp
toolkit/components/extensions/MatchPattern.h
toolkit/components/extensions/ext-webRequest.js
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
@@ -107,16 +107,22 @@ interface ChannelWrapper {
   readonly attribute boolean isSystemLoad;
 
   [Cached, Pure]
   readonly attribute ByteString? originURL;
 
   [Cached, Pure]
   readonly attribute ByteString? documentURL;
 
+  [Pure]
+  readonly attribute URI? originURI;
+
+  [Pure]
+  readonly attribute URI? documentURI;
+
 
   [Cached, GetterThrows, Pure]
   readonly attribute boolean canModify;
 
 
   [Cached, Constant]
   readonly attribute long long windowId;
 
--- a/dom/webidl/MatchPattern.webidl
+++ b/dom/webidl/MatchPattern.webidl
@@ -33,18 +33,22 @@ interface URI;
  ChromeOnly, Exposed=(Window,System)]
 interface MatchPattern {
   /**
    * Returns true if the given URI matches the pattern.
    *
    * If explicit is true, only explicit domain matches, without wildcards, are
    * considered.
    */
+  [Throws]
   boolean matches(URI uri, optional boolean explicit = false);
 
+  [Throws]
+  boolean matches(DOMString url, optional boolean explicit = false);
+
   /**
    * Returns true if a URL exists which a) would be able to access the given
    * cookie, and b) would be matched by this match pattern.
    */
   boolean matchesCookie(Cookie cookie);
 
   /**
    * Returns true if this pattern will match any host which would be matched
@@ -73,18 +77,22 @@ interface MatchPattern {
  ChromeOnly, Exposed=(Window,System)]
 interface MatchPatternSet {
   /**
    * Returns true if the given URI matches any sub-pattern.
    *
    * If explicit is true, only explicit domain matches, without wildcards, are
    * considered.
    */
+  [Throws]
   boolean matches(URI uri, optional boolean explicit = false);
 
+  [Throws]
+  boolean matches(DOMString url, optional boolean explicit = false);
+
   /**
    * Returns true if any sub-pattern matches the given cookie.
    */
   boolean matchesCookie(Cookie cookie);
 
   /**
    * Returns true if any sub-pattern subsumes the given pattern.
    */
--- a/toolkit/components/extensions/MatchPattern.cpp
+++ b/toolkit/components/extensions/MatchPattern.cpp
@@ -372,16 +372,29 @@ MatchPattern::MatchesDomain(const nsACSt
       return true;
     }
   }
 
   return false;
 }
 
 bool
+MatchPattern::Matches(const nsAString& aURL, bool aExplicit, ErrorResult& aRv) const
+{
+  nsCOMPtr<nsIURI> uri;
+  nsresult rv = NS_NewURI(getter_AddRefs(uri), aURL, nullptr, nullptr);
+  if (NS_FAILED(rv)) {
+    aRv.Throw(rv);
+    return false;
+  }
+
+  return Matches(uri.get(), aExplicit);
+}
+
+bool
 MatchPattern::Matches(const URLInfo& aURL, bool aExplicit) const
 {
   if (aExplicit && mMatchSubdomain) {
     return false;
   }
 
   if (!mSchemes->Contains(aURL.Scheme())) {
     return false;
@@ -507,16 +520,29 @@ MatchPatternSet::Constructor(dom::Global
 
   RefPtr<MatchPatternSet> patternSet = new MatchPatternSet(aGlobal.GetAsSupports(),
                                                            Move(patterns));
   return patternSet.forget();
 }
 
 
 bool
+MatchPatternSet::Matches(const nsAString& aURL, bool aExplicit, ErrorResult& aRv) const
+{
+  nsCOMPtr<nsIURI> uri;
+  nsresult rv = NS_NewURI(getter_AddRefs(uri), aURL, nullptr, nullptr);
+  if (NS_FAILED(rv)) {
+    aRv.Throw(rv);
+    return false;
+  }
+
+  return Matches(uri.get(), aExplicit);
+}
+
+bool
 MatchPatternSet::Matches(const URLInfo& aURL, bool aExplicit) const
 {
   for (const auto& pattern : mPatterns) {
     if (pattern->Matches(aURL, aExplicit)) {
       return true;
     }
   }
   return false;
--- a/toolkit/components/extensions/MatchPattern.h
+++ b/toolkit/components/extensions/MatchPattern.h
@@ -202,18 +202,26 @@ class MatchPattern final : public nsISup
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(MatchPattern)
 
   static already_AddRefed<MatchPattern>
   Constructor(dom::GlobalObject& aGlobal,
               const nsAString& aPattern,
               const MatchPatternOptions& aOptions,
               ErrorResult& aRv);
 
+  bool Matches(const nsAString& aURL, bool aExplicit, ErrorResult& aRv) const;
+
   bool Matches(const URLInfo& aURL, bool aExplicit = false) const;
 
+  bool Matches(const URLInfo& aURL, bool aExplicit, ErrorResult& aRv) const
+  {
+    return Matches(aURL, aExplicit);
+  }
+
+
   bool MatchesCookie(const CookieInfo& aCookie) const;
 
   bool MatchesDomain(const nsACString& aDomain) const;
 
   bool Subsumes(const MatchPattern& aPattern) const;
 
   bool Overlaps(const MatchPattern& aPattern) const;
 
@@ -279,18 +287,26 @@ class MatchPatternSet final : public nsI
 
   static already_AddRefed<MatchPatternSet>
   Constructor(dom::GlobalObject& aGlobal,
               const nsTArray<dom::OwningStringOrMatchPattern>& aPatterns,
               const MatchPatternOptions& aOptions,
               ErrorResult& aRv);
 
 
+  bool Matches(const nsAString& aURL, bool aExplicit, ErrorResult& aRv) const;
+
   bool Matches(const URLInfo& aURL, bool aExplicit = false) const;
 
+  bool Matches(const URLInfo& aURL, bool aExplicit, ErrorResult& aRv) const
+  {
+    return Matches(aURL, aExplicit);
+  }
+
+
   bool MatchesCookie(const CookieInfo& aCookie) const;
 
   bool Subsumes(const MatchPattern& aPattern) const;
 
   bool Overlaps(const MatchPattern& aPattern) const;
 
   bool Overlaps(const MatchPatternSet& aPatternSet) const;
 
--- a/toolkit/components/extensions/ext-webRequest.js
+++ b/toolkit/components/extensions/ext-webRequest.js
@@ -20,23 +20,23 @@ function WebRequestEventManager(context,
       // Prevent listening in on requests originating from system principal to
       // prevent tinkering with OCSP, app and addon updates, etc.
       if (data.isSystemPrincipal) {
         return;
       }
 
       // Check hosts permissions for both the resource being requested,
       const hosts = context.extension.whiteListedHosts;
-      if (!hosts.matches(Services.io.newURI(data.url))) {
+      if (!hosts.matches(data.URI)) {
         return;
       }
       // and the origin that is loading the resource.
       const origin = data.documentUrl;
       const own = origin && origin.startsWith(context.extension.getURL());
-      if (origin && !own && !hosts.matches(Services.io.newURI(origin))) {
+      if (origin && !own && !hosts.matches(data.documentURI)) {
         return;
       }
 
       let browserData = {tabId: -1, windowId: -1};
       if (data.browser) {
         browserData = tabTracker.getBrowserData(data.browser);
       }
       if (filter.tabId != null && browserData.tabId != filter.tabId) {
--- a/toolkit/components/extensions/webrequest/ChannelWrapper.cpp
+++ b/toolkit/components/extensions/webrequest/ChannelWrapper.cpp
@@ -379,41 +379,58 @@ ChannelWrapper::GetCanModify(ErrorResult
            AddonManagerWebAPI::IsValidSite(uri))) {
           return false;
       }
     }
   }
   return true;
 }
 
+already_AddRefed<nsIURI>
+ChannelWrapper::GetOriginURI() const
+{
+  nsCOMPtr<nsIURI> uri;
+  if (nsCOMPtr<nsILoadInfo> loadInfo = GetLoadInfo()) {
+    if (nsIPrincipal* prin = loadInfo->TriggeringPrincipal()) {
+      if (prin->GetIsCodebasePrincipal()) {
+        Unused << prin->GetURI(getter_AddRefs(uri));
+      }
+    }
+  }
+  return uri.forget();
+}
+
+already_AddRefed<nsIURI>
+ChannelWrapper::GetDocumentURI() const
+{
+  nsCOMPtr<nsIURI> uri;
+  if (nsCOMPtr<nsILoadInfo> loadInfo = GetLoadInfo()) {
+    if (nsIPrincipal* prin = loadInfo->LoadingPrincipal()) {
+      if (prin->GetIsCodebasePrincipal()) {
+        Unused << prin->GetURI(getter_AddRefs(uri));
+      }
+    }
+  }
+  return uri.forget();
+}
+
+
 void
 ChannelWrapper::GetOriginURL(nsCString& aRetVal) const
 {
-  if (nsCOMPtr<nsILoadInfo> loadInfo = GetLoadInfo()) {
-    if (nsIPrincipal* prin = loadInfo->TriggeringPrincipal()) {
-      nsCOMPtr<nsIURI> uri;
-      if (prin->GetIsCodebasePrincipal() &&
-          NS_SUCCEEDED(prin->GetURI(getter_AddRefs(uri)))) {
-        Unused << uri->GetSpec(aRetVal);
-      }
-    }
+  if (nsCOMPtr<nsIURI> uri = GetOriginURI()) {
+    Unused << uri->GetSpec(aRetVal);
   }
 }
 
 void
 ChannelWrapper::GetDocumentURL(nsCString& aRetVal) const
 {
-  if (nsCOMPtr<nsILoadInfo> loadInfo = GetLoadInfo()) {
-    if (nsIPrincipal* prin = loadInfo->LoadingPrincipal()) {
-      nsCOMPtr<nsIURI> uri;
-      if (prin->GetIsCodebasePrincipal() &&
-          NS_SUCCEEDED(prin->GetURI(getter_AddRefs(uri)))) {
-        Unused << uri->GetSpec(aRetVal);
-      }
-    }
+  if (nsCOMPtr<nsIURI> uri = GetDocumentURI()) {
+    Unused << uri->GetSpec(aRetVal);
   }
 }
 
 int64_t
 NormalizeWindowID(nsILoadInfo* aLoadInfo, uint64_t windowID)
 {
   if (windowID == aLoadInfo->GetTopOuterWindowID()) {
     return 0;
--- a/toolkit/components/extensions/webrequest/ChannelWrapper.h
+++ b/toolkit/components/extensions/webrequest/ChannelWrapper.h
@@ -154,16 +154,20 @@ public:
   int64_t ParentWindowId() const;
 
   bool IsSystemLoad() const;
 
   void GetOriginURL(nsCString& aRetVal) const;
 
   void GetDocumentURL(nsCString& aRetVal) const;
 
+  already_AddRefed<nsIURI> GetOriginURI() const;
+
+  already_AddRefed<nsIURI> GetDocumentURI() const;
+
 
   already_AddRefed<nsILoadContext> GetLoadContext() const;
 
   already_AddRefed<nsIDOMElement> GetBrowserElement() const;
 
 
   bool GetCanModify(ErrorResult& aRv) const;
 
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -209,16 +209,18 @@ var ContentPolicyManager = {
       if (!callback) {
         // It's possible that this listener has been removed and the
         // child hasn't learned yet.
         continue;
       }
       let response = null;
       let listenerKind = "onStop";
       let data = Object.assign({requestId, browser}, msg.data);
+      data.URI = data.url;
+
       delete data.ids;
       try {
         response = callback(data);
         if (response) {
           if (response.cancel) {
             listenerKind = "onError";
             data.error = "NS_ERROR_ABORT";
             return {cancel: true};
@@ -669,23 +671,26 @@ HttpObserverManager = {
     }
     return errorData;
   },
 
   getRequestData(channel, extraData) {
     let data = {
       requestId: String(channel.id),
       url: channel.finalURL,
+      URI: channel.finalURI,
       method: channel.method,
       browser: channel.browserElement,
       type: channel.type,
       fromCache: channel.fromCache,
 
       originUrl: channel.originURL || undefined,
       documentUrl: channel.documentURL || undefined,
+      originURI: channel.originURI,
+      documentURI: channel.documentURI,
       isSystemPrincipal: channel.isSystemLoad,
 
       windowId: channel.windowId,
       parentWindowId: channel.parentWindowId,
 
       ip: channel.remoteAddress,
 
       proxyInfo: channel.proxyInfo,