Bug 1283281 - Remove PB Flag from DOMStorage. r=jdm
authorJames Andreou <jandreou25@gmail.com>
Wed, 29 Jun 2016 14:01:00 +0200
changeset 308911 34acf7fe34f4acf377bc3f49b608d9bfb61e2dc9
parent 308910 8c1e5c61b39590d8bf1c2313bf027401dcbb1d68
child 308912 ac13985054939a332884655b03d9a3ee87fe59b5
push id80455
push usercbook@mozilla.com
push dateWed, 10 Aug 2016 14:37:44 +0000
treeherdermozilla-inbound@a29eab248d20 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs1283281
milestone51.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 1283281 - Remove PB Flag from DOMStorage. r=jdm
browser/components/sessionstore/SessionStorage.jsm
caps/BasePrincipal.cpp
caps/BasePrincipal.h
caps/nsIPrincipal.idl
docshell/base/nsDocShell.cpp
dom/base/nsGlobalWindow.cpp
dom/interfaces/storage/nsIDOMStorageManager.idl
dom/storage/DOMStorage.cpp
dom/storage/DOMStorage.h
dom/storage/DOMStorageCache.h
dom/storage/DOMStorageManager.cpp
dom/storage/DOMStorageManager.h
embedding/components/windowwatcher/nsWindowWatcher.cpp
--- a/browser/components/sessionstore/SessionStorage.jsm
+++ b/browser/components/sessionstore/SessionStorage.jsm
@@ -117,17 +117,17 @@ var SessionStorageInternal = {
       }
 
       let storageManager = aDocShell.QueryInterface(Ci.nsIDOMStorageManager);
       let window = aDocShell.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindow);
 
       // There is no need to pass documentURI, it's only used to fill documentURI property of
       // domstorage event, which in this case has no consumer. Prevention of events in case
       // of missing documentURI will be solved in a followup bug to bug 600307.
