Fix a bunch of issues relating to dynamic changes of border images. (Bug 445810) r=robarnold sr=bzbarsky
authorL. David Baron <dbaron@dbaron.org>
Sat, 26 Jul 2008 09:14:49 -0700
changeset 16232 7370786111c2e1870d4b46c4d303ae4bcb286f98
parent 16231 420a21e47c4ec73509b09a83f391b7be3f590c97
child 16233 4dee7a27839916e346dacf5fd44b70a5b3d01cfd
push idunknown
push userunknown
push dateunknown
reviewersrobarnold, bzbarsky
bugs445810
milestone1.9.1a2pre
Fix a bunch of issues relating to dynamic changes of border images. (Bug 445810) r=robarnold sr=bzbarsky
layout/base/nsFrameManager.cpp
layout/base/nsImageLoader.cpp
layout/base/nsPresContext.cpp
layout/base/nsPresContext.h
layout/base/tests/Makefile.in
layout/base/tests/test_bug445810.html
layout/style/nsStyleStruct.cpp
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -86,16 +86,17 @@
 #include "nsContentUtils.h"
 #include "nsReadableUtils.h"
 #include "nsUnicharUtils.h"
 #include "nsPrintfCString.h"
 #include "nsLayoutErrors.h"
 #include "nsLayoutUtils.h"
 #include "nsAutoPtr.h"
 #include "imgIRequest.h"
+#include "nsStyleStructInlines.h"
 
 #include "nsFrameManager.h"
 #ifdef ACCESSIBILITY
 #include "nsIAccessibilityService.h"
 #include "nsIAccessibleEvent.h"
 #endif
 
   #ifdef DEBUG
@@ -1058,16 +1059,34 @@ CaptureChange(nsStyleContext* aOldContex
   nsChangeHint ourChange = aOldContext->CalcStyleDifference(aNewContext);
   NS_UpdateHint(ourChange, aChangeToAssume);
   if (NS_UpdateHint(aMinChange, ourChange)) {
     aChangeList->AppendChange(aFrame, aContent, ourChange);
   }
   return aMinChange;
 }
 
