Bug 1440177 - Part 6: Don't dereference display items during AddPaintedDisplayItem for the LAYER_NONE case. r?jnicol draft
authorMatt Woodrow <mwoodrow@mozilla.com>
Fri, 02 Mar 2018 11:19:17 +1300
changeset 762225 f119175e79ca494d12ec5ba49e20b12c9e03ada4
parent 762224 5190523dd6cf20ccae2d6621204e23e8d23c3d4c
child 762226 5a59913b34bc4855876a8ec9ac058446d6711a5f
push id101106
push usermwoodrow@mozilla.com
push dateThu, 01 Mar 2018 22:19:48 +0000
reviewersjnicol
bugs1440177
milestone60.0a1
Bug 1440177 - Part 6: Don't dereference display items during AddPaintedDisplayItem for the LAYER_NONE case. r?jnicol The display items are almost certainly gone from the cache at this point, so dereferencing them can take a while. This moves the DisplayItemData lookup, and the IsReused/HasMergedFrames checks into the ProcessDisplayItems pass (where we already use the items), and then just uses the AssignedDisplayItem entry for everything. MozReview-Commit-ID: 8udcE0bmyF3
layout/painting/FrameLayerBuilder.cpp
layout/painting/FrameLayerBuilder.h
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -192,33 +192,48 @@ DisplayItemData::EndUpdate(nsAutoPtr<nsD
   mItem = nullptr;
   EndUpdate();
 }
 
 void
 DisplayItemData::BeginUpdate(Layer* aLayer, LayerState aState,
                              nsDisplayItem* aItem /* = nullptr */)
 {
+  BeginUpdate(aLayer, aState, aItem,
+              aItem ? aItem->IsReused() : false,
+              aItem ? aItem->HasMergedFrames() : false);
+}
+
+void
+DisplayItemData::BeginUpdate(Layer* aLayer, LayerState aState,
+                             nsDisplayItem* aItem, bool aIsReused,
+                             bool aIsMerged)
+{
   MOZ_RELEASE_ASSERT(mLayer);
   MOZ_RELEASE_ASSERT(aLayer);
   mLayer = aLayer;
   mOptLayer = nullptr;
   mInactiveManager = nullptr;
   mLayerState = aState;
   mUsed = true;
 
   if (aLayer->AsPaintedLayer()) {
     mItem = aItem;
-    mReusedItem = aItem->IsReused();
+    mReusedItem = aIsReused;
   }
 
   if (!aItem) {
     return;
   }
 
+  if (!aIsMerged && mFrameList.Length() == 1) {
+    MOZ_ASSERT(mFrameList[0] == aItem->Frame());
+    return;
+  }
+
   // We avoid adding or removing element unnecessarily
   // since we have to modify userdata each time
   AutoTArray<nsIFrame*, 4> copy(mFrameList);
   if (!copy.RemoveElement(aItem->Frame())) {
     AddFrame(aItem->Frame());
     mChangedFrameInvalidations.Or(mChangedFrameInvalidations,
                                   aItem->Frame()->GetVisualOverflowRect());
   }
@@ -3222,25 +3237,20 @@ void ContainerState::FinishPaintedLayerD
                  "Layer already in list???");
     mNewChildLayers[data->mNewChildLayersIndex].mLayer = paintedLayer.forget();
   }
 
   for (auto& item : data->mAssignedDisplayItems) {
     MOZ_ASSERT(item.mItem->GetType() != DisplayItemType::TYPE_LAYER_EVENT_REGIONS);
     MOZ_ASSERT(item.mItem->GetType() != DisplayItemType::TYPE_COMPOSITOR_HITTEST_INFO);
 
-    DisplayItemData* oldData =
-      mLayerBuilder->GetOldLayerForFrame(item.mItem->Frame(),
-                                         item.mItem->GetPerFrameKey(),
-                                         item.mItem->HasMergedFrames() ? nullptr : item.mItem->GetDisplayItemData());
-    InvalidateForLayerChange(item.mItem, data->mLayer, oldData);
-    mLayerBuilder->AddPaintedDisplayItem(data, item.mItem, item.mClip,
-                                         *this, item.mLayerState,
-                                         data->mAnimatedGeometryRootOffset,
-                                         oldData, item);
+    InvalidateForLayerChange(item.mItem, data->mLayer, item.mDisplayItemData);
+    mLayerBuilder->AddPaintedDisplayItem(data, item, *this,
+                                         data->mAnimatedGeometryRootOffset);
+    item.mDisplayItemData = nullptr;
   }
 
   PaintedDisplayItemLayerUserData* userData = GetPaintedDisplayItemLayerUserData(data->mLayer);
   NS_ASSERTION(userData, "where did our user data go?");
   userData->mLastItemCount = data->mAssignedDisplayItems.Length();
 
   NewLayerEntry* newLayerEntry = &mNewChildLayers[data->mNewChildLayersIndex];
 
