Bug 451420, Avoid calling OnSecurityChange unnecessarily
authorKai Engert <kaie@kuix.de>
Mon, 01 Sep 2008 22:55:50 +0200
changeset 18559 98cc97026fefea4277d051cd7c4979c18dcd75b3
parent 18558 f95e6897b536331520142eec098f10a18cb0aebf
child 18560 9599ebdf16b02762d8da588130e9cdb8d832f651
push id1642
push userkaie@kuix.de
push dateMon, 01 Sep 2008 20:56:01 +0000
treeherdermozilla-central@98cc97026fef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs451420
milestone1.9.1b1pre
Bug 451420, Avoid calling OnSecurityChange unnecessarily r=rrelyea
security/manager/boot/src/nsSecureBrowserUIImpl.cpp
security/manager/boot/src/nsSecureBrowserUIImpl.h
--- a/security/manager/boot/src/nsSecureBrowserUIImpl.cpp
+++ b/security/manager/boot/src/nsSecureBrowserUIImpl.cpp
@@ -497,17 +497,18 @@ void nsSecureBrowserUIImpl::ResetStateTr
     PL_DHashTableFinish(&mTransferringRequests);
     mTransferringRequests.ops = nsnull;
   }
   PL_DHashTableInit(&mTransferringRequests, &gMapOps, nsnull,
                     sizeof(RequestHashEntry), 16);
 }
 
 nsresult
-nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest* aRequest, nsISupports *info)
+nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest* aRequest, nsISupports *info,
+                                                      PRBool withNewLocation)
 {
   /* I explicitly ignore the camelCase variable naming style here,
      I want to make it clear these are temp variables that relate to the 
      member variables with the same suffix.*/
 
   PRUint32 temp_NewToplevelSecurityState = nsIWebProgressListener::STATE_IS_INSECURE;
   PRBool temp_NewToplevelIsEV = PR_FALSE;
 
@@ -561,17 +562,18 @@ nsSecureBrowserUIImpl::EvaluateAndUpdate
       mInfoTooltip = temp_InfoTooltip;
     }
     PR_LOG(gSecureDocLog, PR_LOG_DEBUG,
            ("SecureUI:%p: remember securityInfo %p\n", this,
             info));
     mCurrentToplevelSecurityInfo = info;
   }
 
-  return UpdateSecurityState(aRequest);
+  return UpdateSecurityState(aRequest, withNewLocation, 
+                             updateStatus, updateTooltip);
 }
 
 void
 nsSecureBrowserUIImpl::UpdateSubrequestMembers(nsISupports *securityInfo)
 {
   // For wyciwyg channels in subdocuments we only update our
   // subrequest state members.
   PRUint32 reqState = GetSecurityStateFromSecurityInfo(securityInfo);
@@ -1188,17 +1190,17 @@ nsSecureBrowserUIImpl::OnStateChange(nsI
       }
       --mDocumentRequestsInProgress;
     }
 
     if (allowSecurityStateChange && requestHasTransferedData) {
       // Data has been transferred for the single toplevel
       // request. Evaluate the security state.
 
-      return EvaluateAndUpdateSecurityState(aRequest, securityInfo);
+      return EvaluateAndUpdateSecurityState(aRequest, securityInfo, PR_FALSE);
     }
     
     return NS_OK;
   }
   
   if (aProgressStateFlags & STATE_STOP
       &&
       aProgressStateFlags & STATE_IS_REQUEST)
@@ -1226,17 +1228,17 @@ nsSecureBrowserUIImpl::OnStateChange(nsI
 
       PRBool temp_NewToplevelSecurityStateKnown;
       {
         nsAutoMonitor lock(mMonitor);
         temp_NewToplevelSecurityStateKnown = mNewToplevelSecurityStateKnown;
       }
 
       if (temp_NewToplevelSecurityStateKnown)
-        return UpdateSecurityState(aRequest);
+        return UpdateSecurityState(aRequest, PR_FALSE, PR_FALSE, PR_FALSE);
     }
 
     return NS_OK;
   }
 
   return NS_OK;
 }
 
