Bug 772079 - Fix ThebesLayerInvalidRegion being destroyed too soon. r=roc

A comment in ApplyThebesLayerInvalidation says that it preserves the content
of ThebesLayerInvalidRegion, in case there are multiple container layers for
the same frame. SetHasContainerLayer, however, immediately clears said property.

This was causing invalidations to be lost since Bug 758620 on fixed-position
elements, as they were being separated out onto their own layers but were still
merged in the root scroll layer. This is tracked in Bug 769541.

This fixes the problem by storing the new invalid region in DisplayItemDataEntry
and clearing/setting the ThebesLayerInvalidRegion property in the
UpdateDisplayItemData callback from FrameLayerBuilder::WillEndTransaction.

layout/base/FrameLayerBuilder.cpp

@@ -72,11 +72,6 @@ static inline MaskLayerImageCache* GetMaskLayerImageCache()
   return gMaskLayerImageCache;
 }
 
-class RefCountedRegion : public RefCounted<RefCountedRegion> {
-public:
-  nsRegion mRegion;
-};
-
 static void DestroyRefCountedRegion(void* aPropertyValue)
 {
   static_cast<RefCountedRegion*>(aPropertyValue)->Release();
@@ -721,8 +716,7 @@ FrameLayerBuilder::WillEndTransaction(LayerManager* aManager)
  * region property. Otherwise set it to the frame's region property.
  */
 static void
-SetHasContainerLayer(nsIFrame* aFrame, nsPoint aOffsetToRoot,
-                     RefCountedRegion** aThebesLayerInvalidRegion)
+SetHasContainerLayer(nsIFrame* aFrame, nsPoint aOffsetToRoot)
 {
   aFrame->AddStateBits(NS_FRAME_HAS_CONTAINER_LAYER);
   for (nsIFrame* f = aFrame;
@@ -739,24 +733,6 @@ SetHasContainerLayer(nsIFrame* aFrame, nsPoint aOffsetToRoot,
   } else {
     props.Set(ThebesLayerLastPaintOffsetProperty(), new nsPoint(aOffsetToRoot));
   }
-
-  // Reset or create the invalid region now so we can start collecting
-  // new dirty areas.
-  if (*aThebesLayerInvalidRegion) {
-    (*aThebesLayerInvalidRegion)->AddRef();
-    props.Set(ThebesLayerInvalidRegionProperty(), *aThebesLayerInvalidRegion);
-  } else {
-    RefCountedRegion* invalidRegion = static_cast<RefCountedRegion*>
-      (props.Get(ThebesLayerInvalidRegionProperty()));
-    if (invalidRegion) {
-      invalidRegion->mRegion.SetEmpty();
-    } else {
-      invalidRegion = new RefCountedRegion();
-      invalidRegion->AddRef();
-      props.Set(ThebesLayerInvalidRegionProperty(), invalidRegion);
-    }
-    *aThebesLayerInvalidRegion = invalidRegion;
-  }
 }
 
 static void
@@ -768,6 +744,21 @@ SetNoContainerLayer(nsIFrame* aFrame)
   aFrame->RemoveStateBits(NS_FRAME_HAS_CONTAINER_LAYER);
 }
 
+/* static */ void
+FrameLayerBuilder::SetAndClearInvalidRegion(DisplayItemDataEntry* aEntry)
+{
+  if (aEntry->mInvalidRegion) {
+    nsIFrame* f = aEntry->GetKey();
+    FrameProperties props = f->Properties();
+
+    RefCountedRegion* invalidRegion;
+    aEntry->mInvalidRegion.forget(&invalidRegion);
+
+    invalidRegion->mRegion.SetEmpty();
+    props.Set(ThebesLayerInvalidRegionProperty(), invalidRegion);
+  }
+}
+
 /* static */ PLDHashOperator
 FrameLayerBuilder::UpdateDisplayItemDataForFrame(DisplayItemDataEntry* aEntry,
                                                  void* aUserArg)
@@ -790,8 +781,12 @@ FrameLayerBuilder::UpdateDisplayItemDataForFrame(DisplayItemDataEntry* aEntry,
     SetNoContainerLayer(f);
   }
 
-  // Steal the list of display item layers
+  // Steal the list of display item layers and invalid region
   aEntry->mData.SwapElements(newDisplayItems->mData);
+  aEntry->mInvalidRegion.swap(newDisplayItems->mInvalidRegion);
+  // Clear and reset the invalid region now so we can start collecting new
+  // dirty areas.
+  SetAndClearInvalidRegion(aEntry);
   // Don't need to process this frame again
   builder->mNewDisplayItemData.RawRemoveEntry(newDisplayItems);
   return PL_DHASH_NEXT;
@@ -804,6 +799,11 @@ FrameLayerBuilder::StoreNewDisplayItemData(DisplayItemDataEntry* aEntry,
   LayerManagerData* data = static_cast<LayerManagerData*>(aUserArg);
   nsIFrame* f = aEntry->GetKey();
   FrameProperties props = f->Properties();
+
+  // Clear and reset the invalid region now so we can start collecting new
+  // dirty areas.
+  SetAndClearInvalidRegion(aEntry);
+
   // Remember that this frame has display items in retained layers
   NS_ASSERTION(!data->mFramesWithLayers.GetEntry(f),
                "We shouldn't get here if we're already in mFramesWithLayers");
@@ -2137,16 +2137,23 @@ FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder* aBuilder,
                        scaleParameters);
 
   if (aManager == mRetainingManager) {
+    FrameProperties props = aContainerFrame->Properties();
+    RefCountedRegion* thebesLayerInvalidRegion = nsnull;
     DisplayItemDataEntry* entry = mNewDisplayItemData.PutEntry(aContainerFrame);
     if (entry) {
       entry->mData.AppendElement(
           DisplayItemData(containerLayer, containerDisplayItemKey, LAYER_ACTIVE));
+      thebesLayerInvalidRegion = static_cast<RefCountedRegion*>
+        (props.Get(ThebesLayerInvalidRegionProperty()));
+      if (!thebesLayerInvalidRegion) {
+        thebesLayerInvalidRegion = new RefCountedRegion();
+      }
+      entry->mInvalidRegion = thebesLayerInvalidRegion;
     }
     nsPoint currentOffset;
     ApplyThebesLayerInvalidation(aBuilder, aContainerFrame, aContainerItem, state,
                                  &currentOffset);
-    RefCountedRegion* thebesLayerInvalidRegion = nsnull;
-    SetHasContainerLayer(aContainerFrame, currentOffset, &thebesLayerInvalidRegion);
+    SetHasContainerLayer(aContainerFrame, currentOffset);
 
     nsAutoTArray<nsIFrame*,4> mergedFrames;
     if (aContainerItem) {
@@ -2159,10 +2166,15 @@ FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder* aBuilder,
         // Ensure that UpdateDisplayItemDataForFrame recognizes that we
         // still have a container layer associated with this frame.
         entry->mIsSharingContainerLayer = true;
+
+        // Store the invalid region property in case this frame is represented
+        // by multiple container layers. This is cleared and set when iterating
+        // over the DisplayItemDataEntry's in WillEndTransaction.
+        entry->mInvalidRegion = thebesLayerInvalidRegion;
       }
       ApplyThebesLayerInvalidation(aBuilder, mergedFrame, nsnull, state,
                                    &currentOffset);
-      SetHasContainerLayer(mergedFrame, currentOffset, &thebesLayerInvalidRegion);
+      SetHasContainerLayer(mergedFrame, currentOffset);
     }
   }
 

