Back out bug 785333 (revisions 79345542f853 and a1756976e61d) to fix crasher bug 785626
authorGavin Sharp <gavin@gavinsharp.com>
Mon, 27 Aug 2012 16:41:19 -0700
changeset 105601 ee5484eb9f4a8449f6d53abe9cfe4331e481c3bf
parent 105600 8b18a47000f82d4acbe778b28d9a7bf390e66364
child 105643 8af2ff9c601856448aa399f743fbfec8e1a9334d
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
bugs785333, 785626
milestone18.0a1
Back out bug 785333 (revisions 79345542f853 and a1756976e61d) to fix crasher bug 785626
layout/base/FrameLayerBuilder.cpp
layout/base/FrameLayerBuilder.h
layout/base/nsDisplayList.cpp
--- a/layout/base/FrameLayerBuilder.cpp
+++ b/layout/base/FrameLayerBuilder.cpp
@@ -877,17 +877,17 @@ FrameLayerBuilder::HasRetainedLayerFor(n
         return true;
       }
     }
   }
   return false;
 }
 
 Layer*
-FrameLayerBuilder::GetOldLayerForFrame(nsIFrame* aFrame, uint32_t aDisplayItemKey)
+FrameLayerBuilder::GetOldLayerFor(nsIFrame* aFrame, uint32_t aDisplayItemKey)
 {
   // If we need to build a new layer tree, then just refuse to recycle
   // anything.
   if (!mRetainingManager || mInvalidateAllLayers)
     return nullptr;
 
   nsTArray<DisplayItemData> *array = GetDisplayItemDataArrayForFrame(aFrame);
   if (!array)
@@ -898,41 +898,16 @@ FrameLayerBuilder::GetOldLayerForFrame(n
       Layer* layer = array->ElementAt(i).mLayer;
       if (layer->Manager() == mRetainingManager)
         return layer;
     }
   }
   return nullptr;
 }
 
