Bug 745254, r=kaie, a=akeybl
authorHonza Bambas <honzab.moz@firemni.cz>
Mon, 07 May 2012 13:28:19 +0200
changeset 95644 cd5706ecd36415e684548edc35de8c665d267ea7
parent 95643 22fbb77fc58576883c9db253d28d31fb65821975
child 95645 5dc031d43ff358dc567e513cf43b6ee0b67ebbbb
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskaie, akeybl
bugs745254
milestone14.0a2
Bug 745254, r=kaie, a=akeybl
browser/base/content/test/browser_alltabslistener.js
security/manager/boot/src/nsSecureBrowserUIImpl.cpp
security/manager/boot/src/nsSecureBrowserUIImpl.h
--- a/browser/base/content/test/browser_alltabslistener.js
+++ b/browser/base/content/test/browser_alltabslistener.js
@@ -126,17 +126,16 @@ function startTest1() {
 }
 
 function startTest2() {
   info("\nTest 2");
   gAllNotifications = [
     "onStateChange",
     "onLocationChange",
     "onSecurityChange",
-    "onSecurityChange",
     "onStateChange"
   ];
   gFrontNotifications = gAllNotifications;
   runTest(gForegroundBrowser, "https://example.com" + gTestPage, startTest3);
 }
 
 function startTest3() {
   info("\nTest 3");
@@ -151,17 +150,16 @@ function startTest3() {
 }
 
 function startTest4() {
   info("\nTest 4");
   gAllNotifications = [
     "onStateChange",
     "onLocationChange",
     "onSecurityChange",
-    "onSecurityChange",
     "onStateChange"
   ];
   gFrontNotifications = [];
   runTest(gBackgroundBrowser, "https://example.com" + gTestPage, startTest5);
 }
 
 function startTest5() {
   info("\nTest 5");
--- a/security/manager/boot/src/nsSecureBrowserUIImpl.cpp
+++ b/security/manager/boot/src/nsSecureBrowserUIImpl.cpp
@@ -164,16 +164,17 @@ nsSecureBrowserUIImpl::nsSecureBrowserUI
   , mNewToplevelSecurityState(STATE_IS_INSECURE)
   , mNewToplevelIsEV(false)
   , mNewToplevelSecurityStateKnown(true)
   , mIsViewSource(false)
   , mSubRequestsHighSecurity(0)
   , mSubRequestsLowSecurity(0)
   , mSubRequestsBrokenSecurity(0)
   , mSubRequestsNoSecurity(0)
+  , mRestoreSubrequests(false)
 #ifdef DEBUG
   , mOnStateLocationChangeReentranceDetection(0)
 #endif
 {
   mTransferringRequests.ops = nsnull;
   ResetStateTracking();
   
 #if defined(PR_LOGGING)
@@ -568,16 +569,21 @@ nsSecureBrowserUIImpl::EvaluateAndUpdate
            ("SecureUI:%p: remember securityInfo %p\n", this,
             info));
     nsCOMPtr<nsIAssociatedContentSecurity> associatedContentSecurityFromRequest =
         do_QueryInterface(aRequest);
     if (associatedContentSecurityFromRequest)
         mCurrentToplevelSecurityInfo = aRequest;
     else
         mCurrentToplevelSecurityInfo = info;
+
+    // The subrequest counters are now in sync with 
+    // mCurrentToplevelSecurityInfo, don't restore after top level
+    // document load finishes.
+    mRestoreSubrequests = false;
   }
 
   return UpdateSecurityState(aRequest, withNewLocation, 
                              updateStatus, updateTooltip);
 }
 
 void
 nsSecureBrowserUIImpl::UpdateSubrequestMembers(nsISupports *securityInfo)
@@ -1108,16 +1114,18 @@ nsSecureBrowserUIImpl::OnStateChange(nsI
   
         // before resetting our state, let's save information about
         // sub element loads, so we can restore it later
         prevContentSecurity->SetCountSubRequestsHighSecurity(saveSubHigh);
         prevContentSecurity->SetCountSubRequestsLowSecurity(saveSubLow);
         prevContentSecurity->SetCountSubRequestsBrokenSecurity(saveSubBroken);
         prevContentSecurity->SetCountSubRequestsNoSecurity(saveSubNo);
         prevContentSecurity->Flush();
+        PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI:%p: Saving subs in START to %p as %d,%d,%d,%d\n", 
+          this, prevContentSecurity.get(), saveSubHigh, saveSubLow, saveSubBroken, saveSubNo));      
       }
 
       bool retrieveAssociatedState = false;
 
       if (securityInfo &&
           (aProgressStateFlags & nsIWebProgressListener::STATE_RESTORING) != 0) {
         retrieveAssociatedState = true;
       } else {
@@ -1141,18 +1149,29 @@ nsSecureBrowserUIImpl::OnStateChange(nsI
           PR_LOG(gSecureDocLog, PR_LOG_DEBUG,
                  ("SecureUI:%p: OnStateChange: start, loading old sub state\n", this
                   ));
     
           newContentSecurity->GetCountSubRequestsHighSecurity(&newSubHigh);
           newContentSecurity->GetCountSubRequestsLowSecurity(&newSubLow);
           newContentSecurity->GetCountSubRequestsBrokenSecurity(&newSubBroken);
           newContentSecurity->GetCountSubRequestsNoSecurity(&newSubNo);
+          PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI:%p: Restoring subs in START from %p to %d,%d,%d,%d\n", 
+            this, newContentSecurity.get(), newSubHigh, newSubLow, newSubBroken, newSubNo));      
         }
       }