@@ -3530,17 +3540,21 @@ PaintedLayerData::Accumulate(ContainerSt
     // Note that the transform (if any) on the PaintedLayer is always an integer translation so
     // we don't have to factor that in here.
     aItem->DisableComponentAlpha();
   }
 
   bool clipMatches = mItemClip == aClip;
   mItemClip = aClip;
 
-  mAssignedDisplayItems.AppendElement(AssignedDisplayItem(aItem, aClip, aLayerState));
+  DisplayItemData* oldData =
+      aState->mLayerBuilder->GetOldLayerForFrame(aItem->Frame(),
+                                         aItem->GetPerFrameKey(),
+                                         aItem->HasMergedFrames() ? nullptr : aItem->GetDisplayItemData());
+  mAssignedDisplayItems.AppendElement(AssignedDisplayItem(aItem, aClip, aLayerState, oldData));
 
   if (aItem->MustPaintOnContentSide()) {
      mShouldPaintOnContentSide = true;
   }
 
   if (!mIsSolidColorInVisibleRegion && mOpaqueRegion.Contains(aVisibleRect) &&
       mVisibleRegion.Contains(aVisibleRect) && !mImage) {
     // A very common case! Most pages have a PaintedLayer with the page
@@ -4638,19 +4652,21 @@ ContainerState::ProcessDisplayItems(nsDi
         newLayerEntry->mBaseScrollMetadata =
           static_cast<nsDisplaySubDocument*>(item)->ComputeScrollMetadata(ownLayer->Manager(), mParameters);
       }
 
       /**
        * No need to allocate geometry for items that aren't
        * part of a PaintedLayer.
        */
-      oldData =
-        mLayerBuilder->GetOldLayerForFrame(item->Frame(), item->GetPerFrameKey());
-      mLayerBuilder->AddLayerDisplayItem(ownLayer, item, layerState, nullptr, oldData);
+      if (ownLayer->Manager() == mLayerBuilder->GetRetainingLayerManager()) {
+        oldData =
+          mLayerBuilder->GetOldLayerForFrame(item->Frame(), item->GetPerFrameKey());
+        mLayerBuilder->StoreDataForFrame(item, ownLayer, layerState, oldData);
+      }
     } else {
       const bool backfaceHidden = item->In3DContextAndBackfaceIsHidden();
       const nsIFrame* referenceFrame = item->ReferenceFrame();
 
       PaintedLayerData* paintedLayerData =
         mPaintedLayerDataTree.FindPaintedLayerFor(animatedGeometryRoot, itemASR, layerClipChain,
                                                   itemVisibleRect, backfaceHidden,
                                                   [&](PaintedLayerData* aData) {
@@ -4831,146 +4847,152 @@ FrameLayerBuilder::ComputeGeometryChange
         layerData->mTranslation);
   }
 
   aData->EndUpdate(geometry);
 }
 
 void
 FrameLayerBuilder::AddPaintedDisplayItem(PaintedLayerData* aLayerData,
-                                        nsDisplayItem* aItem,
-                                        const DisplayItemClip& aClip,
-                                        ContainerState& aContainerState,
-                                        LayerState aLayerState,
-                                        const nsPoint& aTopLeft,
-                                        DisplayItemData* aData,
-                                        AssignedDisplayItem& aAssignedDisplayItem)
+                                         AssignedDisplayItem& aItem,
+                                         ContainerState& aContainerState,
+                                         const nsPoint& aTopLeft)
 {
   PaintedLayer* layer = aLayerData->mLayer;
   PaintedDisplayItemLayerUserData* paintedData =
     static_cast<PaintedDisplayItemLayerUserData*>
       (layer->GetUserData(&gPaintedDisplayItemLayerUserData));
   RefPtr<BasicLayerManager> tempManager;
   nsIntRect intClip;
   bool hasClip = false;
-  if (aLayerState != LAYER_NONE) {
-    if (aData) {
-      tempManager = aData->mInactiveManager;
-
-      // We need to grab these before calling AddLayerDisplayItem because it will overwrite them.
+  if (aItem.mLayerState != LAYER_NONE) {
+    if (aItem.mDisplayItemData) {
+      tempManager = aItem.mDisplayItemData->mInactiveManager;
+
+      // We need to grab these before updating the DisplayItemData because it will overwrite them.
       nsRegion clip;
-      if (aClip.ComputeRegionInClips(&aData->GetClip(),
+      if (aItem.mClip.ComputeRegionInClips(&aItem.mDisplayItemData->GetClip(),
                                      aTopLeft - paintedData->mLastAnimatedGeometryRootOrigin,
                                      &clip)) {
         intClip = clip.GetBounds().ScaleToOutsidePixels(paintedData->mXScale,
                                                         paintedData->mYScale,
                                                         paintedData->mAppUnitsPerDevPixel);
       }
     }
     if (!tempManager) {
       tempManager = new BasicLayerManager(BasicLayerManager::BLM_INACTIVE);
     }
   }
 
-  AddLayerDisplayItem(layer, aItem, aLayerState, tempManager, aData);
+  if (layer->Manager() == mRetainingManager) {
+    DisplayItemData *data = aItem.mDisplayItemData;
+    if (data) {
+      if (!data->mUsed) {
+        data->BeginUpdate(layer, aItem.mLayerState, aItem.mItem, aItem.mReused, aItem.mMerged);
+      }
+    } else {
+      data = StoreDataForFrame(aItem.mItem, layer, aItem.mLayerState, nullptr);
+    }
+    data->mInactiveManager = tempManager;
+  }
 
   if (tempManager) {
-    FLB_LOG_PAINTED_LAYER_DECISION(aLayerData, "Creating nested FLB for item %p\n", aItem);
+    FLB_LOG_PAINTED_LAYER_DECISION(aLayerData, "Creating nested FLB for item %p\n", aItem.mItem);
     FrameLayerBuilder* layerBuilder = new FrameLayerBuilder();
     layerBuilder->Init(mDisplayListBuilder, tempManager, aLayerData, true,
-                       &aClip);
+                       &aItem.mClip);
 
     tempManager->BeginTransaction();
     if (mRetainingManager) {
       layerBuilder->DidBeginRetainedLayerTransaction(tempManager);
     }
 
     UniquePtr<LayerProperties> props(LayerProperties::CloneFrom(tempManager->GetRoot()));
     RefPtr<Layer> tmpLayer =
-      aItem->BuildLayer(mDisplayListBuilder, tempManager, ContainerLayerParameters());
+      aItem.mItem->BuildLayer(mDisplayListBuilder, tempManager, ContainerLayerParameters());
     // We have no easy way of detecting if this transaction will ever actually get finished.
     // For now, I've just silenced the warning with nested transactions in BasicLayers.cpp
     if (!tmpLayer) {
       tempManager->EndTransaction(nullptr, nullptr);
       tempManager->SetUserData(&gLayerManagerLayerBuilder, nullptr);
-      aAssignedDisplayItem.mItem = nullptr;
+      aItem.mItem = nullptr;
       return;
     }
 
     bool snap;
     nsRect visibleRect =
-      aItem->GetVisibleRect().Intersect(aItem->GetBounds(mDisplayListBuilder, &snap));
+      aItem.mItem->GetVisibleRect().Intersect(aItem.mItem->GetBounds(mDisplayListBuilder, &snap));
     nsIntRegion rgn = visibleRect.ToOutsidePixels(paintedData->mAppUnitsPerDevPixel);
 
     // Convert the visible rect to a region and give the item
     // a chance to try restrict it further.
-    nsRegion tightBounds = aItem->GetTightBounds(mDisplayListBuilder, &snap);
+    nsRegion tightBounds = aItem.mItem->GetTightBounds(mDisplayListBuilder, &snap);
     if (!tightBounds.IsEmpty()) {
       rgn.AndWith(tightBounds.ToOutsidePixels(paintedData->mAppUnitsPerDevPixel));
     }
     SetOuterVisibleRegion(tmpLayer, &rgn);
 
     // If BuildLayer didn't call BuildContainerLayerFor, then our new layer won't have been
     // stored in layerBuilder. Manually add it now.
     if (mRetainingManager) {
 #ifdef DEBUG_DISPLAY_ITEM_DATA
       LayerManagerData* parentLmd = static_cast<LayerManagerData*>
         (layer->Manager()->GetUserData(&gLayerManagerUserData));
       LayerManagerData* lmd = static_cast<LayerManagerData*>
         (tempManager->GetUserData(&gLayerManagerUserData));
       lmd->mParent = parentLmd;
 #endif
-      DisplayItemData* data = layerBuilder->GetDisplayItemDataForManager(aItem, tempManager);
-      layerBuilder->StoreDataForFrame(aItem, tmpLayer, LAYER_ACTIVE, data);
+      DisplayItemData* data = layerBuilder->GetDisplayItemDataForManager(aItem.mItem, tempManager);
+      layerBuilder->StoreDataForFrame(aItem.mItem, tmpLayer, LAYER_ACTIVE, data);
     }
 
     tempManager->SetRoot(tmpLayer);
     layerBuilder->WillEndTransaction();
     tempManager->AbortTransaction();
 
 #ifdef MOZ_DUMP_PAINTING
     if (gfxUtils::DumpDisplayList() || gfxEnv::DumpPaint()) {
-      fprintf_stderr(gfxUtils::sDumpPaintFile, "Basic layer tree for painting contents of display item %s(%p):\n", aItem->Name(), aItem->Frame());
+      fprintf_stderr(gfxUtils::sDumpPaintFile, "Basic layer tree for painting contents of display item %s(%p):\n", aItem.mItem->Name(), aItem.mItem->Frame());
       std::stringstream stream;
       tempManager->Dump(stream, "", gfxEnv::DumpPaintToFile());
       fprint_stderr(gfxUtils::sDumpPaintFile, stream);  // not a typo, fprint_stderr declared in LayersLogging.h
     }
 #endif
 
     nsIntPoint offset = GetLastPaintOffset(layer) - GetTranslationForPaintedLayer(layer);
     props->MoveBy(-offset);
     // Effective transforms are needed by ComputeDifferences().
     tmpLayer->ComputeEffectiveTransforms(Matrix4x4());
     nsIntRegion invalid;
     if (!props->ComputeDifferences(tmpLayer, invalid, nullptr)) {
-      nsRect visible = aItem->Frame()->GetVisualOverflowRect();
+      nsRect visible = aItem.mItem->Frame()->GetVisualOverflowRect();
       invalid = visible.ToOutsidePixels(paintedData->mAppUnitsPerDevPixel);
     }
-    if (aLayerState == LAYER_SVG_EFFECTS) {
-      invalid = nsSVGIntegrationUtils::AdjustInvalidAreaForSVGEffects(aItem->Frame(),
-                                                                      aItem->ToReferenceFrame(),
+    if (aItem.mLayerState == LAYER_SVG_EFFECTS) {
+      invalid = nsSVGIntegrationUtils::AdjustInvalidAreaForSVGEffects(aItem.mItem->Frame(),
+                                                                      aItem.mItem->ToReferenceFrame(),
                                                                       invalid);
     }
     if (!invalid.IsEmpty()) {
 #ifdef MOZ_DUMP_PAINTING
       if (nsLayoutUtils::InvalidationDebuggingIsEnabled()) {
-        printf_stderr("Inactive LayerManager(%p) for display item %s(%p) has an invalid region - invalidating layer %p\n", tempManager.get(), aItem->Name(), aItem->Frame(), layer);
+        printf_stderr("Inactive LayerManager(%p) for display item %s(%p) has an invalid region - invalidating layer %p\n", tempManager.get(), aItem.mItem->Name(), aItem.mItem->Frame(), layer);
       }
 #endif
       invalid.ScaleRoundOut(paintedData->mXScale, paintedData->mYScale);
 
       if (hasClip) {
         invalid.And(invalid, intClip);
       }
 
       InvalidatePostTransformRegion(layer, invalid,
                                     GetTranslationForPaintedLayer(layer));
     }
   }
-  aAssignedDisplayItem.mInactiveLayerManager = tempManager;
+  aItem.mInactiveLayerManager = tempManager;
 }
 
 DisplayItemData*
 FrameLayerBuilder::StoreDataForFrame(nsDisplayItem* aItem, Layer* aLayer,
                                      LayerState aState, DisplayItemData* aData)
 {
   if (aData) {
     if (!aData->mUsed) {
@@ -5013,37 +5035,35 @@ FrameLayerBuilder::StoreDataForFrame(nsI
   RefPtr<DisplayItemData> data =
     new (aFrame->PresContext()) DisplayItemData(lmd, aDisplayItemKey, aLayer, aFrame);
 
   data->BeginUpdate(aLayer, aState);
 
   lmd->mDisplayItems.PutEntry(data);
 }
 
+AssignedDisplayItem::AssignedDisplayItem(nsDisplayItem* aItem,
+                                         const DisplayItemClip& aClip,
+                                         LayerState aLayerState,
+                                         DisplayItemData* aData)
+  : mItem(aItem)
+  , mClip(aClip)
+  , mLayerState(aLayerState)
+  , mDisplayItemData(aData)
+  , mReused(aItem->IsReused())
+  , mMerged(aItem->HasMergedFrames())
+{}
+
 AssignedDisplayItem::~AssignedDisplayItem()
 {
   if (mInactiveLayerManager) {
     mInactiveLayerManager->SetUserData(&gLayerManagerLayerBuilder, nullptr);
   }
 }
 
-void
-FrameLayerBuilder::AddLayerDisplayItem(Layer* aLayer,
-                                       nsDisplayItem* aItem,
-                                       LayerState aLayerState,
-                                       BasicLayerManager* aManager,
-                                       DisplayItemData* aData)
-{
-  if (aLayer->Manager() != mRetainingManager)
-    return;
-
-  DisplayItemData *data = StoreDataForFrame(aItem, aLayer, aLayerState, aData);
-  data->mInactiveManager = aManager;
-}
-
 nsIntPoint
 FrameLayerBuilder::GetLastPaintOffset(PaintedLayer* aLayer)
 {
   PaintedDisplayItemLayerUserData* layerData = GetPaintedDisplayItemLayerUserData(aLayer);
   MOZ_ASSERT(layerData);
   if (layerData->mHasExplicitLastPaintOffset) {
     return layerData->mLastPaintOffset;
   }
--- a/layout/painting/FrameLayerBuilder.h
+++ b/layout/painting/FrameLayerBuilder.h
@@ -151,16 +151,18 @@ private:
     * object.
     * Set the passed in parameters, and clears the opt layer and inactive manager.
     * Parent, and display item key are assumed to be the same.
     *
     * EndUpdate must be called before the end of the transaction to complete the update.
     */
   void BeginUpdate(layers::Layer* aLayer, LayerState aState,
                    nsDisplayItem* aItem = nullptr);
+  void BeginUpdate(layers::Layer* aLayer, LayerState aState,
+                   nsDisplayItem* aItem, bool aIsReused, bool aIsMerged);
 
   /**
     * Completes the update of this, and removes any references to data that won't live
     * longer than the transaction.
     *
     * Updates the geometry, frame list and clip.
     * For items within a PaintedLayer, a geometry object must be specified to retain
     * until the next transaction.
@@ -207,34 +209,34 @@ public:
   nsRegion mRegion;
   bool mIsInfinite;
 };
 
 struct AssignedDisplayItem
 {
   AssignedDisplayItem(nsDisplayItem* aItem,
                       const DisplayItemClip& aClip,
-                      LayerState aLayerState)
-    : mItem(aItem)
-    , mClip(aClip)
-    , mLayerState(aLayerState)
-  {}
-
+                      LayerState aLayerState,
+                      DisplayItemData* aData);
   ~AssignedDisplayItem();
 
   nsDisplayItem* mItem;
   DisplayItemClip mClip;
   LayerState mLayerState;
+  DisplayItemData* mDisplayItemData;
 
   /**
    * If the display item is being rendered as an inactive
    * layer, then this stores the layer manager being
    * used for the inactive transaction.
    */
   RefPtr<layers::LayerManager> mInactiveLayerManager;
+
+  bool mReused;
+  bool mMerged;
 };
 
 
 struct ContainerLayerParameters {
   ContainerLayerParameters()
     : mXScale(1)
     , mYScale(1)
     , mLayerContentsVisibleRect(nullptr)
@@ -497,45 +499,26 @@ public:
 
 
   /******* 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.
-   *
-   * @param aLayer Layer that the display item will be rendered into
-   * @param aItem Display item to be drawn.
-   * @param aLayerState What LayerState the item is using.
-   * @param aManager If the layer is in the LAYER_INACTIVE state,
-   * then this is the temporary layer manager to draw with.
-   */
-  void AddLayerDisplayItem(Layer* aLayer,
-                           nsDisplayItem* aItem,
-                           LayerState aLayerState,
-                           BasicLayerManager* aManager,
-                           DisplayItemData* aData);
-
-  /**
    * Record aItem as a display item that is rendered by the PaintedLayer
    * aLayer, with aClipRect, where aContainerLayerFrame is the frame
    * for the container layer this ThebesItem belongs to.
    * aItem must have an underlying frame.
    * @param aTopLeft offset from active scrolled root to reference frame
    */
   void AddPaintedDisplayItem(PaintedLayerData* aLayer,
-                            nsDisplayItem* aItem,
-                            const DisplayItemClip& aClip,
-                            ContainerState& aContainerState,
-                            LayerState aLayerState,
-                            const nsPoint& aTopLeft,
-                            DisplayItemData* aData,
-                            AssignedDisplayItem& aAssignedDisplayItem);
+                             AssignedDisplayItem& aAssignedDisplayItem,
+                             ContainerState& aContainerState,
+                             const nsPoint& aTopLeft);
 
   /**
    * 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,
                         nsDisplayItemGeometry** aOldGeometry = nullptr,
@@ -622,31 +605,30 @@ 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 PaintedLayer
    * that renders many display items.
    */
   DisplayItemData* GetOldLayerForFrame(nsIFrame* aFrame, uint32_t aDisplayItemKey, DisplayItemData* aOldData = nullptr);
 
-protected:
-
-  friend class LayerManagerData;
-
   /**
    * Stores DisplayItemData associated with aFrame, stores the data in
    * mNewDisplayItemData.
    */
   DisplayItemData* StoreDataForFrame(nsDisplayItem* aItem, Layer* aLayer,
                                      LayerState aState, DisplayItemData* aData);
   void StoreDataForFrame(nsIFrame* aFrame,
                          uint32_t aDisplayItemKey,
                          Layer* aLayer,
                          LayerState aState);
 
+protected:
+  friend class LayerManagerData;
+
   // Flash the area within the context clip if paint flashing is enabled.
   static void FlashPaint(gfxContext *aContext);
 
   /*
    * Get the DisplayItemData array associated with this frame, or null if one
    * doesn't exist.
    *
    * Note that the pointer returned here is only valid so long as you don't