Bug 1317173 - Better propagation of private browsing flag when a new window is opened. r=smaug, a=gchang
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 15 Dec 2016 07:54:00 -0500
changeset 353020 123b0bf81002bf07a8780a9080bec16741499b7e
parent 353019 21b41507b2e5f29fbd72d3600f0bfef4bf5f0798
child 353021 dd0f0ac09dbaab20b7dd217c6dff3c652fd8421a
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, gchang
bugs1317173
milestone52.0a2
Bug 1317173 - Better propagation of private browsing flag when a new window is opened. r=smaug, a=gchang
embedding/components/windowwatcher/nsWindowWatcher.cpp
--- a/embedding/components/windowwatcher/nsWindowWatcher.cpp
+++ b/embedding/components/windowwatcher/nsWindowWatcher.cpp
@@ -1112,28 +1112,48 @@ nsWindowWatcher::OpenWindowInternal(mozI
   //
   // Note: The check for the current JSContext isn't necessarily sensical.
   // It's just designed to preserve old semantics during a mass-conversion
   // patch.
   nsCOMPtr<nsIPrincipal> subjectPrincipal =
     nsContentUtils::GetCurrentJSContext() ? nsContentUtils::SubjectPrincipal() :
                                             nullptr;
 
+  bool isPrivateBrowsingWindow = false;
+
   if (windowIsNew) {
     auto* docShell = static_cast<nsDocShell*>(newDocShell.get());
 
     // If this is not a chrome docShell, we apply originAttributes from the
     // subjectPrincipal unless if it's an expanded or system principal.
     if (subjectPrincipal &&
         !nsContentUtils::IsSystemOrExpandedPrincipal(subjectPrincipal) &&
         docShell->ItemType() != nsIDocShellTreeItem::typeChrome) {
       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();
+      }
+    }
 
-      docShell->SetOriginAttributes(attrs);
+    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
     // this no matter whether we were opened from JS; if there is nothing on
     // the JS stack, just use the principal of our parent window.  In those
     // cases we do _not_ set the parent window principal as the owner of the
     // load--since we really don't know who the owner is, just leave it null.
     nsCOMPtr<nsPIDOMWindowOuter> newWindow = do_QueryInterface(*aResult);
@@ -1153,36 +1173,16 @@ nsWindowWatcher::OpenWindowInternal(mozI
                                                // counter even if the above
                                                // assert fails.
           globalWin->SetIsPopupSpamWindow(true);
         }
       }
     }
   }
 
-  // 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 =
     !!(chromeFlags & nsIWebBrowserChrome::CHROME_REMOTE_WINDOW);
 
   if (isNewToplevelWindow) {
     nsCOMPtr<nsIDocShellTreeItem> childRoot;
     newDocShellItem->GetRootTreeItem(getter_AddRefs(childRoot));