Bug 1164977 - Hoist app attributes into a struct on BasePrincipal and refer to them as 'origin attributes'. r=gabor

This sets the stage for the upcoming work for signed apps.

caps/BasePrincipal.cpp

@@ -76,16 +76,16 @@ BasePrincipal::GetIsNullPrincipal(bool* aIsNullPrincipal)
 NS_IMETHODIMP
 BasePrincipal::GetJarPrefix(nsACString& aJarPrefix)
 {
-  MOZ_ASSERT(mAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID);
+  MOZ_ASSERT(AppId() != nsIScriptSecurityManager::UNKNOWN_APP_ID);
 
-  mozilla::GetJarPrefix(mAppId, mIsInBrowserElement, aJarPrefix);
+  mozilla::GetJarPrefix(AppId(), IsInBrowserElement(), aJarPrefix);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetAppStatus(uint16_t* aAppStatus)
 {
-  if (mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
+  if (AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
     NS_WARNING("Asking for app status on a principal with an unknown app id");
     *aAppStatus = nsIPrincipal::APP_STATUS_NOT_INSTALLED;
     return NS_OK;
@@ -98,27 +98,27 @@ BasePrincipal::GetAppStatus(uint16_t* aAppStatus)
 NS_IMETHODIMP
 BasePrincipal::GetAppId(uint32_t* aAppId)
 {
-  if (mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
+  if (AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
     MOZ_ASSERT(false);
     *aAppId = nsIScriptSecurityManager::NO_APP_ID;
     return NS_OK;
   }
 
-  *aAppId = mAppId;
+  *aAppId = AppId();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetIsInBrowserElement(bool* aIsInBrowserElement)
 {
-  *aIsInBrowserElement = mIsInBrowserElement;
+  *aIsInBrowserElement = IsInBrowserElement();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetUnknownAppId(bool* aUnknownAppId)
 {
-  *aUnknownAppId = mAppId == nsIScriptSecurityManager::UNKNOWN_APP_ID;
+  *aUnknownAppId = AppId() == nsIScriptSecurityManager::UNKNOWN_APP_ID;
   return NS_OK;
 }
 

caps/BasePrincipal.h

@@ -23,10 +23,7 @@ namespace mozilla {
 class BasePrincipal : public nsJSPrincipals
 {
 public:
-  BasePrincipal()
-    : mAppId(nsIScriptSecurityManager::NO_APP_ID)
-    , mIsInBrowserElement(false)
-  {}
+  BasePrincipal() {}
 
   enum DocumentDomainConsideration { DontConsiderDocumentDomain, ConsiderDocumentDomain};
   bool Subsumes(nsIPrincipal* aOther, DocumentDomainConsideration aConsideration);
@@ -48,14 +45,35 @@ public:
 
   static BasePrincipal* Cast(nsIPrincipal* aPrin) { return static_cast<BasePrincipal*>(aPrin); }
 
+  struct OriginAttributes {
+    uint32_t mAppId;
+    bool mIsInBrowserElement;
+
+    OriginAttributes() : mAppId(nsIScriptSecurityManager::NO_APP_ID), mIsInBrowserElement(false) {}
+    OriginAttributes(uint32_t aAppId, bool aIsInBrowserElement)
+      : mAppId(aAppId), mIsInBrowserElement(aIsInBrowserElement) {}
+    bool operator==(const OriginAttributes& aOther) const
+    {
+      return mAppId == aOther.mAppId &&
+             mIsInBrowserElement == aOther.mIsInBrowserElement;
+    }
+    bool operator!=(const OriginAttributes& aOther) const
+    {
+      return !(*this == aOther);
+    }
+  };
+
+  const OriginAttributes& OriginAttributesRef() { return mOriginAttributes; }
+  uint32_t AppId() const { return mOriginAttributes.mAppId; }
+  bool IsInBrowserElement() const { return mOriginAttributes.mIsInBrowserElement; }
+
 protected:
   virtual ~BasePrincipal() {}
 
   virtual bool SubsumesInternal(nsIPrincipal* aOther, DocumentDomainConsideration aConsider) = 0;
 
   nsCOMPtr<nsIContentSecurityPolicy> mCSP;
-  uint32_t mAppId;
-  bool mIsInBrowserElement;
+  OriginAttributes mOriginAttributes;
 };
 
 } // namespace mozilla

caps/nsNullPrincipal.cpp

@@ -41,27 +41,25 @@ NS_IMPL_CI_INTERFACE_GETTER(nsNullPrincipal,
 nsNullPrincipal::CreateWithInheritedAttributes(nsIPrincipal* aInheritFrom)
 {
   nsRefPtr<nsNullPrincipal> nullPrin = new nsNullPrincipal();
-  nsresult rv = nullPrin->Init(aInheritFrom->GetAppId(),
-                               aInheritFrom->GetIsInBrowserElement());
+  nsresult rv = nullPrin->Init(Cast(aInheritFrom)->OriginAttributesRef());
   return NS_SUCCEEDED(rv) ? nullPrin.forget() : nullptr;
 }
 
 /* static */ already_AddRefed<nsNullPrincipal>
-nsNullPrincipal::Create(uint32_t aAppId, bool aInMozBrowser)
+nsNullPrincipal::Create(const OriginAttributes& aOriginAttributes)
 {
   nsRefPtr<nsNullPrincipal> nullPrin = new nsNullPrincipal();
-  nsresult rv = nullPrin->Init(aAppId, aInMozBrowser);
+  nsresult rv = nullPrin->Init(aOriginAttributes);
   NS_ENSURE_SUCCESS(rv, nullptr);
 
   return nullPrin.forget();
 }
 
 nsresult
-nsNullPrincipal::Init(uint32_t aAppId, bool aInMozBrowser)
+nsNullPrincipal::Init(const OriginAttributes& aOriginAttributes)
 {
-  MOZ_ASSERT(aAppId != nsIScriptSecurityManager::UNKNOWN_APP_ID);
-  mAppId = aAppId;
-  mIsInBrowserElement = aInMozBrowser;
+  mOriginAttributes = aOriginAttributes;
+  MOZ_ASSERT(AppId() != nsIScriptSecurityManager::UNKNOWN_APP_ID);
 
   mURI = nsNullPrincipalURI::Create();
   NS_ENSURE_TRUE(mURI, NS_ERROR_NOT_AVAILABLE);
@@ -164,10 +162,10 @@ nsNullPrincipal::Read(nsIObjectInputStream* aStream)
   // that the Init() method has already been invoked by the time we deserialize.
   // This is in contrast to nsPrincipal, which uses NS_GENERIC_FACTORY_CONSTRUCTOR,
   // in which case ::Read needs to invoke Init().
-  nsresult rv = aStream->Read32(&mAppId);
+  nsresult rv = aStream->Read32(&mOriginAttributes.mAppId);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = aStream->ReadBoolean(&mIsInBrowserElement);
+  rv = aStream->ReadBoolean(&mOriginAttributes.mIsInBrowserElement);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -176,8 +174,8 @@ nsNullPrincipal::Read(nsIObjectInputStream* aStream)
 NS_IMETHODIMP
 nsNullPrincipal::Write(nsIObjectOutputStream* aStream)
 {
-  aStream->Write32(mAppId);
-  aStream->WriteBoolean(mIsInBrowserElement);
+  aStream->Write32(AppId());
+  aStream->WriteBoolean(IsInBrowserElement());
   return NS_OK;
 }
 

caps/nsNullPrincipal.h

@@ -54,11 +54,9 @@ public:
 
   // Returns null on failure.
   static already_AddRefed<nsNullPrincipal>
-    Create(uint32_t aAppId = nsIScriptSecurityManager::NO_APP_ID,
-           bool aInMozBrowser = false);
+    Create(const OriginAttributes& aOriginAttributes = OriginAttributes());
 
-  nsresult Init(uint32_t aAppId = nsIScriptSecurityManager::NO_APP_ID,
-                bool aInMozBrowser = false);
+  nsresult Init(const OriginAttributes& aOriginAttributes = OriginAttributes());
 
   virtual void GetScriptLocation(nsACString &aStr) override;
 

caps/nsPrincipal.cpp

@@ -78,9 +78,7 @@ nsPrincipal::~nsPrincipal()
 { }
 
 nsresult
-nsPrincipal::Init(nsIURI *aCodebase,
-                  uint32_t aAppId,
-                  bool aInMozBrowser)
+nsPrincipal::Init(nsIURI *aCodebase, const OriginAttributes& aOriginAttributes)
 {
   NS_ENSURE_STATE(!mInitialized);
   NS_ENSURE_ARG(aCodebase);
@@ -89,9 +87,7 @@ nsPrincipal::Init(nsIURI *aCodebase,
 
   mCodebase = NS_TryToMakeImmutable(aCodebase);
   mCodebaseImmutable = URIIsImmutable(mCodebase);
-
-  mAppId = aAppId;
-  mIsInBrowserElement = aInMozBrowser;
+  mOriginAttributes = aOriginAttributes;
 
   return NS_OK;
 }
@@ -172,8 +168,8 @@ nsPrincipal::SubsumesInternal(nsIPrincipal* aOther,
     return true;
   }
 
-  if (!nsScriptSecurityManager::AppAttributesEqual(this, aOther)) {
-      return false;
+  if (OriginAttributesRef() != Cast(aOther)->OriginAttributesRef()) {
+    return false;
   }
 
   // If either the subject or the object has changed its principal by
@@ -380,7 +376,8 @@ nsPrincipal::Read(nsIObjectInputStream* aStream)
   // This may be null.
   nsCOMPtr<nsIContentSecurityPolicy> csp = do_QueryInterface(supports, &rv);
 
-  rv = Init(codebase, appId, inMozBrowser);
+  OriginAttributes attrs(appId, inMozBrowser);
+  rv = Init(codebase, attrs);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = SetCsp(csp);
@@ -414,8 +411,8 @@ nsPrincipal::Write(nsIObjectOutputStream* aStream)
     return rv;
   }
 
-  aStream->Write32(mAppId);
-  aStream->WriteBoolean(mIsInBrowserElement);
+  aStream->Write32(AppId());
+  aStream->WriteBoolean(IsInBrowserElement());
 
   rv = NS_WriteOptionalCompoundObject(aStream, mCSP,
                                       NS_GET_IID(nsIContentSecurityPolicy),

caps/nsPrincipal.h

@@ -34,9 +34,7 @@ public:
   nsPrincipal();
 
   // Init() must be called before the principal is in a usable state.
-  nsresult Init(nsIURI* aCodebase,
-                uint32_t aAppId,
-                bool aInMozBrowser);
+  nsresult Init(nsIURI* aCodebase, const OriginAttributes& aOriginAttributes);
 
   virtual void GetScriptLocation(nsACString& aStr) override;
   void SetURI(nsIURI* aURI);

caps/nsScriptSecurityManager.cpp

@@ -487,26 +487,6 @@ nsScriptSecurityManager::HashPrincipalByOrigin(nsIPrincipal* aPrincipal)
     return SecurityHashURI(uri);
 }
 
-/* static */ bool
-nsScriptSecurityManager::AppAttributesEqual(nsIPrincipal* aFirst,
-                                            nsIPrincipal* aSecond)
-{
-    MOZ_ASSERT(aFirst && aSecond, "Don't pass null pointers!");
-
-    uint32_t firstAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
-    if (!aFirst->GetUnknownAppId()) {
-        firstAppId = aFirst->GetAppId();
-    }
-
-    uint32_t secondAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
-    if (!aSecond->GetUnknownAppId()) {
-        secondAppId = aSecond->GetAppId();
-    }
-
-    return ((firstAppId == secondAppId) &&
-            (aFirst->GetIsInBrowserElement() == aSecond->GetIsInBrowserElement()));
-}
-
 NS_IMETHODIMP
 nsScriptSecurityManager::CheckLoadURIFromScript(JSContext *cx, nsIURI *aURI)
 {
@@ -1022,9 +1002,9 @@ nsScriptSecurityManager::CreateCodebasePrincipal(nsIURI* aURI, uint32_t aAppId,
         return NS_OK;
     }
 
+    BasePrincipal::OriginAttributes attrs(aAppId, aInMozBrowser);
     nsRefPtr<nsPrincipal> codebase = new nsPrincipal();
-
-    nsresult rv = codebase->Init(aURI, aAppId, aInMozBrowser);
+    nsresult rv = codebase->Init(aURI, attrs);
     if (NS_FAILED(rv))
         return rv;
 

caps/nsScriptSecurityManager.h

@@ -80,22 +80,6 @@ public:
         return sStrictFileOriginPolicy;
     }
 
-    /**
-     * Returns true if the two principals share the same app attributes.
-     *
-     * App attributes are appId and the inBrowserElement flag.
-     * Two principals have the same app attributes if those information are
-     * equals.
-     * This method helps keeping principals from different apps isolated from
-     * each other. Also, it helps making sure mozbrowser (web views) and their
-     * parent are isolated from each other. All those entities do not share the
-     * same data (cookies, IndexedDB, localStorage, etc.) so we shouldn't allow
-     * violating that principle.
-     */
-    static bool
-    AppAttributesEqual(nsIPrincipal* aFirst,
-                       nsIPrincipal* aSecond);
-
     void DeactivateDomainPolicy();
 
 private: