Bug 564991. Part 38: Make invalidation-during-paint work reliably. r=mats,sr=dbaron

layout/base/FrameLayerBuilder.cpp

@@ -333,7 +333,7 @@ FrameLayerBuilder::DestroyDisplayItemData(nsIFrame* aFrame,
 }
 
 void
-FrameLayerBuilder::BeginUpdatingRetainedLayers(LayerManager* aManager)
+FrameLayerBuilder::WillBeginRetainedLayerTransaction(LayerManager* aManager)
 {
   mRetainingManager = aManager;
   LayerManagerData* data = static_cast<LayerManagerData*>
@@ -371,10 +371,20 @@ FrameLayerBuilder::DidEndTransaction(LayerManager* aManager)
     if (root) {
       RemoveThebesItemsForLayerSubtree(root);
     }
-    return;
   }
+}
 
-  // We need to save the data we'll need to support retaining.
+void
+FrameLayerBuilder::WillEndTransaction(LayerManager* aManager)
+{
+  if (aManager != mRetainingManager)
+    return;
+
+  // We need to save the data we'll need to support retaining. We do this
+  // before we paint so that invalidation triggered by painting will
+  // be able to update the ThebesLayerInvalidRegionProperty values
+  // correctly and the NS_FRAME_HAS_CONTAINER_LAYER bits will be set
+  // correctly.
   LayerManagerData* data = static_cast<LayerManagerData*>
     (mRetainingManager->GetUserData());
   if (data) {
@@ -949,7 +959,10 @@ ContainerState::InvalidateForLayerChange(nsDisplayItem* aItem, Layer* aNewLayer)
       }
     }
 
-    mContainerFrame->Invalidate(bounds - mBuilder->ToReferenceFrame(mContainerFrame));
+    mContainerFrame->InvalidateWithFlags(
+        bounds - mBuilder->ToReferenceFrame(mContainerFrame),
+        nsIFrame::INVALIDATE_NO_THEBES_LAYERS |
+        nsIFrame::INVALIDATE_EXCLUDE_CURRENT_PAINT);
   }
 }
 
@@ -1261,7 +1274,9 @@ FrameLayerBuilder::DrawThebesLayer(ThebesLayer* aLayer,
   nscoord appUnitsPerDevPixel = presContext->AppUnitsPerDevPixel();
   nsRect r = (aRegionToInvalidate.GetBounds() + offset).
     ToAppUnits(appUnitsPerDevPixel);
-  containerLayerFrame->Invalidate(r);
+  containerLayerFrame->InvalidateWithFlags(r,
+      nsIFrame::INVALIDATE_NO_THEBES_LAYERS |
+      nsIFrame::INVALIDATE_EXCLUDE_CURRENT_PAINT);
 
   // Our list may contain content with different prescontexts at
   // different zoom levels. 'rc' contains the nsIRenderingContext

layout/base/FrameLayerBuilder.h

@@ -109,13 +109,20 @@ public:
   }
 
   /**
-   * Call this to register a layer tree which was retained since the last
-   * paint.
+   * Call this to notify that we are about to start a transaction on the
+   * retained layer manager aManager.
    */
-  void BeginUpdatingRetainedLayers(LayerManager* aManager);
+  void WillBeginRetainedLayerTransaction(LayerManager* aManager);
 
   /**
-   * Call this whenever we end a transaction on aManager. If aManager
+   * Call this just before we end a transaction on aManager. If aManager
+   * is not the retained layer manager then it must be a temporary layer
+   * manager that will not be used again.
+   */
+  void WillEndTransaction(LayerManager* aManager);
+
+  /**
+   * Call this after we end a transaction on aManager. If aManager
    * is not the retained layer manager then it must be a temporary layer
    * manager that will not be used again.
    */

layout/base/nsDisplayList.cpp

@@ -364,7 +364,7 @@ void nsDisplayList::PaintForFrame(nsDisplayListBuilder* aBuilder,
     if (window) {
       layerManager = window->GetLayerManager();
       if (layerManager) {
-        aBuilder->LayerBuilder()->BeginUpdatingRetainedLayers(layerManager);
+        aBuilder->LayerBuilder()->WillBeginRetainedLayerTransaction(layerManager);
       }
     }
   }
