Bug 455367. Don't just assume that all image loads get data; ask imagelib instead. r=kaie, r=joedrew, sr=jst

modules/libpr0n/src/imgRequest.cpp

@@ -83,8 +83,8 @@ NS_IMPL_ISUPPORTS8(imgRequest, imgILoad,
 
 imgRequest::imgRequest() : 
   mLoading(PR_FALSE), mProcessing(PR_FALSE), mHadLastPart(PR_FALSE),
-  mImageStatus(imgIRequest::STATUS_NONE), mState(0), mCacheId(0),
+  mGotData(PR_FALSE), mImageStatus(imgIRequest::STATUS_NONE),
-  mValidator(nsnull), mIsMultiPartChannel(PR_FALSE),
+  mState(0), mCacheId(0), mValidator(nsnull), mIsMultiPartChannel(PR_FALSE),
   mImageSniffers("image-sniffing-services") 
 {
   /* member initializers and constructor code */
@@ -833,6 +833,8 @@ NS_IMETHODIMP imgRequest::OnDataAvailable(nsIRequest *aRequest, nsISupports *ctx
 
   NS_ASSERTION(aRequest, "imgRequest::OnDataAvailable -- no request!");
 
+  mGotData = PR_TRUE;
+
   if (!mProcessing) {
     LOG_SCOPE(gImgLog, "imgRequest::OnDataAvailable |First time through... finding mimetype|");
 

modules/libpr0n/src/imgRequest.h

@@ -152,6 +152,9 @@ private:
   // on behalf of the given proxy.
   void AdjustPriority(imgRequestProxy *aProxy, PRInt32 aDelta);
 
+  // Return whether we've seen some data at this point
+  PRBool HasTransferredData() const { return mGotData; }
+
 public:
   NS_DECL_IMGILOAD
   NS_DECL_IMGIDECODEROBSERVER
@@ -180,6 +183,7 @@ private:
   PRPackedBool mLoading;
   PRPackedBool mProcessing;
   PRPackedBool mHadLastPart;
+  PRPackedBool mGotData;
   PRUint32 mImageStatus;
   PRUint32 mState;
   nsCString mContentType;

modules/libpr0n/src/imgRequestProxy.cpp

@@ -412,6 +412,17 @@ NS_IMETHODIMP imgRequestProxy::GetSecurityInfo(nsISupports** _retval)
   return NS_OK;
 }
 
+NS_IMETHODIMP imgRequestProxy::GetHasTransferredData(PRBool* hasData)
+{
+  if (mOwner) {
+    *hasData = mOwner->HasTransferredData();
+  } else {
+    // The safe thing to do is to claim we have data
+    *hasData = PR_TRUE;
+  }
+  return NS_OK;
+}
+
 /** imgIContainerObserver methods **/
 
 void imgRequestProxy::FrameChanged(imgIContainer *container, gfxIImageFrame *newframe, nsIntRect * dirtyRect)

netwerk/base/public/nsISecurityInfoProvider.idl

@@ -37,8 +37,17 @@
  
 #include "nsISupports.idl"
 
-[scriptable, uuid(9E03B4C6-8B79-41a8-A3E5-C41F9E015598)]
+[scriptable, uuid(b8cc9126-9319-4415-afd9-b82220d453ed)]
 interface nsISecurityInfoProvider : nsISupports
 {
+  /**
+   * The security info for this provider, if any.
+   */
   readonly attribute nsISupports securityInfo;
+
+  /**
+   * Whether this provider has transferred data.  If it hasn't, its
+   * security info should be ignored.
+   */
+  readonly attribute boolean hasTransferredData;
 };

security/manager/boot/src/nsSecureBrowserUIImpl.cpp

@@ -823,17 +823,10 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress,
 #endif
 
   PRBool isSubDocumentRelevant = PR_TRUE;
-  PRBool isImageRequest = PR_FALSE;
 
   // We are only interested in requests that load in the browser window...
   nsCOMPtr<imgIRequest> imgRequest(do_QueryInterface(aRequest));
   if (imgRequest) {
-    // Remember this is an image request. Because image loads doesn't
-    // support any TRANSFERRING notifications but only START and
-    // STOP we must simply predict there were a content transferred.
-    // See bug 432685 for details.
-    isImageRequest = PR_TRUE;
-
     // for image requests, we get the URI from here
     imgRequest->GetURI(getter_AddRefs(uri));
   } else { // is not imgRequest
@@ -998,11 +991,8 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress,
     // The listing of a request in mTransferringRequests
     // means, there has already been data transfered.
 
-    if (!isImageRequest) 
-    {
     nsAutoMonitor lock(mMonitor);
     PL_DHashTableOperate(&mTransferringRequests, aRequest, PL_DHASH_ADD);
-    }
     
     return NS_OK;
   }
@@ -1013,12 +1003,7 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress,
       &&
       aProgressStateFlags & STATE_IS_REQUEST)
   {
-    if (isImageRequest) 
+    { /* scope for the nsAutoMonitor */
-    {
-      requestHasTransferedData = PR_TRUE;
-    }
-    else
-    {
       nsAutoMonitor lock(mMonitor);
       PLDHashEntryHdr *entry = PL_DHashTableOperate(&mTransferringRequests, aRequest, PL_DHASH_LOOKUP);
       if (PL_DHASH_ENTRY_IS_BUSY(entry))
@@ -1028,6 +1013,22 @@ nsSecureBrowserUIImpl::OnStateChange(nsIWebProgress* aWebProgress,
         requestHasTransferedData = PR_TRUE;
       }
     }
+
+    if (!requestHasTransferedData) {
+      // Because image loads doesn't support any TRANSFERRING notifications but
+      // only START and STOP we must ask them directly whether content was
+      // transferred.  See bug 432685 for details.
+      nsCOMPtr<nsISecurityInfoProvider> securityInfoProvider =
+        do_QueryInterface(aRequest);
+      // Guess true in all failure cases to be safe.  But if we're not
+      // an nsISecurityInfoProvider, then we just haven't transferred
+      // any data.
+      PRBool hasTransferred;
+      requestHasTransferedData =
+        securityInfoProvider &&
+        (NS_FAILED(securityInfoProvider->GetHasTransferredData(&hasTransferred)) ||
+         hasTransferred);
+    }
   }
 
   PRBool allowSecurityStateChange = PR_TRUE;