Bug 481553 - Make imgRequests not try to update their status in the image cache when they aren't stored in it. r=vlad
authorJoe Drew <joe@drew.ca>
Wed, 04 Mar 2009 22:56:14 -0500
changeset 25748 dc3e1ab79397e6876af8c7413c7641436babe064
parent 25747 21ffeb54ece8d8d3d279b005436dcb83748af9de
child 25750 7313e9cc07245d4e6b3df8dd0a23633c667c57fa
push id5721
push userjdrew@mozilla.com
push dateThu, 05 Mar 2009 03:58:02 +0000
treeherdermozilla-central@dc3e1ab79397 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvlad
bugs481553
milestone1.9.2a1pre
Bug 481553 - Make imgRequests not try to update their status in the image cache when they aren't stored in it. r=vlad
modules/libpr0n/src/imgLoader.cpp
modules/libpr0n/src/imgRequest.cpp
modules/libpr0n/src/imgRequest.h
--- a/modules/libpr0n/src/imgLoader.cpp
+++ b/modules/libpr0n/src/imgLoader.cpp
@@ -687,16 +687,18 @@ PRBool imgLoader::PutIntoCache(nsIURI *k
   // the cache.
   nsRefPtr<imgCacheEntry> tmpCacheEntry;
   if (cache.Get(spec, getter_AddRefs(tmpCacheEntry)) && tmpCacheEntry) {
     PR_LOG(gImgLog, PR_LOG_DEBUG,
            ("[this=%p] imgLoader::PutIntoCache -- Element already in the cache", nsnull));
     nsRefPtr<imgRequest> tmpRequest = getter_AddRefs(tmpCacheEntry->GetRequest());
     void *cacheId = NS_GetCurrentThread();
 
+    // If the existing request is currently loading, or loading on a different
+    // thread, we'll leave it be, and not put this new entry into the cache.
     if (!tmpRequest->IsReusable(cacheId))
       return PR_FALSE;
 
     // If it already exists, and we're putting the same key into the cache, we
     // should remove the old version.
     PR_LOG(gImgLog, PR_LOG_DEBUG,
            ("[this=%p] imgLoader::PutIntoCache -- Replacing cached element", nsnull));
 
@@ -1262,17 +1264,18 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIUR
       PR_LOG(gImgLog, PR_LOG_DEBUG,
              ("[this=%p] imgLoader::LoadImage -- AsyncOpen() failed: 0x%x\n",
               this, openRes));
       request->CancelAndAbort(openRes);
       return openRes;
     }
 
     // Try to add the new request into the cache.
-    PutIntoCache(aURI, entry);
+    if (!PutIntoCache(aURI, entry))
+      request->SetCacheable(PR_FALSE);
 
   // If we did get a cache hit, use it.
   } else {
     // XXX: Should this be executed if an expired cache entry does not have a caching channel??
     LOG_MSG_WITH_PARAM(gImgLog, 
                        "imgLoader::LoadImage |cache hit|", "request", request);
 
     // Update the request's LoadId
@@ -1398,17 +1401,18 @@ NS_IMETHODIMP imgLoader::LoadImageWithCh
     NS_ADDREF(pl);
 
     *listener = static_cast<nsIStreamListener*>(pl);
     NS_ADDREF(*listener);
 
     NS_RELEASE(pl);
 
     // Try to add the new request into the cache.
-    PutIntoCache(uri, entry);
+    if (!PutIntoCache(uri, entry))
+      request->SetCacheable(PR_FALSE);
   }
 
   // XXX: It looks like the wrong load flags are being passed in...
   requestFlags &= 0xFFFF;
 
   rv = CreateNewProxyForRequest(request, loadGroup, aObserver,
                                 requestFlags, nsnull, _retval);
   request->NotifyProxyListener(static_cast<imgRequestProxy*>(*_retval));
@@ -1677,17 +1681,18 @@ NS_IMETHODIMP imgCacheValidator::OnStart
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   mDestListener = static_cast<nsIStreamListener*>(pl);
 
   // Try to add the new request into the cache. Note that the entry must be in
   // the cache before the proxies' ownership changes, because adding a proxy
   // changes the caching behaviour for imgRequests.