@@ -1244,30 +1246,40 @@ nsSecureBrowserUIImpl::OnStateChange(nsI
 // for bug 412456. We should inline this in a follow up patch.
 void nsSecureBrowserUIImpl::ObtainEventSink(nsIChannel *channel, 
                                             nsCOMPtr<nsISecurityEventSink> &sink)
 {
   if (!sink)
     NS_QueryNotificationCallbacks(channel, sink);
 }
 
-nsresult nsSecureBrowserUIImpl::UpdateSecurityState(nsIRequest* aRequest)
+nsresult nsSecureBrowserUIImpl::UpdateSecurityState(nsIRequest* aRequest, 
+                                                    PRBool withNewLocation, 
+                                                    PRBool withUpdateStatus, 
+                                                    PRBool withUpdateTooltip)
 {
   lockIconState warnSecurityState = lis_no_security;
   PRBool showWarning = PR_FALSE;
+  nsresult rv = NS_OK;
 
-  UpdateMyFlags(showWarning, warnSecurityState);
-  return TellTheWorld(showWarning, warnSecurityState, aRequest);
+  PRBool flagsChanged = UpdateMyFlags(showWarning, warnSecurityState);
+
+  if (flagsChanged || withNewLocation || withUpdateStatus || withUpdateTooltip)
+    rv = TellTheWorld(showWarning, warnSecurityState, aRequest);
+
+  return rv;
 }
 
 // must not fail, by definition, only trivial assignments
 // or string operations are allowed
-void nsSecureBrowserUIImpl::UpdateMyFlags(PRBool &showWarning, lockIconState &warnSecurityState)
+// returns true if our overall state has changed and we must send out notifications
+PRBool nsSecureBrowserUIImpl::UpdateMyFlags(PRBool &showWarning, lockIconState &warnSecurityState)
 {
   nsAutoMonitor lock(mMonitor);
+  PRBool mustTellTheWorld = PR_FALSE;
 
   lockIconState newSecurityState;
 
   if (mNewToplevelSecurityState & STATE_IS_SECURE)
   {
     if (mNewToplevelSecurityState & STATE_SECURE_LOW
         ||
         mNewToplevelSecurityState & STATE_SECURE_MED)
@@ -1317,18 +1329,18 @@ void nsSecureBrowserUIImpl::UpdateMyFlag
 
   PR_LOG(gSecureDocLog, PR_LOG_DEBUG,
          ("SecureUI:%p: UpdateSecurityState:  old-new  %d - %d\n", this,
          mNotifiedSecurityState, newSecurityState
           ));
 
   if (mNotifiedSecurityState != newSecurityState)
   {
-    // must show alert
-    
+    mustTellTheWorld = PR_TRUE;
+
     // we'll treat "broken" exactly like "insecure",
     // i.e. we do not show alerts when switching between broken and insecure
 
     /*
       from                 to           shows alert
     ------------------------------     ---------------
 
     no or broken -> no or broken    => <NOTHING SHOWN>
@@ -1389,17 +1401,22 @@ void nsSecureBrowserUIImpl::UpdateMyFlag
 
     if (lis_no_security == newSecurityState)
     {
       mSSLStatus = nsnull;
       mInfoTooltip.Truncate();
     }
   }
 
-  mNotifiedToplevelIsEV = mNewToplevelIsEV;
+  if (mNotifiedToplevelIsEV != mNewToplevelIsEV) {
+    mustTellTheWorld = PR_TRUE;
+    mNotifiedToplevelIsEV = mNewToplevelIsEV;
+  }
+
+  return mustTellTheWorld;
 }
 
 nsresult nsSecureBrowserUIImpl::TellTheWorld(PRBool showWarning, 
                                              lockIconState warnSecurityState, 
                                              nsIRequest* aRequest)
 {
   nsCOMPtr<nsISecurityEventSink> temp_ToplevelEventSink;
   lockIconState temp_NotifiedSecurityState;
@@ -1516,17 +1533,17 @@ nsSecureBrowserUIImpl::OnLocationChange(
 
   nsCOMPtr<nsIDOMWindow> windowForProgress;
   aWebProgress->GetDOMWindow(getter_AddRefs(windowForProgress));
 
   nsCOMPtr<nsISupports> securityInfo(ExtractSecurityInfo(aRequest));
 
   if (windowForProgress.get() == window.get()) {
     // For toplevel channels, update the security state right away.
-    return EvaluateAndUpdateSecurityState(aRequest, securityInfo);
+    return EvaluateAndUpdateSecurityState(aRequest, securityInfo, PR_TRUE);
   }
 
   // For channels in subdocuments we only update our subrequest state members.
   UpdateSubrequestMembers(securityInfo);
 
   // Care for the following scenario:
 
   // A new toplevel document load might have already started, but the security
@@ -1540,17 +1557,17 @@ nsSecureBrowserUIImpl::OnLocationChange(
 
   PRBool temp_NewToplevelSecurityStateKnown;
   {
     nsAutoMonitor lock(mMonitor);
     temp_NewToplevelSecurityStateKnown = mNewToplevelSecurityStateKnown;
   }
 
   if (temp_NewToplevelSecurityStateKnown)
-    return UpdateSecurityState(aRequest);
+    return UpdateSecurityState(aRequest, PR_TRUE, PR_FALSE, PR_FALSE);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSecureBrowserUIImpl::OnStatusChange(nsIWebProgress* aWebProgress,
                                       nsIRequest* aRequest,
                                       nsresult aStatus,
--- a/security/manager/boot/src/nsSecureBrowserUIImpl.h
+++ b/security/manager/boot/src/nsSecureBrowserUIImpl.h
@@ -125,23 +125,25 @@ protected:
   PRInt32 mDocumentRequestsInProgress;
   PRInt32 mSubRequestsHighSecurity;
   PRInt32 mSubRequestsLowSecurity;
   PRInt32 mSubRequestsBrokenSecurity;
   PRInt32 mSubRequestsNoSecurity;
 
   static already_AddRefed<nsISupports> ExtractSecurityInfo(nsIRequest* aRequest);
   static nsresult MapInternalToExternalState(PRUint32* aState, lockIconState lock, PRBool ev);
-  nsresult UpdateSecurityState(nsIRequest* aRequest);
-  void UpdateMyFlags(PRBool &showWarning, lockIconState &warnSecurityState);
+  nsresult UpdateSecurityState(nsIRequest* aRequest, PRBool withNewLocation,
+                               PRBool withUpdateStatus, PRBool withUpdateTooltip);
+  PRBool UpdateMyFlags(PRBool &showWarning, lockIconState &warnSecurityState);
   nsresult TellTheWorld(PRBool showWarning, 
                         lockIconState warnSecurityState, 
                         nsIRequest* aRequest);
 
-  nsresult EvaluateAndUpdateSecurityState(nsIRequest* aRequest, nsISupports *info);
+  nsresult EvaluateAndUpdateSecurityState(nsIRequest* aRequest, nsISupports *info,
+                                          PRBool withNewLocation);
   void UpdateSubrequestMembers(nsISupports *securityInfo);
 
   void ObtainEventSink(nsIChannel *channel, 
                        nsCOMPtr<nsISecurityEventSink> &sink);
 
   nsCOMPtr<nsISupports> mSSLStatus;
   nsCOMPtr<nsISupports> mCurrentToplevelSecurityInfo;