Only do a new image load if we're forced to or if the URI really changed. Bug

285428, r=biesi, sr=jst (note: sr was given in bug 286000 as part of the patch
there).

content/base/src/nsImageLoadingContent.cpp

@@ -387,14 +387,15 @@ nsImageLoadingContent::LoadImageWithChannel(nsIChannel* aChannel,
 // XXX This should be a protected method, not an interface method!!!
 NS_IMETHODIMP
 nsImageLoadingContent::ImageURIChanged(const nsAString& aNewURI) {
-  return ImageURIChanged(NS_ConvertUCS2toUTF8(aNewURI));
+  return ImageURIChanged(aNewURI, PR_TRUE);
 }
 
 /*
  * Non-interface methods
  */
 nsresult
-nsImageLoadingContent::ImageURIChanged(const nsACString& aNewURI)
+nsImageLoadingContent::ImageURIChanged(const nsAString& aNewURI,
+                                       PRBool aForce)
 {
   if (!mLoadingEnabled) {
     return NS_OK;
@@ -413,6 +414,18 @@ nsImageLoadingContent::ImageURIChanged(const nsACString& aNewURI)
   rv = StringToURI(aNewURI, doc, getter_AddRefs(imageURI));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  if (!aForce) {
+    nsCOMPtr<nsIURI> currentURI;
+    GetCurrentURI(getter_AddRefs(currentURI));
+    PRBool equal;
+    if (currentURI &&
+        NS_SUCCEEDED(currentURI->Equals(imageURI, &equal)) &&
+        equal) {
+      // Nothing to do here.
+      return NS_OK;
+    }
+  }
+
   // Remember the URL of this request, in case someone asks us for it later
   // But this only matters if we are affecting the current request
   if (!mCurrentRequest)
@@ -469,11 +482,11 @@ nsImageLoadingContent::ImageURIChanged(const nsACString& aNewURI)
     return NS_OK;
   }
 
-  // Only continue if we have a parent and a document -- that would mean we're
-  // a useful chunk of the content model and _may_ have a frame.  This should
-  // eliminate things like SetAttr calls during the parsing process, as well as
-  // things like setting src on |new Image()|-type things.
-  if (!thisContent->GetDocument() || !thisContent->GetParent()) {
+  // Only continue if we're in a document -- that would mean we're a useful
+  // chunk of the content model and _may_ have a frame.  This should eliminate
+  // things like SetAttr calls during the parsing process, as well as things
+  // like setting src on |new Image()|-type things.
+  if (!thisContent->IsInDoc()) {
     return NS_OK;
   }
 
@@ -560,7 +573,7 @@ nsImageLoadingContent::GetOurDocument()
 }
 
 nsresult
-nsImageLoadingContent::StringToURI(const nsACString& aSpec,
+nsImageLoadingContent::StringToURI(const nsAString& aSpec,
                                    nsIDocument* aDocument,
                                    nsIURI** aURI)
 {

content/base/src/nsImageLoadingContent.h

@@ -73,15 +73,19 @@ protected:
    * ImageURIChanged is called by subclasses when the appropriate
    * attributes (eg 'src' for <img> tags) change.  The string passed
    * in is the new uri string; this consolidates the code for getting
-   * the charset and any other incidentals into this superclass.
+   * the charset, constructing URI objects, and any other incidentals
+   * into this superclass.   
    *
    * Note that this is different from the ImageURIChanged(AString)
-   * declared in nsIImageLoadingContent.idl -- that takes an
-   * nsAString, this takes an nsACString.
+   * declared in nsIImageLoadingContent.idl -- because it allows
+   * control over whether loading is to be forced.
    *
    * @param aNewURI the URI spec to be loaded (may be a relative URI)
+   * @param aForce If true, make sure to load the URI.  If false, only
+   *        load if the URI is different from the currently loaded URI.
    */
-  nsresult ImageURIChanged(const nsACString& aNewURI);
+  nsresult ImageURIChanged(const nsAString& aNewURI,
+                           PRBool aForce);
 
 private:
   /**
@@ -140,7 +144,7 @@ private:
    * @param aDocument the document we belong to
    * @return the URI we want to be loading
    */
-  nsresult StringToURI(const nsACString& aSpec, nsIDocument* aDocument,
+  nsresult StringToURI(const nsAString& aSpec, nsIDocument* aDocument,
                        nsIURI** aURI);
 
   /**

content/html/content/src/nsHTMLImageElement.cpp

@@ -548,8 +548,8 @@ nsHTMLImageElement::SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
   // If we plan to call ImageURIChanged, we want to do it first so that the
   // image load kicks off _before_ the reflow triggered by the SetAttr.  But if
   // aNotify is false, we are coming from the parser or some such place; we'll
-  // get our parent set after all the attributes have been set, so we'll do the
-  // image load from SetParent.  Skip the ImageURIChanged call in that case.
+  // get bound after all the attributes have been set, so we'll do the
+  // image load from BindToTree.  Skip the ImageURIChanged call in that case.
   if (aNotify &&
       aNameSpaceID == kNameSpaceID_None && aName == nsHTMLAtoms::src) {
 
@@ -562,7 +562,9 @@ nsHTMLImageElement::SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
 
     nsCOMPtr<imgIRequest> oldCurrentRequest = mCurrentRequest;
 
-    ImageURIChanged(aValue);
+    // Force image loading here, so that we'll try to load the image from
+    // network if it's set to be not cacheable...
+    ImageURIChanged(aValue, PR_TRUE);
 
     if (mCurrentRequest && !mPendingRequest &&
         oldCurrentRequest != mCurrentRequest) {
@@ -596,7 +598,7 @@ nsHTMLImageElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
   nsAutoString uri;
   nsresult result = GetAttr(kNameSpaceID_None, nsHTMLAtoms::src, uri);
   if (result == NS_CONTENT_ATTR_HAS_VALUE) {
-    ImageURIChanged(uri);
+    ImageURIChanged(uri, PR_FALSE);
   }
 
   return rv;

content/html/content/src/nsHTMLInputElement.cpp

@@ -447,7 +447,7 @@ nsHTMLInputElement::BeforeSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
   } else if (aNotify && aName == nsHTMLAtoms::src &&
              aValue && mType == NS_FORM_INPUT_IMAGE) {
     // Null value means the attr got unset; don't trigger on that
-    ImageURIChanged(*aValue);
+    ImageURIChanged(*aValue, PR_TRUE);
   } else if (aNotify && aName == nsHTMLAtoms::disabled) {
     SET_BOOLBIT(mBitField, BF_DISABLED_CHANGED, PR_TRUE);
   }
@@ -535,7 +535,7 @@ nsHTMLInputElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
       nsAutoString src;
       nsresult rv = GetAttr(kNameSpaceID_None, nsHTMLAtoms::src, src);
       if (rv == NS_CONTENT_ATTR_HAS_VALUE) {
-        ImageURIChanged(src);
+        ImageURIChanged(src, PR_FALSE);
       }
     }
   }
@@ -1690,7 +1690,7 @@ nsHTMLInputElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
     nsAutoString uri;
     nsresult result = GetAttr(kNameSpaceID_None, nsHTMLAtoms::src, uri);
     if (result == NS_CONTENT_ATTR_HAS_VALUE) {
-      ImageURIChanged(uri);
+      ImageURIChanged(uri, PR_FALSE);
     }
   }
 

content/svg/content/src/nsSVGImageElement.cpp

@@ -385,7 +385,7 @@ nsSVGImageElement::DidModifySVGObservable(nsISVGValue* aObservable,
       return NS_OK;
     }
 
-    ImageURIChanged(href);
+    ImageURIChanged(href, PR_TRUE);
   }
 
   return nsSVGImageElementBase::DidModifySVGObservable(aObservable, aModType);