-  sImgLoader.PutIntoCache(uri, entry);
+  if (!sImgLoader.PutIntoCache(uri, entry))
+    request->SetCacheable(PR_FALSE);
 
   PRUint32 count = mProxies.Count();
   for (PRInt32 i = count-1; i>=0; i--) {
     imgRequestProxy *proxy = static_cast<imgRequestProxy *>(mProxies[i]);
     proxy->ChangeOwner(request);
     request->NotifyProxyListener(proxy);
   }
 
--- a/modules/libpr0n/src/imgRequest.cpp
+++ b/modules/libpr0n/src/imgRequest.cpp
@@ -77,20 +77,20 @@ PRLogModuleInfo *gImgLog = PR_NewLogModu
 NS_IMPL_ISUPPORTS8(imgRequest, imgILoad,
                    imgIDecoderObserver, imgIContainerObserver,
                    nsIStreamListener, nsIRequestObserver,
                    nsISupportsWeakReference,
                    nsIChannelEventSink,
                    nsIInterfaceRequestor)
 
 imgRequest::imgRequest() : 
-  mLoading(PR_FALSE), mProcessing(PR_FALSE), mHadLastPart(PR_FALSE),
-  mGotData(PR_FALSE), mImageStatus(imgIRequest::STATUS_NONE),
-  mState(0), mCacheId(0), mValidator(nsnull), mIsMultiPartChannel(PR_FALSE),
-  mImageSniffers("image-sniffing-services") 
+  mImageStatus(imgIRequest::STATUS_NONE), mState(0), mCacheId(0), 
+  mValidator(nsnull), mImageSniffers("image-sniffing-services"), 
+  mIsMultiPartChannel(PR_FALSE), mLoading(PR_FALSE), mProcessing(PR_FALSE),
+  mHadLastPart(PR_FALSE), mGotData(PR_FALSE), mIsCacheable(PR_TRUE)
 {
   /* member initializers and constructor code */
 }
 
 imgRequest::~imgRequest()
 {
   if (mKeyURI) {
     nsCAutoString spec;
@@ -298,17 +298,17 @@ nsresult imgRequest::NotifyProxyListener
   if (mState & onStopContainer)
     proxy->OnStopContainer(mImage);
 
   // OnStopDecode
   if (mState & onStopDecode)
     proxy->OnStopDecode(GetResultFromImageStatus(mImageStatus), nsnull);
 
   if (mImage && !HaveProxyWithObserver(proxy) && proxy->HasObserver()) {
-    LOG_MSG(gImgLog, "imgRequest::AddProxy", "resetting animation");
+    LOG_MSG(gImgLog, "imgRequest::NotifyProxyListener", "resetting animation");
 
     mImage->ResetAnimation();
   }
 
   if (mState & onStopRequest) {
     proxy->OnStopRequest(nsnull, nsnull,
                          GetResultFromImageStatus(mImageStatus),
                          mHadLastPart);
@@ -414,22 +414,24 @@ nsresult imgRequest::GetSecurityInfo(nsI
   NS_IF_ADDREF(*aSecurityInfo = mSecurityInfo);
   return NS_OK;
 }
 
 void imgRequest::RemoveFromCache()
 {
   LOG_SCOPE(gImgLog, "imgRequest::RemoveFromCache");
 
-  if (mCacheEntry)
-    imgLoader::RemoveFromCache(mCacheEntry);
-  else
-    imgLoader::RemoveFromCache(mKeyURI);
+  if (mIsCacheable) {
+    if (mCacheEntry)
+      imgLoader::RemoveFromCache(mCacheEntry);
+    else
+      imgLoader::RemoveFromCache(mKeyURI);
 
-  mCacheEntry = nsnull;
+    mCacheEntry = nsnull;
+  }
 }
 
 PRBool imgRequest::HaveProxyWithObserver(imgRequestProxy* aProxyToIgnore) const
 {
   nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
   imgRequestProxy* proxy;
   while (iter.HasMore()) {
     proxy = iter.GetNext();
@@ -466,16 +468,25 @@ void imgRequest::AdjustPriority(imgReque
   if (mObservers.SafeElementAt(0, nsnull) != proxy)
     return;
 
   nsCOMPtr<nsISupportsPriority> p = do_QueryInterface(mRequest);
   if (p)
     p->AdjustPriority(delta);
 }
 
+void imgRequest::SetCacheable(PRBool cacheable)
+{
+  LOG_FUNC_WITH_PARAM(gImgLog, "imgRequest::SetIsCacheable", "cacheable", cacheable);
+  mIsCacheable = cacheable;
+
+  if (!mIsCacheable)
+    mCacheEntry = nsnull;
+}
+
 /** imgILoad methods **/
 
 NS_IMETHODIMP imgRequest::SetImage(imgIContainer *aImage)
 {
   LOG_FUNC(gImgLog, "imgRequest::SetImage");
 
   mImage = aImage;
 
@@ -1082,14 +1093,16 @@ imgRequest::OnChannelRedirect(nsIChannel
   newChannel->GetOriginalURI(getter_AddRefs(mKeyURI));
 
 #if defined(PR_LOGGING)
   mKeyURI->GetSpec(spec);
 
   LOG_MSG_WITH_PARAM(gImgLog, "imgRequest::OnChannelRedirect", "new", spec.get());
 #endif
 
-  // If we don't still have a cache entry, we don't want to refresh the cache.
-  if (mKeyURI && mCacheEntry)
-    imgLoader::PutIntoCache(mKeyURI, mCacheEntry);
+  if (mIsCacheable) {
+    // If we don't still have a cache entry, we don't want to refresh the cache.
+    if (mKeyURI && mCacheEntry)
+      imgLoader::PutIntoCache(mKeyURI, mCacheEntry);
+  }
 
   return rv;
 }
--- a/modules/libpr0n/src/imgRequest.h
+++ b/modules/libpr0n/src/imgRequest.h
@@ -159,16 +159,21 @@ private:
 
   // Adjust the priority of the underlying network request by the given delta
   // 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; }
 
+  // Set whether this request is cacheable. By default, all requests are
+  // cacheable, but they might not be if there is already a request with this
+  // key URI in the cache.
+  void SetCacheable(PRBool cacheable);
+
 public:
   NS_DECL_IMGILOAD
   NS_DECL_IMGIDECODEROBSERVER
   NS_DECL_IMGICONTAINEROBSERVER
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIINTERFACEREQUESTOR
@@ -184,30 +189,31 @@ private:
   nsCOMPtr<imgIDecoder> mDecoder;
   nsCOMPtr<nsIProperties> mProperties;
   nsCOMPtr<nsISupports> mSecurityInfo;
   nsCOMPtr<nsIChannel> mChannel;
   nsCOMPtr<nsIInterfaceRequestor> mPrevChannelSink;
 
   nsTObserverArray<imgRequestProxy*> mObservers;
 
-  PRPackedBool mLoading;
-  PRPackedBool mProcessing;
-  PRPackedBool mHadLastPart;
-  PRPackedBool mGotData;
   PRUint32 mImageStatus;
   PRUint32 mState;
   nsCString mContentType;
 
   nsRefPtr<imgCacheEntry> mCacheEntry; /* we hold on to this to this so long as we have observers */
 
   void *mCacheId;
 
   void *mLoadId;
   PRTime mLoadTime;
 
   imgCacheValidator *mValidator;
-  PRBool   mIsMultiPartChannel;
+  nsCategoryCache<nsIContentSniffer> mImageSniffers;
 
-  nsCategoryCache<nsIContentSniffer> mImageSniffers;
+  PRPackedBool mIsMultiPartChannel : 1;
+  PRPackedBool mLoading : 1;
+  PRPackedBool mProcessing : 1;
+  PRPackedBool mHadLastPart : 1;
+  PRPackedBool mGotData : 1;
+  PRPackedBool mIsCacheable : 1;
 };
 
 #endif