+      else
+      {
+        // If we don't get OnLocationChange for this top level load later,
+        // it didn't get rendered.  But we reset the state to unknown and
+        // mSubRequests* to zeros.  If we would have left these values after 
+        // this top level load stoped, we would override the original top level
+        // load with all zeros and break mixed content state on back and forward.
+        mRestoreSubrequests = true;
+      }
     }
 
     {
       ReentrantMonitorAutoEnter lock(mReentrantMonitor);
 
       if (allowSecurityStateChange && !inProgress)
       {
         ResetStateTracking();
@@ -1209,30 +1228,85 @@ nsSecureBrowserUIImpl::OnStateChange(nsI
     if (!temp_ToplevelEventSink && channel)
     {
       if (allowSecurityStateChange)
       {
         ObtainEventSink(channel, temp_ToplevelEventSink);
       }
     }
 
+    bool sinkChanged = false;
+    bool inProgress;
     {
       ReentrantMonitorAutoEnter lock(mReentrantMonitor);
       if (allowSecurityStateChange)
       {
+        sinkChanged = (mToplevelEventSink != temp_ToplevelEventSink);
         mToplevelEventSink = temp_ToplevelEventSink;
       }
       --mDocumentRequestsInProgress;
+      inProgress = mDocumentRequestsInProgress > 0;
     }
 
     if (allowSecurityStateChange && requestHasTransferedData) {
       // Data has been transferred for the single toplevel
       // request. Evaluate the security state.
 
-      return EvaluateAndUpdateSecurityState(aRequest, securityInfo, false);
+      // Do this only when the sink has changed.  We update and notify
+      // the state from OnLacationChange, this is actually redundant.
+      // But when the target sink changes between OnLocationChange and
+      // OnStateChange, we have to fire the notification here (again).
+
+      if (sinkChanged)
+        return EvaluateAndUpdateSecurityState(aRequest, securityInfo, false);
+    }
+
+    if (mRestoreSubrequests && !inProgress)
+    {
+      // We get here when there were no OnLocationChange between 
+      // OnStateChange(START) and OnStateChange(STOP).  Then the load has not
+      // been rendered but has been retargeted in some other way then by external
+      // app handler.  Restore mSubRequests* members to what the current security 
+      // state info holds (it was reset to all zero in OnStateChange(START) 
+      // before).
+      nsCOMPtr<nsIAssociatedContentSecurity> currentContentSecurity;
+      {
+        ReentrantMonitorAutoEnter lock(mReentrantMonitor);
+        currentContentSecurity = do_QueryInterface(mCurrentToplevelSecurityInfo);
+
+        // Drop this indication flag, the restore opration is just being
+        // done.
+        mRestoreSubrequests = false;
+
+        // We can do this since the state didn't actually change.
+        mNewToplevelSecurityStateKnown = true;
+      }
+
+      PRInt32 subHigh = 0;
+      PRInt32 subLow = 0;
+      PRInt32 subBroken = 0;
+      PRInt32 subNo = 0;
+
+      if (currentContentSecurity)
+      {
+        currentContentSecurity->GetCountSubRequestsHighSecurity(&subHigh);
+        currentContentSecurity->GetCountSubRequestsLowSecurity(&subLow);
+        currentContentSecurity->GetCountSubRequestsBrokenSecurity(&subBroken);
+        currentContentSecurity->GetCountSubRequestsNoSecurity(&subNo);
+        PR_LOG(gSecureDocLog, PR_LOG_DEBUG, ("SecureUI:%p: Restoring subs in STOP from %p to %d,%d,%d,%d\n", 
+          this, currentContentSecurity.get(), subHigh, subLow, subBroken, subNo));      
+      }
+
+      {
+        ReentrantMonitorAutoEnter lock(mReentrantMonitor);
+        mSubRequestsHighSecurity = subHigh;
+        mSubRequestsLowSecurity = subLow;
+        mSubRequestsBrokenSecurity = subBroken;
+        mSubRequestsNoSecurity = subNo;
+      }
     }
     
     return NS_OK;
   }
   
   if (aProgressStateFlags & STATE_STOP
       &&
       aProgressStateFlags & STATE_IS_REQUEST)
--- a/security/manager/boot/src/nsSecureBrowserUIImpl.h
+++ b/security/manager/boot/src/nsSecureBrowserUIImpl.h
@@ -125,16 +125,17 @@ protected:
   bool mIsViewSource;
 
   nsXPIDLString mInfoTooltip;
   PRInt32 mDocumentRequestsInProgress;
   PRInt32 mSubRequestsHighSecurity;
   PRInt32 mSubRequestsLowSecurity;
   PRInt32 mSubRequestsBrokenSecurity;
   PRInt32 mSubRequestsNoSecurity;
+  bool mRestoreSubrequests;
 #ifdef DEBUG
   /* related to mReentrantMonitor */
   PRInt32 mOnStateLocationChangeReentranceDetection;
 #endif
 
   static already_AddRefed<nsISupports> ExtractSecurityInfo(nsIRequest* aRequest);
   static nsresult MapInternalToExternalState(PRUint32* aState, lockIconState lock, bool ev);
   nsresult UpdateSecurityState(nsIRequest* aRequest, bool withNewLocation,