Bug 781516 - Fix invalidation caused by unprocessed merged frames. r=roc
authorChris Lord <chrislord.net@gmail.com>
Thu, 23 Aug 2012 09:30:53 +0100
changeset 105189 330c2cc06bbd902a8079ed587151183071cc6879
parent 105188 06f6eede6b774c09122233171093bdac54c6ad39
child 105190 4b228bcfa794137f041604bd47ab7359c621b92a
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewersroc
bugs781516
milestone17.0a1
Bug 781516 - Fix invalidation caused by unprocessed merged frames. r=roc As clip items aren't processed, they have no associated layer entries. This is a problem when a clip item's underlying frame is the same as one that gets merged into a container layer, as display-item data will be created, but no layer entry will be added. This will cause it to be removed on the next layer-build, and cause a full invalidation. Fix this by adding an 'mIsMergedFrame' entry to DisplayItemDataEntry and setting it on all merged frames in BuildContainerLayerFor. This property stops the entry from being removed when it gets updated.
layout/base/FrameLayerBuilder.cpp
layout/base/FrameLayerBuilder.h
--- a/layout/base/FrameLayerBuilder.cpp
+++ b/layout/base/FrameLayerBuilder.cpp
@@ -804,17 +804,18 @@ FrameLayerBuilder::SetAndClearInvalidReg
 FrameLayerBuilder::UpdateDisplayItemDataForFrame(DisplayItemDataEntry* aEntry,
                                                  void* aUserArg)
 {
   FrameLayerBuilder* builder = static_cast<FrameLayerBuilder*>(aUserArg);
   nsIFrame* f = aEntry->GetKey();
   FrameProperties props = f->Properties();
   DisplayItemDataEntry* newDisplayItems =
     builder ? builder->mNewDisplayItemData.GetEntry(f) : nullptr;
-  if (!newDisplayItems || newDisplayItems->mData.IsEmpty()) {
+  if (!newDisplayItems || (newDisplayItems->mData.IsEmpty() &&
+                           !newDisplayItems->mIsMergedFrame)) {
     // This frame was visible, but isn't anymore.
     if (newDisplayItems) {
       builder->mNewDisplayItemData.RawRemoveEntry(newDisplayItems);
     }
     bool found;
     props.Remove(LayerManagerDataProperty(), &found);
     NS_ASSERTION(found, "How can the frame property be missing?");
     SetNoContainerLayer(f);
@@ -823,16 +824,17 @@ FrameLayerBuilder::UpdateDisplayItemData
 
   if (!newDisplayItems->HasNonEmptyContainerLayer()) {
     SetNoContainerLayer(f);
   }
 
   // Steal the list of display item layers and invalid region
   aEntry->mData.SwapElements(newDisplayItems->mData);
   aEntry->mInvalidRegion.swap(newDisplayItems->mInvalidRegion);
+  aEntry->mIsMergedFrame = newDisplayItems->mIsMergedFrame;
   // 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;
 }
 
@@ -851,16 +853,17 @@ FrameLayerBuilder::StoreNewDisplayItemDa
   // 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");
   DisplayItemDataEntry *newEntry = data->mFramesWithLayers.PutEntry(f);
   NS_ASSERTION(!props.Get(LayerManagerDataProperty()),
                "mFramesWithLayers out of sync");
 
   newEntry->mData.SwapElements(aEntry->mData);
+  newEntry->mIsMergedFrame = aEntry->mIsMergedFrame;
   props.Set(LayerManagerDataProperty(), data);
   return PL_DHASH_REMOVE;
 }
 
 bool
 FrameLayerBuilder::HasRetainedLayerFor(nsIFrame* aFrame, uint32_t aDisplayItemKey)
 {
   nsTArray<DisplayItemData> *array = GetDisplayItemDataArrayForFrame(aFrame);
@@ -2370,16 +2373,20 @@ FrameLayerBuilder::BuildContainerLayerFo
           // 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;
+
+          // Mark that this is a merged frame so that we retain the display
+          // item data when updating.
+          entry->mIsMergedFrame = true;
         }
         ApplyThebesLayerInvalidation(aBuilder, mergedFrame, nullptr, state,
                                      &currentOffset, transformItem);
         SetHasContainerLayer(mergedFrame, currentOffset);
       }
     }
 
     Clip clip;
--- a/layout/base/FrameLayerBuilder.h
+++ b/layout/base/FrameLayerBuilder.h
@@ -461,33 +461,40 @@ protected:
 
   /**
    * We accumulate DisplayItemData elements in a hashtable during
    * the paint process, and store them in the frame property only when
    * paint is complete. This is the hashentry for that hashtable.
    */
   class DisplayItemDataEntry : public nsPtrHashKey<nsIFrame> {
   public:
-    DisplayItemDataEntry(const nsIFrame *key) : nsPtrHashKey<nsIFrame>(key), mIsSharingContainerLayer(false) {}
-    DisplayItemDataEntry(DisplayItemDataEntry &toCopy) :
-      nsPtrHashKey<nsIFrame>(toCopy.mKey), mIsSharingContainerLayer(toCopy.mIsSharingContainerLayer)
+    DisplayItemDataEntry(const nsIFrame *key)
+      : nsPtrHashKey<nsIFrame>(key)
+      , mIsSharingContainerLayer(false)
+      , mIsMergedFrame(false)
+      {}
+    DisplayItemDataEntry(DisplayItemDataEntry &toCopy)
+      : nsPtrHashKey<nsIFrame>(toCopy.mKey)
+      , mIsSharingContainerLayer(toCopy.mIsSharingContainerLayer)
     {
       // This isn't actually a copy-constructor; notice that it steals toCopy's
       // array and invalid region.  Be careful.
       mData.SwapElements(toCopy.mData);
       mInvalidRegion.swap(toCopy.mInvalidRegion);
       mContainerLayerGeneration = toCopy.mContainerLayerGeneration;
+      mIsMergedFrame = toCopy.mIsMergedFrame;
     }
 
     bool HasNonEmptyContainerLayer();
 
     nsAutoTArray<DisplayItemData, 1> mData;
     nsRefPtr<RefCountedRegion> mInvalidRegion;
     uint32_t mContainerLayerGeneration;
     bool mIsSharingContainerLayer;
+    bool mIsMergedFrame;
 
     enum { ALLOW_MEMMOVE = false };
   };
 
   // LayerManagerData needs to see DisplayItemDataEntry.
   friend class LayerManagerData;
 
   // Flash the area within the context clip if paint flashing is enabled.