+static PRBool
+ShouldStopImage(imgIRequest *aOldImage, imgIRequest *aNewImage)
+{
+  if (!aOldImage)
+    return PR_FALSE;
+
+  PRBool stopImages = !aNewImage;
+  if (!stopImages) {
+    nsCOMPtr<nsIURI> oldURI, newURI;
+    aOldImage->GetURI(getter_AddRefs(oldURI));
+    aNewImage->GetURI(getter_AddRefs(newURI));
+    PRBool equal;
+    stopImages =
+      NS_FAILED(oldURI->Equals(newURI, &equal)) || !equal;
+  }
+  return stopImages;
+}
+
 nsChangeHint
 nsFrameManager::ReResolveStyleContext(nsPresContext    *aPresContext,
                                       nsIFrame          *aFrame,
                                       nsIContent        *aParentContent,
                                       nsStyleChangeList *aChangeList, 
                                       nsChangeHint       aMinChange)
 {
   // XXXldb get new context from prev-in-flow if possible, to avoid
@@ -1195,33 +1214,40 @@ nsFrameManager::ReResolveStyleContext(ns
                                    content, aChangeList, aMinChange,
                                    assumeDifferenceHint);
         if (!(aMinChange & nsChangeHint_ReconstructFrame)) {
           // if frame gets regenerated, let it keep old context
           aFrame->SetStyleContext(newContext);
         }
         // if old context had image and new context does not have the same image, 
         // stop the image load for the frame
-        const nsStyleBackground* oldColor = oldContext->GetStyleBackground();
-        const nsStyleBackground* newColor = newContext->GetStyleBackground();
+        if (ShouldStopImage(
+              oldContext->GetStyleBackground()->mBackgroundImage,
+              newContext->GetStyleBackground()->mBackgroundImage)) {
+          // stop the image loading for the frame, the image has changed
+          aPresContext->StopBackgroundImageFor(aFrame);
+        }
 
-        if (oldColor->mBackgroundImage) {
-          PRBool stopImages = !newColor->mBackgroundImage;
-          if (!stopImages) {
-            nsCOMPtr<nsIURI> oldURI, newURI;
-            oldColor->mBackgroundImage->GetURI(getter_AddRefs(oldURI));
-            newColor->mBackgroundImage->GetURI(getter_AddRefs(newURI));
-            PRBool equal;
-            stopImages =
-              NS_FAILED(oldURI->Equals(newURI, &equal)) || !equal;
-          }
-          if (stopImages) {
-            // stop the image loading for the frame, the image has changed
-            aPresContext->StopImagesFor(aFrame);
-          }
+        imgIRequest *newBorderImage =
+          newContext->GetStyleBorder()->GetBorderImage();
+        if (ShouldStopImage(oldContext->GetStyleBorder()->GetBorderImage(),
+                            newBorderImage)) {
+          // stop the image loading for the frame, the image has changed
+          aPresContext->StopBorderImageFor(aFrame);
+        }
+
+        // Since the CalcDifference call depended on the result of
+        // GetActualBorder() and that result depends on whether the
+        // image has loaded, start the image load now so that we'll get
+        // notified when it completes loading and can do a restyle.
+        // Otherwise, the image might finish loading from the network
+        // before we start listening to its notifications, and then
+        // we'll never know that it's finished loading.
+        if (newBorderImage) {
+          aPresContext->LoadBorderImage(newBorderImage, aFrame);
         }
       }
       oldContext->Release();
     }
     else {
       NS_ERROR("resolve style context failed");
       newContext = oldContext;  // new context failed, recover... (take ref)
       oldContext = nsnull;
--- a/layout/base/nsImageLoader.cpp
+++ b/layout/base/nsImageLoader.cpp
@@ -209,21 +209,18 @@ NS_IMETHODIMP nsImageLoader::FrameChange
 
 void
 nsImageLoader::RedrawDirtyFrame(const nsRect* aDamageRect)
 {
   if (mReflowOnLoad) {
     nsIPresShell *shell = mPresContext->GetPresShell();
     nsresult rv = shell->FrameNeedsReflow(mFrame, nsIPresShell::eStyleChange, NS_FRAME_IS_DIRTY);
     NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Could not reflow after loading border-image");
-    // Note that we're assuming that the resulting reflow will
-    // invalidate the entire frame.  Given that we only set
-    // mReflowOnLoad if the actual border width will change when we do
-    // this reflow, this invalidate should happen.
-    return;
+    // The reflow might not do all the invalidation we need, so continue
+    // on with the invalidation codepath.
   }
   // NOTE: It is not sufficient to invalidate only the size of the image:
   //       the image may be tiled! 
   //       The best option is to call into the frame, however lacking this
   //       we have to at least invalidate the frame's bounds, hence
   //       as long as we have a frame we'll use its size.
   //
 
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -1210,35 +1210,34 @@ nsPresContext::LoadBorderImage(imgIReque
 {
   return DoLoadImage(mBorderImageLoaders, aImage, aTargetFrame,
                      aTargetFrame->GetStyleBorder()->ImageBorderDiffers());
 }
 
 void
 nsPresContext::StopImagesFor(nsIFrame* aTargetFrame)
 {
+  StopBackgroundImageFor(aTargetFrame);
+  StopBorderImageFor(aTargetFrame);
+}
+
+void
+nsPresContext::DoStopImageFor(nsPresContext::ImageLoaderTable& aTable,
+                              nsIFrame* aTargetFrame)
+{
   nsCOMPtr<nsImageLoader> loader;
-  mImageLoaders.Get(aTargetFrame, getter_AddRefs(loader));
+  aTable.Get(aTargetFrame, getter_AddRefs(loader));
 
   if (loader) {
     loader->Destroy();
 
-    mImageLoaders.Remove(aTargetFrame);
-  }
-  
-  mBorderImageLoaders.Get(aTargetFrame, getter_AddRefs(loader));
-  
-  if (loader) {
-    loader->Destroy();
-
-    mBorderImageLoaders.Remove(aTargetFrame);
+    aTable.Remove(aTargetFrame);
   }
 }
-
-
+  
 void
 nsPresContext::SetContainer(nsISupports* aHandler)
 {
   mContainer = do_GetWeakReference(aHandler);
   if (mContainer) {
     GetDocumentColorPreferences();
   }
 }
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -389,18 +389,25 @@ public:
 
 private:
   typedef nsInterfaceHashtable<nsVoidPtrHashKey, nsImageLoader> ImageLoaderTable;
 
   NS_HIDDEN_(imgIRequest*) DoLoadImage(ImageLoaderTable& aTable,
                                        imgIRequest* aImage,
                                        nsIFrame* aTargetFrame,
                                        PRBool aReflowOnLoad);
+
+  NS_HIDDEN_(void) DoStopImageFor(ImageLoaderTable& aTable,
+                                  nsIFrame* aTargetFrame);
 public:
 
+  NS_HIDDEN_(void) StopBackgroundImageFor(nsIFrame* aTargetFrame)
+  { DoStopImageFor(mImageLoaders, aTargetFrame); }
+  NS_HIDDEN_(void) StopBorderImageFor(nsIFrame* aTargetFrame)
+  { DoStopImageFor(mBorderImageLoaders, aTargetFrame); }
   /**
    * This method is called when a frame is being destroyed to
    * ensure that the image load gets disassociated from the prescontext
    */
   NS_HIDDEN_(void) StopImagesFor(nsIFrame* aTargetFrame);
 
   NS_HIDDEN_(void) SetContainer(nsISupports* aContainer);
 
--- a/layout/base/tests/Makefile.in
+++ b/layout/base/tests/Makefile.in
@@ -87,16 +87,17 @@ ifdef MOZ_MOCHITEST
 		test_bug394057.html \
 		test_bug396024.html \
 		test_bug399284.html \
 		test_bug399951.html \
 		test_bug404209.xhtml \
 		test_bug416896.html \
 		test_bug420499.xul \
 		test_bug423523.html \
+		test_bug445810.html \
 		$(NULL)
 # test_bug396024.html is currently disabled because it interacts badly with
 # the "You can't print-preview while the page is loading" dialog.
 # (See bug 407080)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
 
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/test_bug445810.html
@@ -0,0 +1,108 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=445810
+-->
+<head>
+  <title>Test for Bug 445810</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=445810">Mozilla Bug 445810</a>
+<div><p id="display"></p></div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 445810 **/
+
+function new_image_url()
+{
+  var width = 10;
+  var height = 10;
+
+  var canvas = document.createElement("canvas");
+  canvas.setAttribute("width", width);
+  canvas.setAttribute("height", height);
+  var cx = canvas.getContext("2d");
+  function random_color() {
+    function random_component() {
+      return Math.floor(Math.random() * 256);
+    }
+    return "rgb(" + random_component() + "," +
+                    random_component() + "," +
+                    random_component() + ")";
+  }
+  for (var x = 0; x < width; ++x) {
+    for (var y = 0; y < height; ++y) {
+      cx.fillStyle = random_color();
+      cx.fillRect(x, y, 1, 1);
+    }
+  }
+  return canvas.toDataURL();
+}
+
+SimpleTest.waitForExplicitFinish();
+
+var p = document.getElementById("display");
+var div = p.parentNode;
+var divcs = getComputedStyle(div, "");
+
+div.style.width = "-moz-min-content";
+p.style.width = "5px";
+p.style.height = "5px";
+is(divcs.width, "5px", "correct width without a border");
+is(divcs.height, "5px", "correct height without a border");
+p.style.border = "3px solid";
+is(divcs.width, "11px", // 3 (border-left) + 5 (width) + 3 (border-right)
+   "correct width without a border image");
+is(divcs.height, "11px", // 3 (border-top) + 5 (height) + 3 (border-bottom)
+   "correct height without a border image");
+
+p.style.MozBorderImage = "url( " + new_image_url() + ") 2 2 2 2 / 7px 2px";
+is(divcs.width, "11px", "border image not loaded yet");
+is(divcs.height, "11px", "border image not loaded yet");
+
+setTimeout(step2, 0);
+function step2() {
+  is(divcs.width, "9px", "border image loading caused reflow");
+  is(divcs.height, "19px", "border image loading caused reflow");
+
+  p.style.border = "";
+  is(divcs.width, "9px", "border image still shows with border-style:none");
+  is(divcs.height, "19px", "border image still shows with border-style:none");
+
+  p.style.MozBorderImage = "";
+  is(divcs.width, "5px", "correct width without a border");
+  is(divcs.height, "5px", "correct height without a border");
+
+  p.style.MozBorderImage = "url( " + new_image_url() + ") 2 2 2 2 / 7px 2px";
+  is(divcs.width, "5px", "border image not loaded yet");
+  is(divcs.height, "5px", "border image not loaded yet");
+  setTimeout(step3, 0);
+}
+
+function step3() {
+  is(divcs.width, "9px", "border image loading caused reflow");
+  is(divcs.height, "19px", "border image loading caused reflow");
+
+  p.style.MozBorderImage = "url( " + new_image_url() + ") 2 2 2 2";
+  p.style.border = "3px none";
+  is(divcs.width, "5px", "border image not loaded yet");
+  is(divcs.height, "5px", "border image not loaded yet");
+  setTimeout(step4, 0);
+}
+
+function step4() {
+  is(divcs.width, "11px", "border image loading caused reflow");
+  is(divcs.height, "11px", "border image loading caused reflow");
+
+  SimpleTest.finish();
+}
+
+</script>
+</pre>
+</body>
+</html>
+
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -429,17 +429,17 @@ nsStyleBorder::Destroy(nsPresContext* aC
 }
 
 
 nsChangeHint nsStyleBorder::CalcDifference(const nsStyleBorder& aOther) const
 {
   // Note that differences in mBorder don't affect rendering (which should only
   // use mComputedBorder), so don't need to be tested for here.
   if (mTwipsPerPixel == aOther.mTwipsPerPixel &&
-      mComputedBorder == aOther.mComputedBorder && 
+      GetActualBorder() == aOther.GetActualBorder() && 
       mFloatEdge == aOther.mFloatEdge) {
     // Note that mBorderStyle stores not only the border style but also
     // color-related flags.  Given that we've already done an mComputedBorder
     // comparison, border-style differences can only lead to a VISUAL hint.  So
     // it's OK to just compare the values directly -- if either the actual
     // style or the color flags differ we want to repaint.
     NS_FOR_CSS_SIDES(ix) {
       if (mBorderStyle[ix] != aOther.mBorderStyle[ix] || 
@@ -448,16 +448,27 @@ nsChangeHint nsStyleBorder::CalcDifferen
       }
     }
 
     if (mBorderRadius != aOther.mBorderRadius ||
         !mBorderColors != !aOther.mBorderColors) {
       return NS_STYLE_HINT_VISUAL;
     }
 
+    if (IsBorderImageLoaded() || aOther.IsBorderImageLoaded()) {
+      if (mBorderImage != aOther.mBorderImage ||
+          mBorderImageHFill != aOther.mBorderImageHFill ||
+          mBorderImageVFill != aOther.mBorderImageVFill ||
+          mBorderImageSplit != aOther.mBorderImageSplit) {
+        return NS_STYLE_HINT_VISUAL;
+      }
+      // The call to GetActualBorder above already considered
+      // mBorderImageWidth and mHaveBorderImageWidth.
+    }
+
     // Note that at this point if mBorderColors is non-null so is
     // aOther.mBorderColors
     if (mBorderColors) {
       NS_FOR_CSS_SIDES(ix) {
         if (!mBorderColors[ix] != !aOther.mBorderColors[ix]) {
           return NS_STYLE_HINT_VISUAL;
         } else if (mBorderColors[ix] && aOther.mBorderColors[ix]) {
           if (!mBorderColors[ix]->Equals(aOther.mBorderColors[ix]))