-      let storage = storageManager.createStorage(window, principal, "", aDocShell.usePrivateBrowsing);
+      let storage = storageManager.createStorage(window, principal, "");
 
       for (let key of Object.keys(data)) {
         try {
           storage.setItem(key, data[key]);
         } catch (e) {
           // throws e.g. for URIs that can't have sessionStorage
           console.error(e);
         }
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -561,16 +561,23 @@ BasePrincipal::GetAddonId(nsAString& aAd
 NS_IMETHODIMP
 BasePrincipal::GetUserContextId(uint32_t* aUserContextId)
 {
   *aUserContextId = UserContextId();
   return NS_OK;
 }
 
 NS_IMETHODIMP
+BasePrincipal::GetPrivateBrowsingId(uint32_t* aPrivateBrowsingId)
+{
+  *aPrivateBrowsingId = PrivateBrowsingId();
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 BasePrincipal::GetIsInIsolatedMozBrowserElement(bool* aIsInIsolatedMozBrowserElement)
 {
   *aIsInIsolatedMozBrowserElement = IsInIsolatedMozBrowserElement();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetUnknownAppId(bool* aUnknownAppId)
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@@ -259,29 +259,31 @@ public:
   NS_IMETHOD GetOriginAttributes(JSContext* aCx, JS::MutableHandle<JS::Value> aVal) final;
   NS_IMETHOD GetOriginSuffix(nsACString& aOriginSuffix) final;
   NS_IMETHOD GetAppStatus(uint16_t* aAppStatus) final;
   NS_IMETHOD GetAppId(uint32_t* aAppStatus) final;
   NS_IMETHOD GetAddonId(nsAString& aAddonId) final;
   NS_IMETHOD GetIsInIsolatedMozBrowserElement(bool* aIsInIsolatedMozBrowserElement) final;
   NS_IMETHOD GetUnknownAppId(bool* aUnknownAppId) final;
   NS_IMETHOD GetUserContextId(uint32_t* aUserContextId) final;
+  NS_IMETHOD GetPrivateBrowsingId(uint32_t* aPrivateBrowsingId) final;
 
   virtual bool IsOnCSSUnprefixingWhitelist() override { return false; }
 
   virtual bool IsCodebasePrincipal() const { return false; };
 
   static BasePrincipal* Cast(nsIPrincipal* aPrin) { return static_cast<BasePrincipal*>(aPrin); }
   static already_AddRefed<BasePrincipal>
   CreateCodebasePrincipal(nsIURI* aURI, const PrincipalOriginAttributes& aAttrs);
   static already_AddRefed<BasePrincipal> CreateCodebasePrincipal(const nsACString& aOrigin);
 
   const PrincipalOriginAttributes& OriginAttributesRef() { return mOriginAttributes; }
   uint32_t AppId() const { return mOriginAttributes.mAppId; }
   uint32_t UserContextId() const { return mOriginAttributes.mUserContextId; }
+  uint32_t PrivateBrowsingId() const { return mOriginAttributes.mPrivateBrowsingId; }
   bool IsInIsolatedMozBrowserElement() const { return mOriginAttributes.mInIsolatedMozBrowser; }
 
   enum PrincipalKind {
     eNullPrincipal,
     eCodebasePrincipal,
     eExpandedPrincipal,
     eSystemPrincipal
   };
--- a/caps/nsIPrincipal.idl
+++ b/caps/nsIPrincipal.idl
@@ -303,16 +303,23 @@ interface nsIPrincipal : nsISerializable
     /**
      * Gets the id of the user context this principal is inside.  If this
      * principal is inside the default userContext, this returns
      * nsIScriptSecurityManager::DEFAULT_USER_CONTEXT_ID.
      */
     [infallible] readonly attribute unsigned long userContextId;
 
     /**
+     * Gets the id of the private browsing state of the context containing
+     * this principal. If the principal has a private browsing value of 0, it
+     * is not in private browsing.
+     */
+    [infallible] readonly attribute unsigned long privateBrowsingId;
+
+    /**
      * Returns true iff the principal is inside an isolated mozbrowser element.
      * <iframe mozbrowser mozapp> and <xul:browser> are not considered to be
      * mozbrowser elements.  <iframe mozbrowser noisolation> does not count as
      * isolated since isolation is disabled.  Isolation can only be disabled if
      * the containing document is chrome.
      */
     [infallible] readonly attribute boolean isInIsolatedMozBrowserElement;
 
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -2979,21 +2979,21 @@ nsDocShell::GetSessionStorageForPrincipa
     return NS_ERROR_UNEXPECTED;
   }
 
   nsCOMPtr<nsPIDOMWindowOuter> domWin = GetWindow();
 
   AssertOriginAttributesMatchPrivateBrowsing();
   if (aCreate) {
     return manager->CreateStorage(domWin->GetCurrentInnerWindow(), aPrincipal,
-                                  aDocumentURI, UsePrivateBrowsing(), aStorage);
+                                  aDocumentURI, aStorage);
   }
 
   return manager->GetStorage(domWin->GetCurrentInnerWindow(), aPrincipal,
-                             UsePrivateBrowsing(), aStorage);
+                             aStorage);
 }
 
 nsresult
 nsDocShell::AddSessionStorage(nsIPrincipal* aPrincipal, nsIDOMStorage* aStorage)
 {
   RefPtr<DOMStorage> storage = static_cast<DOMStorage*>(aStorage);
   if (!storage) {
     return NS_ERROR_UNEXPECTED;
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -10536,17 +10536,16 @@ nsGlobalWindow::GetSessionStorage(ErrorR
     nsCOMPtr<nsIDOMStorageManager> storageManager = do_QueryInterface(docShell, &rv);
     if (NS_FAILED(rv)) {
       aError.Throw(rv);
       return nullptr;
     }
 
     nsCOMPtr<nsIDOMStorage> storage;
     aError = storageManager->CreateStorage(AsInner(), principal, documentURI,
-                                           IsPrivateBrowsing(),
                                            getter_AddRefs(storage));
     if (aError.Failed()) {
       return nullptr;
     }
 
     mSessionStorage = static_cast<DOMStorage*>(storage.get());
     MOZ_ASSERT(mSessionStorage);
 
@@ -10597,17 +10596,16 @@ nsGlobalWindow::GetLocalStorage(ErrorRes
 
     nsString documentURI;
     if (mDoc) {
       mDoc->GetDocumentURI(documentURI);
     }
 
     nsCOMPtr<nsIDOMStorage> storage;
     aError = storageManager->CreateStorage(AsInner(), principal, documentURI,
-                                           IsPrivateBrowsing(),
                                            getter_AddRefs(storage));
     if (aError.Failed()) {
       return nullptr;
     }
 
     mLocalStorage = static_cast<DOMStorage*>(storage.get());
     MOZ_ASSERT(mLocalStorage);
   }
@@ -11450,17 +11448,24 @@ nsGlobalWindow::Observe(nsISupports* aSu
     bool fireMozStorageChanged = false;
     nsAutoString eventType;
     eventType.AssignLiteral("storage");
     principal = GetPrincipal();
     if (!principal) {
       return NS_OK;
     }
 
-    if (changingStorage->IsPrivate() != IsPrivateBrowsing()) {
+    uint32_t privateBrowsingId = 0;
+    nsIPrincipal *storagePrincipal = changingStorage->GetPrincipal();
+    rv = storagePrincipal->GetPrivateBrowsingId(&privateBrowsingId);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+
+    if ((privateBrowsingId > 0) != IsPrivateBrowsing()) {
       return NS_OK;
     }
 
     switch (changingStorage->GetType())
     {
     case DOMStorage::SessionStorage:
     {
       bool check = false;
--- a/dom/interfaces/storage/nsIDOMStorageManager.idl
+++ b/dom/interfaces/storage/nsIDOMStorageManager.idl
@@ -34,34 +34,32 @@ interface nsIDOMStorageManager : nsISupp
    *    Principal to bound storage to.
    * @param aDocumentURI
    *    URL of the demanding document, used for DOM storage event only.
    * @param aPrivate
    *    Whether the demanding document is running in Private Browsing mode or not.
    */
   nsIDOMStorage createStorage(in mozIDOMWindow aWindow,
                               in nsIPrincipal aPrincipal,
-                              in DOMString aDocumentURI,
-                              [optional] in bool aPrivate);
+                              in DOMString aDocumentURI);
   /**
    * Returns instance of DOM storage object for given principal.
    * If there is no storage managed for the scope, then null is returned and
    * no object is created.  Otherwise, an object (new) for the existing storage
    * scope is returned.
    *
    * @param aWindow
    *    The parent window.
    * @param aPrincipal
    *    Principal to bound storage to.
    * @param aPrivate
    *    Whether the demanding document is running in Private Browsing mode or not.
    */
   nsIDOMStorage getStorage(in mozIDOMWindow aWindow,
-                           in nsIPrincipal aPrincipal,
-                           [optional] in bool aPrivate);
+                           in nsIPrincipal aPrincipal);
 
   /**
    * Clones given storage into this storage manager.
    *
    * @param aStorageToCloneFrom
    *    The storage to copy all items from into this manager.  Manager will then
    *    return a new and independent object that contains snapshot of data from
    *    the moment this method was called.  Modification to this new object will
@@ -95,11 +93,10 @@ interface nsIDOMStorageManager : nsISupp
    *
    * Currently just forwards to the createStorage method of this
    * interface.
    *
    * Extension developers are strongly encouraged to use getStorage
    * or createStorage method instead.
    */
   nsIDOMStorage getLocalStorageForPrincipal(in nsIPrincipal aPrincipal,
-                                            in DOMString aDocumentURI,
-                                            [optional] in bool aPrivate);
+                                            in DOMString aDocumentURI);
 };
--- a/dom/storage/DOMStorage.cpp
+++ b/dom/storage/DOMStorage.cpp
@@ -39,24 +39,22 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
   NS_INTERFACE_MAP_ENTRY(nsIDOMStorage)
   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
 NS_INTERFACE_MAP_END
 
 DOMStorage::DOMStorage(nsPIDOMWindowInner* aWindow,
                        DOMStorageManager* aManager,
                        DOMStorageCache* aCache,
                        const nsAString& aDocumentURI,
-                       nsIPrincipal* aPrincipal,
-                       bool aIsPrivate)
+                       nsIPrincipal* aPrincipal)
 : mWindow(aWindow)
 , mManager(aManager)
 , mCache(aCache)
 , mDocumentURI(aDocumentURI)
 , mPrincipal(aPrincipal)
-, mIsPrivate(aIsPrivate)
 , mIsSessionOnly(false)
 {
   mCache->Preload();
 }
 
 DOMStorage::~DOMStorage()
 {
   mCache->KeepAlive();
@@ -226,18 +224,16 @@ DOMStorage::BroadcastChangeNotification(
 static const char kPermissionType[] = "cookie";
 static const char kStorageEnabled[] = "dom.storage.enabled";
 
 // static, public
 bool
 DOMStorage::CanUseStorage(nsPIDOMWindowInner* aWindow, DOMStorage* aStorage)
 {
   // This method is responsible for correct setting of mIsSessionOnly.
-  // It doesn't work with mIsPrivate flag at all, since it is checked
-  // regardless mIsSessionOnly flag in DOMStorageCache code.
 
   if (!mozilla::Preferences::GetBool(kStorageEnabled)) {
     return false;
   }
 
   nsContentUtils::StorageAccess access = nsContentUtils::StorageAccess::eDeny;
   if (aWindow) {
     access = nsContentUtils::StorageAllowedForWindow(aWindow);
@@ -278,16 +274,27 @@ PrincipalsEqual(nsIPrincipal* aObjectPri
 
 bool
 DOMStorage::PrincipalEquals(nsIPrincipal* aPrincipal)
 {
   return PrincipalsEqual(mPrincipal, aPrincipal);
 }
 
 bool
+DOMStorage::IsPrivate() const
+{
+  uint32_t privateBrowsingId = 0;
+  nsresult rv = mPrincipal->GetPrivateBrowsingId(&privateBrowsingId);
+  if (NS_FAILED(rv)) {
+    return false;
+  }
+  return privateBrowsingId > 0;
+}
+
+bool
 DOMStorage::CanAccess(nsIPrincipal* aPrincipal)
 {
   return !aPrincipal || aPrincipal->Subsumes(mPrincipal);
 }
 
 void
 DOMStorage::GetSupportedNames(nsTArray<nsString>& aKeys)
 {
--- a/dom/storage/DOMStorage.h
+++ b/dom/storage/DOMStorage.h
@@ -49,27 +49,22 @@ public:
   DOMStorageCache const* GetCache() const
   {
     return mCache;
   }
 
   nsIPrincipal* GetPrincipal();
   bool PrincipalEquals(nsIPrincipal* aPrincipal);
   bool CanAccess(nsIPrincipal* aPrincipal);
-  bool IsPrivate()
-  {
-    return mIsPrivate;
-  }
 
   DOMStorage(nsPIDOMWindowInner* aWindow,
              DOMStorageManager* aManager,
              DOMStorageCache* aCache,
              const nsAString& aDocumentURI,
-             nsIPrincipal* aPrincipal,
-             bool aIsPrivate);
+             nsIPrincipal* aPrincipal);
 
   // WebIDL
   JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   nsPIDOMWindowInner* GetParentObject() const
   {
     return mWindow;
   }
@@ -114,17 +109,17 @@ public:
   // on a storage is about to happen and ensures that the storage's
   // session-only flag is properly set according the current settings.
   // It is an optimization since the privileges check and session only
   // state determination are complex and share the code (comes hand in
   // hand together).
   static bool CanUseStorage(nsPIDOMWindowInner* aWindow,
                             DOMStorage* aStorage = nullptr);
 
-  bool IsPrivate() const { return mIsPrivate; }
+  bool IsPrivate() const;
   bool IsSessionOnly() const { return mIsSessionOnly; }
 
   bool IsForkOf(const DOMStorage* aOther) const
   {
     MOZ_ASSERT(aOther);
     return mCache == aOther->mCache;
   }
 
@@ -138,19 +133,16 @@ private:
   RefPtr<DOMStorageManager> mManager;
   RefPtr<DOMStorageCache> mCache;
   nsString mDocumentURI;
 
   // Principal this DOMStorage (i.e. localStorage or sessionStorage) has
   // been created for
   nsCOMPtr<nsIPrincipal> mPrincipal;
 
-  // Whether this storage is running in private-browsing window.
-  bool mIsPrivate : 1;
-
   // Whether storage is set to persist data only per session, may change
   // dynamically and is set by CanUseStorage function that is called
   // before any operation on the storage.
   bool mIsSessionOnly : 1;
 
   void BroadcastChangeNotification(const nsSubstring& aKey,
                                    const nsSubstring& aOldValue,
                                    const nsSubstring& aNewValue);
--- a/dom/storage/DOMStorageCache.h
+++ b/dom/storage/DOMStorageCache.h
@@ -96,17 +96,17 @@ public:
   // Starts async preload of this cache if it persistent and not loaded.
   void Preload();
 
   // Keeps the cache alive (i.e. present in the manager's hash table) for a time.
   void KeepAlive();
 
   // The set of methods that are invoked by DOM storage web API.
   // We are passing the DOMStorage object just to let the cache
-  // read properties like mPrivate and mSessionOnly.
+  // read properties like mPrincipal and mSessionOnly.
   // Get* methods return error when load from the database has failed.
   nsresult GetLength(const DOMStorage* aStorage, uint32_t* aRetval);
   nsresult GetKey(const DOMStorage* aStorage, uint32_t index, nsAString& aRetval);
   nsresult GetItem(const DOMStorage* aStorage, const nsAString& aKey, nsAString& aRetval);
   nsresult SetItem(const DOMStorage* aStorage, const nsAString& aKey, const nsString& aValue, nsString& aOld);
   nsresult RemoveItem(const DOMStorage* aStorage, const nsAString& aKey, nsString& aOld);
   nsresult Clear(const DOMStorage* aStorage);
 
--- a/dom/storage/DOMStorageManager.cpp
+++ b/dom/storage/DOMStorageManager.cpp
@@ -305,17 +305,16 @@ DOMStorageManager::DropCache(DOMStorageC
   table->RemoveEntry(aCache->OriginNoSuffix());
 }
 
 nsresult
 DOMStorageManager::GetStorageInternal(bool aCreate,
                                       mozIDOMWindow* aWindow,
                                       nsIPrincipal* aPrincipal,
                                       const nsAString& aDocumentURI,
-                                      bool aPrivate,
                                       nsIDOMStorage** aRetval)
 {
   nsresult rv;
 
   nsAutoCString originAttrSuffix;
   BasePrincipal::Cast(aPrincipal)->OriginAttributesRef().CreateSuffix(originAttrSuffix);
 
   nsAutoCString originKey;
@@ -356,49 +355,44 @@ DOMStorageManager::GetStorageInternal(bo
       return NS_ERROR_DOM_SECURITY_ERR;
     }
   }
 
   if (aRetval) {
     nsCOMPtr<nsPIDOMWindowInner> inner = nsPIDOMWindowInner::From(aWindow);
 
     nsCOMPtr<nsIDOMStorage> storage = new DOMStorage(
-      inner, this, cache, aDocumentURI, aPrincipal, aPrivate);
+      inner, this, cache, aDocumentURI, aPrincipal);
     storage.forget(aRetval);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DOMStorageManager::PrecacheStorage(nsIPrincipal* aPrincipal)
 {
-  return GetStorageInternal(true, nullptr, aPrincipal, EmptyString(), false,
-                            nullptr);
+  return GetStorageInternal(true, nullptr, aPrincipal, EmptyString(), nullptr);
 }
 
 NS_IMETHODIMP
 DOMStorageManager::CreateStorage(mozIDOMWindow* aWindow,
                                  nsIPrincipal* aPrincipal,
                                  const nsAString& aDocumentURI,
-                                 bool aPrivate,
                                  nsIDOMStorage** aRetval)
 {
-  return GetStorageInternal(true, aWindow, aPrincipal, aDocumentURI, aPrivate,
-                            aRetval);
+  return GetStorageInternal(true, aWindow, aPrincipal, aDocumentURI, aRetval);
 }
 
 NS_IMETHODIMP
 DOMStorageManager::GetStorage(mozIDOMWindow* aWindow,
                               nsIPrincipal* aPrincipal,
-                              bool aPrivate,
                               nsIDOMStorage** aRetval)
 {
-  return GetStorageInternal(false, aWindow, aPrincipal, EmptyString(), aPrivate,
-                            aRetval);
+  return GetStorageInternal(false, aWindow, aPrincipal, EmptyString(), aRetval);
 }
 
 NS_IMETHODIMP
 DOMStorageManager::CloneStorage(nsIDOMStorage* aStorage)
 {
   if (mType != SessionStorage) {
     // Cloning is supported only for sessionStorage
     return NS_ERROR_NOT_IMPLEMENTED;
@@ -468,24 +462,23 @@ DOMStorageManager::CheckStorage(nsIPrinc
   return NS_OK;
 }
 
 // Obsolete nsIDOMStorageManager methods
 
 NS_IMETHODIMP
 DOMStorageManager::GetLocalStorageForPrincipal(nsIPrincipal* aPrincipal,
                                                const nsAString& aDocumentURI,
-                                               bool aPrivate,
                                                nsIDOMStorage** aRetval)
 {
   if (mType != LocalStorage) {
     return NS_ERROR_UNEXPECTED;
   }
 
-  return CreateStorage(nullptr, aPrincipal, aDocumentURI, aPrivate, aRetval);
+  return CreateStorage(nullptr, aPrincipal, aDocumentURI, aRetval);
 }
 
 void
 DOMStorageManager::ClearCaches(uint32_t aUnloadFlags,
                                const OriginAttributesPattern& aPattern,
                                const nsACString& aOriginScope)
 {
   for (auto iter1 = mCaches.Iter(); !iter1.Done(); iter1.Next()) {
--- a/dom/storage/DOMStorageManager.h
+++ b/dom/storage/DOMStorageManager.h
@@ -88,17 +88,16 @@ private:
                                              const nsACString& aOriginNoSuffix,
                                              nsIPrincipal* aPrincipal);
 
   // Helper for creation of DOM storage objects
   nsresult GetStorageInternal(bool aCreate,
                               mozIDOMWindow* aWindow,
                               nsIPrincipal* aPrincipal,
                               const nsAString& aDocumentURI,
-                              bool aPrivate,
                               nsIDOMStorage** aRetval);
 
   // Suffix->origin->cache map
   typedef nsTHashtable<DOMStorageCacheHashKey> CacheOriginHashtable;
   nsClassHashtable<nsCStringHashKey, CacheOriginHashtable> mCaches;
 
   const DOMStorage::StorageType mType;
 
--- a/embedding/components/windowwatcher/nsWindowWatcher.cpp
+++ b/embedding/components/windowwatcher/nsWindowWatcher.cpp
@@ -1239,17 +1239,16 @@ nsWindowWatcher::OpenWindowInternal(mozI
     nsCOMPtr<nsIDOMStorageManager> newStorageManager =
       do_QueryInterface(newDocShell);
 
     if (parentStorageManager && newStorageManager) {
       nsCOMPtr<nsIDOMStorage> storage;
       nsCOMPtr<nsPIDOMWindowInner> pInnerWin = parentWindow->GetCurrentInnerWindow();
 
       parentStorageManager->GetStorage(pInnerWin, subjectPrincipal,
-                                       isPrivateBrowsingWindow,
                                        getter_AddRefs(storage));
       if (storage) {
         newStorageManager->CloneStorage(storage);
       }
     }
   }
 
   if (isNewToplevelWindow) {