Bug 1292450 - Check mPrivateBrowsingId is correct in LoadInfo and LoadContext. r=smaug, ehsan

Make sure the mPrivateBrowsingId of Origin Attributes is consistent
between LoadInfo and LoadContext.

For chrome docshell, its mPrivateBrowsingId remains 0 even if its
UserPrivateBrowsing() is true (bug 1278664). So we sync the
mPrivateBrowsingId field in LoadInfo in the same way.

docshell/base/LoadContext.cpp

@@ -45,7 +45,6 @@ LoadContext::LoadContext(nsIPrincipal* aPrincipal,
   : mTopFrameElement(nullptr)
   , mNestedFrameId(0)
   , mIsContent(true)
-  , mUsePrivateBrowsing(false)
   , mUseRemoteTabs(false)
 #ifdef DEBUG
   , mIsNotNull(true)
@@ -53,7 +52,7 @@ LoadContext::LoadContext(nsIPrincipal* aPrincipal,
 {
   PrincipalOriginAttributes poa = BasePrincipal::Cast(aPrincipal)->OriginAttributesRef();
   mOriginAttributes.InheritFromDocToChildDocShell(poa);
-  mOriginAttributes.SyncAttributesWithPrivateBrowsing(mUsePrivateBrowsing);
+  mUsePrivateBrowsing = (poa.mPrivateBrowsingId != 0);
   if (!aOptionalBase) {
     return;
   }

netwerk/base/LoadInfo.cpp

@@ -188,6 +188,21 @@ LoadInfo::LoadInfo(nsIPrincipal* aLoadingPrincipal,
   }
 
   InheritOriginAttributes(mLoadingPrincipal, mOriginAttributes);
+
+  // For chrome docshell, the mPrivateBrowsingId remains 0 even its
+  // UsePrivateBrowsing() is true, so we only update the mPrivateBrowsingId in
+  // origin attributes if the type of the docshell is content.
+  if (aLoadingContext) {
+    nsCOMPtr<nsIDocShell> docShell = aLoadingContext->OwnerDoc()->GetDocShell();
+    if (docShell) {
+      if (docShell->ItemType() == nsIDocShellTreeItem::typeContent) {
+        mOriginAttributes.SyncAttributesWithPrivateBrowsing(GetUsePrivateBrowsing());
+      } else if (docShell->ItemType() == nsIDocShellTreeItem::typeChrome) {
+        MOZ_ASSERT(mOriginAttributes.mPrivateBrowsingId == 0,
+                   "chrome docshell shouldn't have mPrivateBrowsingId set.");
+      }
+    }
+  }
 }
 
 /* Constructor takes an outer window, but no loadingNode or loadingPrincipal.
@@ -240,6 +255,15 @@ LoadInfo::LoadInfo(nsPIDOMWindowOuter* aOuterWindow,
   MOZ_ASSERT(docShell);
   const DocShellOriginAttributes attrs =
     nsDocShell::Cast(docShell)->GetOriginAttributes();
+
+  if (docShell->ItemType() == nsIDocShellTreeItem::typeContent) {
+    MOZ_ASSERT(GetUsePrivateBrowsing() == (attrs.mPrivateBrowsingId != 0),
+               "docshell and mSecurityFlags have different value for PrivateBrowsing().");
+  } else if (docShell->ItemType() == nsIDocShellTreeItem::typeChrome) {
+    MOZ_ASSERT(attrs.mPrivateBrowsingId == 0,
+               "chrome docshell shouldn't have mPrivateBrowsingId set.");
+  }
+
   mOriginAttributes.InheritFromDocShellToNecko(attrs);
 }
 

netwerk/base/nsILoadInfo.idl

@@ -304,6 +304,8 @@ interface nsILoadInfo : nsISupports
 
   /**
    * If usePrivateBrowsing is true, private browsing will be used.
+   * This value equals to originAttributes.privateBrowsingId in *content*
+   * side.
    */
   [infallible] readonly attribute boolean usePrivateBrowsing;
 
@@ -417,6 +419,11 @@ interface nsILoadInfo : nsISupports
   /**
    * Customized NeckoOriginAttributes within LoadInfo to allow overwriting of the
    * default originAttributes from the loadingPrincipal.
+   *
+   * In chrome side, originAttributes.privateBrowsingId will always be 0 even if
+   * the usePrivateBrowsing is true, because chrome docshell won't set
+   * privateBrowsingId on origin attributes (See bug 1278664). This is to make
+   * sure nsILoadInfo and nsILoadContext have the same origin attributes.
    */
   [implicit_jscontext, binaryname(ScriptableOriginAttributes)]
   attribute jsval originAttributes;

netwerk/base/nsNetUtil.cpp

@@ -2466,12 +2466,14 @@ NS_CompareLoadInfoAndLoadContext(nsIChannel *aChannel)
     return NS_ERROR_UNEXPECTED;
   }
 
-  LOG(("NS_CompareLoadInfoAndLoadContext - loadInfo: %d, %d, %d, %d; "
-       "loadContext: %d %d, %d, %d. [channel=%p]",
+  LOG(("NS_CompareLoadInfoAndLoadContext - loadInfo: %d, %d, %d, %d, %d; "
+       "loadContext: %d %d, %d, %d, %d. [channel=%p]",
        originAttrsLoadInfo.mAppId, originAttrsLoadInfo.mInIsolatedMozBrowser,
-       originAttrsLoadInfo.mUserContextId, loadInfoUsePB,
+       originAttrsLoadInfo.mUserContextId, originAttrsLoadInfo.mPrivateBrowsingId,
+       loadInfoUsePB,
        loadContextAppId, loadContextIsInBE,
-       originAttrsLoadContext.mUserContextId, loadContextUsePB,
+       originAttrsLoadContext.mUserContextId, originAttrsLoadContext.mPrivateBrowsingId,
+       loadContextUsePB,
        aChannel));
 
   MOZ_ASSERT(originAttrsLoadInfo.mAppId == loadContextAppId,
@@ -2488,6 +2490,11 @@ NS_CompareLoadInfoAndLoadContext(nsIChannel *aChannel)
              "The value of mUserContextId in the loadContext and in the "
              "loadInfo are not the same!");
 
+  MOZ_ASSERT(originAttrsLoadInfo.mPrivateBrowsingId ==
+             originAttrsLoadContext.mPrivateBrowsingId,
+             "The value of mPrivateBrowsingId in the loadContext and in the "
+             "loadInfo are not the same!");
+
   MOZ_ASSERT(loadInfoUsePB == loadContextUsePB,
              "The value of usePrivateBrowsing in the loadContext and in the loadInfo "
              "are not the same!");

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -2633,6 +2633,19 @@ HttpBaseChannel::ShouldIntercept(nsIURI* aURI)
   return shouldIntercept;
 }
 
+void HttpBaseChannel::CheckPrivateBrowsing()
+{
+  nsCOMPtr<nsILoadContext> loadContext;
+  NS_QueryNotificationCallbacks(this, loadContext);
+  // For addons it's possible that mLoadInfo is null.
+  if (mLoadInfo && loadContext) {
+      DocShellOriginAttributes docShellAttrs;
+      loadContext->GetOriginAttributes(docShellAttrs);
+      MOZ_ASSERT(mLoadInfo->GetOriginAttributes().mPrivateBrowsingId == docShellAttrs.mPrivateBrowsingId,
+                 "PrivateBrowsingId values are not the same between LoadInfo and LoadContext.");
+  }
+}
+
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsITraceableChannel
 //-----------------------------------------------------------------------------

netwerk/protocol/http/HttpBaseChannel.h

@@ -364,6 +364,9 @@ protected:
   // for a possible synthesized response instead.
   bool ShouldIntercept(nsIURI* aURI = nullptr);
 
+  // Check if mPrivateBrowsingId matches between LoadInfo and LoadContext.
+  void CheckPrivateBrowsing();
+
   friend class PrivateBrowsingChannel<HttpBaseChannel>;
   friend class InterceptFailedOnStop;
 

netwerk/protocol/http/HttpChannelChild.cpp

@@ -1759,6 +1759,13 @@ HttpChannelChild::AsyncOpen(nsIStreamListener *listener, nsISupports *aContext)
 
   LOG(("HttpChannelChild::AsyncOpen [this=%p uri=%s]\n", this, mSpec.get()));
 
+  MOZ_ASSERT(mLoadInfo->GetUsePrivateBrowsing() == (mLoadInfo->GetOriginAttributes().mPrivateBrowsingId != 0),
+             "PrivateBrowsing mismatch on LoadInfo.");
+
+#ifdef DEBUG
+  CheckPrivateBrowsing();
+#endif
+
   if (mCanceled)
     return mStatus;
 

netwerk/protocol/http/nsHttpChannel.cpp

@@ -5460,6 +5460,10 @@ nsHttpChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *context)
 
     NS_CompareLoadInfoAndLoadContext(this);
 
+#ifdef DEBUG
+    CheckPrivateBrowsing();
+#endif
+
     NS_ENSURE_ARG_POINTER(listener);
     NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
     NS_ENSURE_TRUE(!mWasOpened, NS_ERROR_ALREADY_OPENED);

netwerk/test/unit/test_cacheflags.js

@@ -19,6 +19,7 @@ var test404Path = "/test404" + suffix;
 // We attach this to channel when we want to test Private Browsing mode
 function LoadContext(usePrivateBrowsing) {
   this.usePrivateBrowsing = usePrivateBrowsing;
+  this.originAttributes.privateBrowsingId = usePrivateBrowsing ? 1 : 0;
 }
 
 LoadContext.prototype = {
@@ -54,6 +55,7 @@ function make_channel(url, flags, usePrivateBrowsing) {
   if (usePrivateBrowsing) {
     req.notificationCallbacks = PrivateBrowsingLoadContext;
   }
+  req.loadInfo.originAttributes = {privateBrowsingId: usePrivateBrowsing ? 1 : 0};
   return req;
 }