layout/base/FrameLayerBuilder.h

@@ -32,6 +32,11 @@ enum LayerState {
   LAYER_ACTIVE_EMPTY
 };
 
+class RefCountedRegion : public RefCounted<RefCountedRegion> {
+public:
+  nsRegion mRegion;
+};
+
 /**
  * The FrameLayerBuilder belongs to an nsDisplayListBuilder and is
  * responsible for converting display lists into layer trees.
@@ -444,13 +449,15 @@ protected:
       nsPtrHashKey<nsIFrame>(toCopy.mKey), mIsSharingContainerLayer(toCopy.mIsSharingContainerLayer)
     {
       // This isn't actually a copy-constructor; notice that it steals toCopy's
-      // array.  Be careful.
+      // array and invalid region.  Be careful.
       mData.SwapElements(toCopy.mData);
+      mInvalidRegion.swap(toCopy.mInvalidRegion);
     }
 
     bool HasNonEmptyContainerLayer();
 
     nsAutoTArray<DisplayItemData, 1> mData;
+    nsRefPtr<RefCountedRegion> mInvalidRegion;
     bool mIsSharingContainerLayer;
 
     enum { ALLOW_MEMMOVE = false };
@@ -547,6 +554,7 @@ public:
 protected:
   void RemoveThebesItemsForLayerSubtree(Layer* aLayer);
 
+  static void SetAndClearInvalidRegion(DisplayItemDataEntry* aEntry);
   static PLDHashOperator UpdateDisplayItemDataForFrame(DisplayItemDataEntry* aEntry,
                                                        void* aUserArg);
   static PLDHashOperator StoreNewDisplayItemData(DisplayItemDataEntry* aEntry,

layout/base/nsDisplayItemTypes.h

@@ -34,6 +34,7 @@ enum Type {
   TYPE_COMBOBOX_FOCUS,
   TYPE_EVENT_RECEIVER,
   TYPE_FIELDSET_BORDER_BACKGROUND,
+  TYPE_FIXED_POSITION,
   TYPE_FORCEPAINTONSCROLL,
   TYPE_FRAMESET_BORDER,
   TYPE_FRAMESET_BLANK,

layout/base/nsDisplayList.h

@@ -1925,7 +1925,7 @@ public:
   virtual already_AddRefed<Layer> BuildLayer(nsDisplayListBuilder* aBuilder,
                                              LayerManager* aManager,
                                              const ContainerParameters& aContainerParameters);
-  NS_DISPLAY_DECL_NAME("FixedPosition", TYPE_OWN_LAYER)
+  NS_DISPLAY_DECL_NAME("FixedPosition", TYPE_FIXED_POSITION)
 };
 
 /**