@@ -398,6 +398,7 @@ void nsDisplayList::PaintForFrame(nsDisplayListBuilder* aBuilder,
   root->SetVisibleRegion(nsIntRegion(visible));
 
   layerManager->SetRoot(root);
+  aBuilder->LayerBuilder()->WillEndTransaction(layerManager);
   layerManager->EndTransaction(FrameLayerBuilder::DrawThebesLayer,
                                aBuilder);
   aBuilder->LayerBuilder()->DidEndTransaction(layerManager);

layout/generic/nsFrame.cpp

@@ -3957,6 +3957,9 @@ NS_DECLARE_FRAME_PROPERTY(DeferInvalidatesProperty, nsIFrame::DestroyRegion)
 void
 nsIFrame::InvalidateRoot(const nsRect& aDamageRect, PRUint32 aFlags)
 {
+  NS_ASSERTION(nsLayoutUtils::GetDisplayRootFrame(this) == this,
+               "Can only call this on display roots");
+
   nsRect rect;
   rect.IntersectRect(aDamageRect, nsRect(nsPoint(0,0), GetSize()));
 
@@ -3973,11 +3976,13 @@ nsIFrame::InvalidateRoot(const nsRect& aDamageRect, PRUint32 aFlags)
   if (excludeRegion) {
     flags = NS_VMREFRESH_DEFERRED;
 
-    nsRegion r;
-    r.Sub(rect, *excludeRegion);
-    if (r.IsEmpty())
-      return;
-    rect = r.GetBounds();
+    if (aFlags & INVALIDATE_EXCLUDE_CURRENT_PAINT) {
+      nsRegion r;
+      r.Sub(rect, *excludeRegion);
+      if (r.IsEmpty())
+        return;
+      rect = r.GetBounds();
+    }
   }
 
   nsIView* view = GetView();

layout/generic/nsIFrame.h

@@ -1805,7 +1805,8 @@ public:
    * nsLayoutUtils::GetDisplayRootFrame). This causes all invalidates
    * reaching this frame to be performed asynchronously off an event,
    * instead of being applied to the widget immediately. Also,
-   * invalidation of areas in aExcludeRegion is ignored completely.
+   * invalidation of areas in aExcludeRegion is ignored completely
+   * for invalidates with INVALIDATE_EXCLUDE_CURRENT_PAINT specified.
    * These can't be nested; two invocations of
    * BeginDeferringInvalidatesForDisplayRoot for a frame must have a
    * EndDeferringInvalidatesForDisplayRoot between them.
@@ -1884,15 +1885,27 @@ public:
    * @param aFlags INVALIDATE_NO_THEBES_LAYERS: don't invalidate the
    * ThebesLayers of any container layer owned by an ancestor. Set this
    * only if ThebesLayers definitely don't need to be updated.
+   * @param aFlags INVALIDATE_EXCLUDE_CURRENT_PAINT: if the invalidation
+   * occurs while we're painting (to be precise, while
+   * BeginDeferringInvalidatesForDisplayRoot is active on the display root),
+   * then invalidation in the current paint region is simply discarded.
+   * Use this flag if areas that are being painted do not need
+   * to be invalidated. By default, when this flag is not specified,
+   * areas that are invalidated while currently being painted will be repainted
+   * again.
+   * This flag is useful when, during painting, FrameLayerBuilder discovers that
+   * a region of the window needs to be drawn differently, and that region
+   * may or may not be contained in the currently painted region.
    */
   enum {
-  	INVALIDATE_IMMEDIATE = 0x01,
-  	INVALIDATE_CROSS_DOC = 0x02,
-  	INVALIDATE_REASON_SCROLL_BLIT = 0x04,
-  	INVALIDATE_REASON_SCROLL_REPAINT = 0x08,
+    INVALIDATE_IMMEDIATE = 0x01,
+    INVALIDATE_CROSS_DOC = 0x02,
+    INVALIDATE_REASON_SCROLL_BLIT = 0x04,
+    INVALIDATE_REASON_SCROLL_REPAINT = 0x08,
     INVALIDATE_REASON_MASK = INVALIDATE_REASON_SCROLL_BLIT |
                              INVALIDATE_REASON_SCROLL_REPAINT,
-    INVALIDATE_NO_THEBES_LAYERS = 0x10
+    INVALIDATE_NO_THEBES_LAYERS = 0x10,
+    INVALIDATE_EXCLUDE_CURRENT_PAINT = 0x20
   };
   virtual void InvalidateInternal(const nsRect& aDamageRect,
                                   nscoord aOffsetX, nscoord aOffsetY,

layout/generic/test/Makefile.in

@@ -94,6 +94,7 @@ _TEST_FILES = \
 		$(warning test_bug507902.html temporarily disabled - see bug 510001) \
 		test_bug514732.html \
 		test_bug527306.html \
+		test_invalidate_during_plugin_paint.html \
 		test_movement_by_characters.html \
 		test_movement_by_words.html \
 		test_plugin_clipping.xhtml \

layout/generic/test/test_invalidate_during_plugin_paint.html

@@ -0,0 +1,54 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test handling plugins invalidating during paint</title>
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>    
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <style>
+  embed { width:200px; height:200px; display:block; }
+  </style>
+</head>
+<body>
+<p id="display"></p>
+<div id="content" style="display: block">
+  <embed id="p1" type="application/x-test" drawmode="solid" color="80808080"></embed>
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+var p1 = document.getElementById("p1");
+var initialPaintCount;
+
+function checkEnded() {
+  var paints = p1.getPaintCount() - initialPaintCount;
+  if (paints > 20) {
+    ok(true, "Got " + paints + " paints");
+    SimpleTest.finish();
+    return;
+  }
+
+  setTimeout(checkEnded, 30);  
+}
+
+function doTest() {
+  initialPaintCount = p1.getPaintCount();
+
+  // Tell the plugin to invalidate every time it paints
+  p1.setInvalidateDuringPaint(true);
+  // Trigger an invalidation to get painting started
+  p1.setColor("FFFFFFFF");
+  // Now we should have an infinite cycle of painting and invalidations.
+
+  // Poll for more than 20 paints to happen.
+  checkEnded();
+}
+
+// Need to run 'doTest' after painting is unsuppressed, or we'll set clip
+// regions to empty.
+addLoadEvent(doTest);
+SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+</body>
+</html>