-Layer*
-FrameLayerBuilder::GetOldLayerFor(nsDisplayItem* aItem)
-{
-  uint32_t key = aItem->GetPerFrameKey();
-  nsIFrame* frame = aItem->GetUnderlyingFrame();
-
-  if (frame) {
-    Layer* oldLayer = GetOldLayerForFrame(frame, key);
-    if (oldLayer) {
-      return oldLayer;
-    }
-  }
-
-  nsAutoTArray<nsIFrame*,4> mergedFrames;
-  aItem->GetMergedFrames(&mergedFrames);
-  for (uint32_t i = 0; i < mergedFrames.Length(); ++i) {
-    Layer* oldLayer = GetOldLayerForFrame(mergedFrames[i], key);
-    if (oldLayer) {
-      return oldLayer;
-    }
-  }
-
-  return nullptr;
-}
-
 /* static */ Layer*
 FrameLayerBuilder::GetDebugOldLayerFor(nsIFrame* aFrame, uint32_t aDisplayItemKey)
 {
   FrameProperties props = aFrame->Properties();
   LayerManagerData* data = static_cast<LayerManagerData*>(props.Get(LayerManagerDataProperty()));
   if (!data) {
     return nullptr;
   }
@@ -1882,19 +1857,21 @@ ContainerState::ProcessDisplayItems(cons
       data->UpdateCommonClipCount(aClip);
     }
   }
 }
 
 void
 ContainerState::InvalidateForLayerChange(nsDisplayItem* aItem, Layer* aNewLayer)
 {
-  NS_ASSERTION(aItem->GetUnderlyingFrame(), "Display items that render using Thebes must have a frame");
-  NS_ASSERTION(aItem->GetPerFrameKey(), "Display items that render using Thebes must have a key");
-  Layer* oldLayer = mLayerBuilder->GetOldLayerFor(aItem);
+  nsIFrame* f = aItem->GetUnderlyingFrame();
+  NS_ASSERTION(f, "Display items that render using Thebes must have a frame");
+  uint32_t key = aItem->GetPerFrameKey();
+  NS_ASSERTION(key, "Display items that render using Thebes must have a key");
+  Layer* oldLayer = mLayerBuilder->GetOldLayerFor(f, key);
   if (!oldLayer) {
     // Nothing to do here, this item didn't have a layer before
     return;
   }
   if (aNewLayer != oldLayer) {
     // The item has changed layers.
     // Invalidate the bounds in the old layer and new layer.
     // The bounds might have changed, but we assume that any difference
@@ -1957,44 +1934,28 @@ FrameLayerBuilder::AddThebesDisplayItem(
     ClippedDisplayItem* cdi =
       entry->mItems.AppendElement(ClippedDisplayItem(aItem, aClip,
                                                      mContainerLayerGeneration));
     cdi->mInactiveLayer = aLayerState != LAYER_NONE;
   }
 }
 
 void
-FrameLayerBuilder::AddLayerDisplayItemForFrame(Layer* aLayer,
-                                               nsIFrame* aFrame,
-                                               PRUint32 aDisplayItemKey,
-                                               LayerState aLayerState)
-{
-  DisplayItemDataEntry* entry = mNewDisplayItemData.PutEntry(aFrame);
-  if (entry) {
-    entry->mContainerLayerGeneration = mContainerLayerGeneration;
-    entry->mData.AppendElement(DisplayItemData(aLayer, aDisplayItemKey, aLayerState, mContainerLayerGeneration));
-  }
-}
-
-void
 FrameLayerBuilder::AddLayerDisplayItem(Layer* aLayer,
                                        nsDisplayItem* aItem,
                                        LayerState aLayerState)
 {
   if (aLayer->Manager() != mRetainingManager)
     return;
 
   nsIFrame* f = aItem->GetUnderlyingFrame();
-  PRUint32 key = aItem->GetPerFrameKey();
-  AddLayerDisplayItemForFrame(aLayer, f, key, aLayerState);
-
-  nsAutoTArray<nsIFrame*,4> mergedFrames;
-  aItem->GetMergedFrames(&mergedFrames);
-  for (PRUint32 i = 0; i < mergedFrames.Length(); ++i) {
-    AddLayerDisplayItemForFrame(aLayer, mergedFrames[i], key, aLayerState);
+  DisplayItemDataEntry* entry = mNewDisplayItemData.PutEntry(f);
+  entry->mContainerLayerGeneration = mContainerLayerGeneration;
+  if (entry) {
+    entry->mData.AppendElement(DisplayItemData(aLayer, aItem->GetPerFrameKey(), aLayerState, mContainerLayerGeneration));
   }
 }
 
 nsIntPoint
 FrameLayerBuilder::GetLastPaintOffset(ThebesLayer* aLayer)
 {
   ThebesLayerItemsEntry* entry = mThebesLayerItems.PutEntry(aLayer);
   if (entry) {
@@ -2319,24 +2280,17 @@ FrameLayerBuilder::BuildContainerLayerFo
     aContainerItem ? aContainerItem->GetPerFrameKey() : 0;
   NS_ASSERTION(aContainerFrame, "Container display items here should have a frame");
   NS_ASSERTION(!aContainerItem ||
                aContainerItem->GetUnderlyingFrame() == aContainerFrame,
                "Container display item must match given frame");
 
   nsRefPtr<ContainerLayer> containerLayer;
   if (aManager == mRetainingManager) {
-    // Using GetOldLayerFor will search merged frames, as well as the underlying
-    // frame. The underlying frame can change when a page scrolls, so this
-    // avoids layer recreation in the situation that a new underlying frame is
-    // picked for a layer.
-    Layer* oldLayer = aContainerItem ?
-      GetOldLayerFor(aContainerItem) :
-      GetOldLayerForFrame(aContainerFrame, containerDisplayItemKey);
-
+    Layer* oldLayer = GetOldLayerFor(aContainerFrame, containerDisplayItemKey);
     if (oldLayer) {
       NS_ASSERTION(oldLayer->Manager() == aManager, "Wrong manager");
       if (oldLayer->HasUserData(&gThebesDisplayItemLayerUserData)) {
         // The old layer for this item is actually our ThebesLayer
         // because we rendered its layer into that ThebesLayer. So we
         // don't actually have a retained container layer.
       } else {
         NS_ASSERTION(oldLayer->GetType() == Layer::TYPE_CONTAINER,
@@ -2420,23 +2374,16 @@ FrameLayerBuilder::BuildContainerLayerFo
       nsAutoTArray<nsIFrame*,4> mergedFrames;
       if (aContainerItem) {
         aContainerItem->GetMergedFrames(&mergedFrames);
       }
       for (uint32_t i = 0; i < mergedFrames.Length(); ++i) {
         nsIFrame* mergedFrame = mergedFrames[i];
         DisplayItemDataEntry* entry = mNewDisplayItemData.PutEntry(mergedFrame);
         if (entry) {
-          // Append the container layer so we don't regenerate layers when
-          // the underlying frame of an item changes to one of the existing
-          // merged frames.
-          entry->mData.AppendElement(
-              DisplayItemData(containerLayer, containerDisplayItemKey,
-                              LAYER_ACTIVE, mContainerLayerGeneration));
-
           // 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;
@@ -2500,17 +2447,17 @@ FrameLayerBuilder::GetLeafLayerFor(nsDis
                                    LayerManager* aManager,
                                    nsDisplayItem* aItem)
 {
   if (aManager != mRetainingManager)
     return nullptr;
 
   nsIFrame* f = aItem->GetUnderlyingFrame();
   NS_ASSERTION(f, "Can only call GetLeafLayerFor on items that have a frame");
-  Layer* layer = GetOldLayerForFrame(f, aItem->GetPerFrameKey());
+  Layer* layer = GetOldLayerFor(f, aItem->GetPerFrameKey());
   if (!layer)
     return nullptr;
   if (layer->HasUserData(&gThebesDisplayItemLayerUserData)) {
     // This layer was created to render Thebes-rendered content for this
     // display item. The display item should not use it for its own
     // layer rendering.
     return nullptr;
   }
--- a/layout/base/FrameLayerBuilder.h
+++ b/layout/base/FrameLayerBuilder.h
@@ -263,33 +263,25 @@ public:
    */
   static void DumpRetainedLayerTree(LayerManager* aManager, FILE* aFile = stdout, bool aDumpHtml = false);
 #endif
 
   /******* PRIVATE METHODS to FrameLayerBuilder.cpp ********/
   /* These are only in the public section because they need
    * to be called by file-scope helper functions in FrameLayerBuilder.cpp.
    */
-
+  
   /**
    * Record aItem as a display item that is rendered by aLayer.
    */
   void AddLayerDisplayItem(Layer* aLayer,
                            nsDisplayItem* aItem,
                            LayerState aLayerState);
 
   /**
-   * Record aFrame as a frame that is rendered by an item on aLayer.
-   */
-  void AddLayerDisplayItemForFrame(Layer* aLayer,
-                                   nsIFrame* aFrame,
-                                   PRUint32 aDisplayItemKey,
-                                   LayerState aLayerState);
-
-  /**
    * Record aItem as a display item that is rendered by the ThebesLayer
    * aLayer, with aClipRect, where aContainerLayerFrame is the frame
    * for the container layer this ThebesItem belongs to.
    * aItem must have an underlying frame.
    */
   struct Clip;
   void AddThebesDisplayItem(ThebesLayer* aLayer,
                             nsDisplayItem* aItem,
@@ -299,24 +291,17 @@ public:
 
   /**
    * Given a frame and a display item key that uniquely identifies a
    * display item for the frame, find the layer that was last used to
    * render that display item. Returns null if there is no such layer.
    * This could be a dedicated layer for the display item, or a ThebesLayer
    * that renders many display items.
    */
-  Layer* GetOldLayerForFrame(nsIFrame* aFrame, uint32_t aDisplayItemKey);
-
-  /**
-   * Calls GetOldLayerForFrame on the underlying frame of the display item,
-   * and each subsequent merged frame if no layer is found for the underlying
-   * frame.
-   */
-  Layer* GetOldLayerFor(nsDisplayItem* aItem);
+  Layer* GetOldLayerFor(nsIFrame* aFrame, uint32_t aDisplayItemKey);
 
   static Layer* GetDebugOldLayerFor(nsIFrame* aFrame, uint32_t aDisplayItemKey);
   /**
    * Try to determine whether the ThebesLayer aLayer paints an opaque
    * single color everywhere it's visible in aRect.
    * If successful, return that color, otherwise return NS_RGBA(0,0,0,0).
    */
   nscolor FindOpaqueColorCovering(nsDisplayListBuilder* aBuilder,
--- a/layout/base/nsDisplayList.cpp
+++ b/layout/base/nsDisplayList.cpp
@@ -2687,16 +2687,24 @@ nsDisplayScrollLayer::TryMerge(nsDisplay
   if (other->mScrolledFrame != this->mScrolledFrame) {
     return false;
   }
 
   FrameProperties props = mScrolledFrame->Properties();
   props.Set(nsIFrame::ScrollLayerCount(),
     reinterpret_cast<void*>(GetScrollLayerCount() - 1));
 
+  // Swap frames with the other item before doing MergeFrom.
+  // XXX - This ensures that the frame associated with a scroll layer after
+  // merging is the first, rather than the last. This tends to change less,
+  // ensuring we're more likely to retain the associated gfx layer.
+  // See Bug 729534 and Bug 731641.
+  nsIFrame* tmp = mFrame;
+  mFrame = other->mFrame;
+  other->mFrame = tmp;
   MergeFromTrackingMergedFrames(other);
   return true;
 }
 
 bool
 nsDisplayScrollLayer::ShouldFlattenAway(nsDisplayListBuilder* aBuilder)
 {
   return GetScrollLayerCount() > 1;