Backed out changeset 237acf2879f6 (bug 1407428) for frequent crashes, e.g. in xpcshell's test_bug248970_cookie.js. r=backout on a CLOSED TREE
authorSebastian Hengst <archaeopteryx@coole-files.de>
Mon, 16 Oct 2017 00:00:15 +0200
changeset 386308 994ea93958587c927e5daa729c9393056f655a9b
parent 386307 414400dccb126eab4d8b66603535040771809a24
child 386309 d9cbfa157c86091d29d3e28bce623846c2001a35
push id96211
push userarchaeopteryx@coole-files.de
push dateSun, 15 Oct 2017 22:03:07 +0000
treeherdermozilla-inbound@994ea9395858 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1407428
milestone58.0a1
backs out237acf2879f6222bc4b076c377bf026d18a6ebef
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
Backed out changeset 237acf2879f6 (bug 1407428) for frequent crashes, e.g. in xpcshell's test_bug248970_cookie.js. r=backout on a CLOSED TREE
caps/ExpandedPrincipal.cpp
caps/nsIPrincipal.idl
caps/nsScriptSecurityManager.cpp
dom/base/nsDocument.cpp
dom/xhr/XMLHttpRequestMainThread.cpp
extensions/cookie/nsPermissionManager.cpp
ipc/glue/BackgroundUtils.cpp
js/xpconnect/src/XPCJSContext.cpp
--- a/caps/ExpandedPrincipal.cpp
+++ b/caps/ExpandedPrincipal.cpp
@@ -95,24 +95,25 @@ ExpandedPrincipal::SetDomain(nsIURI* aDo
 }
 
 bool
 ExpandedPrincipal::SubsumesInternal(nsIPrincipal* aOther,
                                     BasePrincipal::DocumentDomainConsideration aConsideration)
 {
   // If aOther is an ExpandedPrincipal too, we break it down into its component
   // nsIPrincipals, and check subsumes on each one.
-  if (Cast(aOther)->Is<ExpandedPrincipal>()) {
-    auto* expanded = Cast(aOther)->As<ExpandedPrincipal>();
-
-    for (auto& other : expanded->WhiteList()) {
+  nsCOMPtr<nsIExpandedPrincipal> expanded = do_QueryInterface(aOther);
+  if (expanded) {
+    nsTArray< nsCOMPtr<nsIPrincipal> >* otherList;
+    expanded->GetWhiteList(&otherList);
+    for (uint32_t i = 0; i < otherList->Length(); ++i){
       // Use SubsumesInternal rather than Subsumes here, since OriginAttribute
       // checks are only done between non-expanded sub-principals, and we don't
       // need to incur the extra virtual call overhead.
-      if (!SubsumesInternal(other, aConsideration)) {
+      if (!SubsumesInternal((*otherList)[i], aConsideration)) {
         return false;
       }
     }
     return true;
   }
 
   // We're dealing with a regular principal. One of our principals must subsume
   // it.
@@ -145,20 +146,21 @@ ExpandedPrincipal::GetHashValue(uint32_t
 
 NS_IMETHODIMP
 ExpandedPrincipal::GetURI(nsIURI** aURI)
 {
   *aURI = nullptr;
   return NS_OK;
 }
 
-const nsTArray<nsCOMPtr<nsIPrincipal>>&
-ExpandedPrincipal::WhiteList()
+NS_IMETHODIMP
+ExpandedPrincipal::GetWhiteList(nsTArray<nsCOMPtr<nsIPrincipal> >** aWhiteList)
 {
-  return mPrincipals;
+  *aWhiteList = &mPrincipals;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 ExpandedPrincipal::GetBaseDomain(nsACString& aBaseDomain)
 {
   return NS_ERROR_NOT_AVAILABLE;
 }
 
--- a/caps/nsIPrincipal.idl
+++ b/caps/nsIPrincipal.idl
@@ -37,17 +37,17 @@ class OriginAttributes;
 %}
 
 interface nsIURI;
 interface nsIContentSecurityPolicy;
 interface nsIDOMDocument;
 
 [ptr] native JSContext(JSContext);
 [ptr] native JSPrincipals(JSPrincipals);
-[ref] native PrincipalArray(const nsTArray<nsCOMPtr<nsIPrincipal>>);
+[ptr] native PrincipalArray(nsTArray<nsCOMPtr<nsIPrincipal> >);
 [ref] native const_OriginAttributes(const mozilla::OriginAttributes);
 
 [scriptable, builtinclass, uuid(f75f502d-79fd-48be-a079-e5a7b8f80c8b)]
 interface nsIPrincipal : nsISerializable
 {
     /**
      * Returns whether the other principal is equivalent to this principal.
      * Principals are considered equal if they are the same principal, or
@@ -335,11 +335,10 @@ interface nsIPrincipal : nsISerializable
 [uuid(f3e177Df-6a5e-489f-80a7-2dd1481471d8)]
 interface nsIExpandedPrincipal : nsISupports
 {
   /**
    * An array of principals that the expanded principal subsumes.
    * Note: this list is not reference counted, it is shared, so
    * should not be changed and should only be used ephemerally.
    */
-  [noscript, notxpcom, nostdcall]
-  PrincipalArray WhiteList();
+  [noscript] readonly attribute PrincipalArray whiteList;
 };
--- a/caps/nsScriptSecurityManager.cpp
+++ b/caps/nsScriptSecurityManager.cpp
@@ -15,17 +15,16 @@
 #include "nsIServiceManager.h"
 #include "nsIScriptObjectPrincipal.h"
 #include "nsIScriptContext.h"
 #include "nsIURL.h"
 #include "nsINestedURI.h"
 #include "nspr.h"
 #include "nsJSPrincipals.h"
 #include "mozilla/BasePrincipal.h"
-#include "ExpandedPrincipal.h"
 #include "SystemPrincipal.h"
 #include "NullPrincipal.h"
 #include "DomainPolicy.h"
 #include "nsString.h"
 #include "nsCRT.h"
 #include "nsCRTGlue.h"
 #include "nsDocShell.h"
 #include "nsError.h"
@@ -664,21 +663,22 @@ nsScriptSecurityManager::CheckLoadURIWit
     if (aPrincipal == mSystemPrincipal) {
         // Allow access
         return NS_OK;
     }
 
     nsCOMPtr<nsIURI> sourceURI;
     aPrincipal->GetURI(getter_AddRefs(sourceURI));
     if (!sourceURI) {
-        auto* basePrin = BasePrincipal::Cast(aPrincipal);
-        if (basePrin->Is<ExpandedPrincipal>()) {
-            auto expanded = basePrin->As<ExpandedPrincipal>();
-            for (auto& prin : expanded->WhiteList()) {
-                nsresult rv = CheckLoadURIWithPrincipal(prin,
+        nsCOMPtr<nsIExpandedPrincipal> expanded = do_QueryInterface(aPrincipal);
+        if (expanded) {
+            nsTArray< nsCOMPtr<nsIPrincipal> > *whiteList;
+            expanded->GetWhiteList(&whiteList);
+            for (uint32_t i = 0; i < whiteList->Length(); ++i) {
+                nsresult rv = CheckLoadURIWithPrincipal((*whiteList)[i],
                                                         aTargetURI,
                                                         aFlags);
                 if (NS_SUCCEEDED(rv)) {
                     // Allow access if it succeeded with one of the white listed principals
                     return NS_OK;
                 }
             }
             // None of our whitelisted principals worked.
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -2489,19 +2489,21 @@ nsDocument::MaybeDowngradePrincipal(nsIP
 
   // We can't load a document with an expanded principal. If we're given one,
   // automatically downgrade it to the last principal it subsumes (which is the
   // extension principal, in the case of extension content scripts).
   auto* basePrin = BasePrincipal::Cast(aPrincipal);
   if (basePrin->Is<ExpandedPrincipal>()) {
     auto* expanded = basePrin->As<ExpandedPrincipal>();
 
-    MOZ_ASSERT(expanded->WhiteList().Length() > 0);
-
-    return do_AddRef(expanded->WhiteList().LastElement().get());
+    nsTArray<nsCOMPtr<nsIPrincipal>>* whitelist;
+    MOZ_ALWAYS_SUCCEEDS(expanded->GetWhiteList(&whitelist));
+    MOZ_ASSERT(whitelist->Length() > 0);
+
+    return do_AddRef(whitelist->LastElement().get());
   }
 
   if (!sChromeInContentPrefCached) {
     sChromeInContentPrefCached = true;
     Preferences::AddBoolVarCache(&sChromeInContentAllowed,
                                  kChromeInContentPref, false);
   }
   if (!sChromeInContentAllowed &&
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -2494,21 +2494,26 @@ XMLHttpRequestMainThread::CreateChannel(
   // Using the provided principal as the triggeringPrincipal is fine, since we
   // want to be able to access any of the origins that the principal has access
   // to during the security checks, but we don't want a document to inherit an
   // expanded principal, so in that case we need to select the principal in the
   // expanded principal's whitelist that can load our URL as principalToInherit.
   nsCOMPtr<nsIPrincipal> resultingDocumentPrincipal(mPrincipal);
   nsCOMPtr<nsIExpandedPrincipal> ep = do_QueryInterface(mPrincipal);
   if (ep) {
+    nsTArray<nsCOMPtr<nsIPrincipal>>* whitelist = nullptr;
+    ep->GetWhiteList(&whitelist);
+    if (!whitelist) {
+      return NS_ERROR_FAILURE;
+    }
     MOZ_ASSERT(!(secFlags & nsILoadInfo::SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS));
     bool dataInherits = (secFlags &
       (nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
        nsILoadInfo::SEC_REQUIRE_CORS_DATA_INHERITS)) != 0;
-    for (const auto& principal : ep->WhiteList()) {
+    for (const auto& principal : *whitelist) {
       if (NS_SUCCEEDED(principal->CheckMayLoad(mRequestURL, false, dataInherits))) {
         resultingDocumentPrincipal = principal;
         break;
       }
     }
   }
 
   nsCOMPtr<nsILoadInfo> loadInfo = mChannel->GetLoadInfo();
--- a/extensions/cookie/nsPermissionManager.cpp
+++ b/extensions/cookie/nsPermissionManager.cpp
@@ -39,17 +39,16 @@
 #include "mozilla/Telemetry.h"
 #include "nsIConsoleService.h"
 #include "nsINavHistoryService.h"
 #include "nsToolkitCompsCID.h"
 #include "nsIObserverService.h"
 #include "nsPrintfCString.h"
 #include "mozilla/AbstractThread.h"
 #include "ContentPrincipal.h"
-#include "ExpandedPrincipal.h"
 
 static nsPermissionManager *gPermissionManager = nullptr;
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 static bool
 IsChildProcess()
@@ -2229,23 +2228,26 @@ nsPermissionManager::CommonTestPermissio
     return NS_OK;
   }
 
   // Set the default.
   *aPermission = nsIPermissionManager::UNKNOWN_ACTION;
 
   // For expanded principals, we want to iterate over the whitelist and see
   // if the permission is granted for any of them.
-  auto* basePrin = BasePrincipal::Cast(aPrincipal);
-  if (basePrin->Is<ExpandedPrincipal>()) {
-    auto ep = basePrin->As<ExpandedPrincipal>();
-    for (auto& prin : ep->WhiteList()) {
+  nsCOMPtr<nsIExpandedPrincipal> ep = do_QueryInterface(aPrincipal);
+  if (ep) {
+    nsTArray<nsCOMPtr<nsIPrincipal>>* whitelist;
+    nsresult rv = ep->GetWhiteList(&whitelist);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    for (size_t i = 0; i < whitelist->Length(); ++i) {
       uint32_t perm;
-      nsresult rv = CommonTestPermission(prin, aType, &perm,
-                                         aExactHostMatch, aIncludingSession);
+      rv = CommonTestPermission(whitelist->ElementAt(i), aType, &perm,
+                                aExactHostMatch, aIncludingSession);
       NS_ENSURE_SUCCESS(rv, rv);
       if (perm == nsIPermissionManager::ALLOW_ACTION) {
         *aPermission = perm;
         return NS_OK;
       } else if (perm == nsIPermissionManager::PROMPT_ACTION) {
         // Store it, but keep going to see if we can do better.
         *aPermission = perm;
       }
--- a/ipc/glue/BackgroundUtils.cpp
+++ b/ipc/glue/BackgroundUtils.cpp
@@ -187,25 +187,28 @@ PrincipalToPrincipalInfo(nsIPrincipal* a
   }
 
   if (isSystemPrincipal) {
     *aPrincipalInfo = SystemPrincipalInfo();
     return NS_OK;
   }
 
   // might be an expanded principal
-  auto* basePrin = BasePrincipal::Cast(aPrincipal);
-  if (basePrin->Is<ExpandedPrincipal>()) {
-    auto* expanded = basePrin->As<ExpandedPrincipal>();
+  nsCOMPtr<nsIExpandedPrincipal> expanded =
+    do_QueryInterface(aPrincipal);
 
+  if (expanded) {
     nsTArray<PrincipalInfo> whitelistInfo;
     PrincipalInfo info;
 
-    for (auto& prin : expanded->WhiteList()) {
-      rv = PrincipalToPrincipalInfo(prin, &info);
+    nsTArray< nsCOMPtr<nsIPrincipal> >* whitelist;
+    MOZ_ALWAYS_SUCCEEDS(expanded->GetWhiteList(&whitelist));
+
+    for (uint32_t i = 0; i < whitelist->Length(); i++) {
+      rv = PrincipalToPrincipalInfo((*whitelist)[i], &info);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       // append that spec to the whitelist
       whitelistInfo.AppendElement(info);
     }
 
     *aPrincipalInfo =
--- a/js/xpconnect/src/XPCJSContext.cpp
+++ b/js/xpconnect/src/XPCJSContext.cpp
@@ -578,17 +578,19 @@ static bool
 IsWebExtensionContentScript(BasePrincipal* principal, nsAString& addonId)
 {
     if (!principal->Is<ExpandedPrincipal>()) {
         return false;
     }
 
     auto expanded = principal->As<ExpandedPrincipal>();
 
-    for (auto& prin : expanded->WhiteList()) {
+    nsTArray<nsCOMPtr<nsIPrincipal>>* principals;
+    expanded->GetWhiteList(&principals);
+    for (auto prin : *principals) {
         if (IsWebExtensionPrincipal(prin, addonId)) {
             return true;
         }
     }
 
     return false;
 }