Bug 1317173 - Better propagation of private browsing flag when a new window is opened, r=smaug

embedding/components/windowwatcher/nsWindowWatcher.cpp

@@ -1093,7 +1093,8 @@ nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy* aParent,
     nsContentUtils::GetCurrentJSContext() ? nsContentUtils::SubjectPrincipal() :
                                             nullptr;
 
-  bool shouldCheckPrivateBrowsingId = false;
+  bool isPrivateBrowsingWindow = false;
+
   if (windowIsNew) {
     auto* docShell = static_cast<nsDocShell*>(newDocShell.get());
 
@@ -1102,11 +1103,28 @@ nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy* aParent,
     if (subjectPrincipal &&
         !nsContentUtils::IsSystemOrExpandedPrincipal(subjectPrincipal) &&
         docShell->ItemType() != nsIDocShellTreeItem::typeChrome) {
-      shouldCheckPrivateBrowsingId = true;
       DocShellOriginAttributes attrs;
       attrs.InheritFromDocToChildDocShell(BasePrincipal::Cast(subjectPrincipal)->OriginAttributesRef());
-
+      isPrivateBrowsingWindow = !!attrs.mPrivateBrowsingId;
       docShell->SetOriginAttributes(attrs);
+    } else {
+      nsCOMPtr<nsIDocShellTreeItem> parentItem;
+      GetWindowTreeItem(aParent, getter_AddRefs(parentItem));
+      nsCOMPtr<nsILoadContext> parentContext = do_QueryInterface(parentItem);
+      if (parentContext) {
+        isPrivateBrowsingWindow = parentContext->UsePrivateBrowsing();
+      }
+    }
+
+    bool autoPrivateBrowsing =
+      Preferences::GetBool("browser.privatebrowsing.autostart");
+
+    if (!autoPrivateBrowsing &&
+        (chromeFlags & nsIWebBrowserChrome::CHROME_NON_PRIVATE_WINDOW)) {
+      isPrivateBrowsingWindow = false;
+    } else if (autoPrivateBrowsing ||
+               (chromeFlags & nsIWebBrowserChrome::CHROME_PRIVATE_WINDOW)) {
+      isPrivateBrowsingWindow = true;
     }
 
     // Now set the opener principal on the new window.  Note that we need to do
@@ -1136,26 +1154,6 @@ nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy* aParent,
     }
   }
 
-  // If all windows should be private, make sure the new window is also
-  // private.  Otherwise, see if the caller has explicitly requested a
-  // private or non-private window.
-  bool isPrivateBrowsingWindow =
-    Preferences::GetBool("browser.privatebrowsing.autostart") ||
-    (!!(chromeFlags & nsIWebBrowserChrome::CHROME_PRIVATE_WINDOW) &&
-     !(chromeFlags & nsIWebBrowserChrome::CHROME_NON_PRIVATE_WINDOW));
-
-  // Otherwise, propagate the privacy status of the parent window, if
-  // available, to the child.
-  if (!isPrivateBrowsingWindow &&
-      !(chromeFlags & nsIWebBrowserChrome::CHROME_NON_PRIVATE_WINDOW)) {
-    nsCOMPtr<nsIDocShellTreeItem> parentItem;
-    GetWindowTreeItem(aParent, getter_AddRefs(parentItem));
-    nsCOMPtr<nsILoadContext> parentContext = do_QueryInterface(parentItem);
-    if (parentContext) {
-      isPrivateBrowsingWindow = parentContext->UsePrivateBrowsing();
-    }
-  }
-
   // We rely on CalculateChromeFlags to decide whether remote (out-of-process)
   // tabs should be used.
   bool isRemoteWindow =
@@ -1235,11 +1233,6 @@ nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy* aParent,
       do_QueryInterface(newDocShell);
 
     if (parentStorageManager && newStorageManager) {
-      if (shouldCheckPrivateBrowsingId) {
-        MOZ_DIAGNOSTIC_ASSERT(
-          (subjectPrincipal->GetPrivateBrowsingId() > 0) == isPrivateBrowsingWindow);
-      }
-
       nsCOMPtr<nsIDOMStorage> storage;
       nsCOMPtr<nsPIDOMWindowInner> pInnerWin = parentWindow->GetCurrentInnerWindow();
       parentStorageManager->GetStorage(pInnerWin, subjectPrincipal,