Bug 1426649: Stop tracking DOM changes from painting. r?dbaron draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 21 Dec 2017 14:09:32 +0100
changeset 714419 d9c69be3477904634b9b72eabce3eedc8054b778
parent 714416 fe1794e607ccd549362967dae6f4ff67eb988586
child 714420 65c05bddb218e6e8860a80ff3a9e2b88d0107e10
push id93917
push userbmo:emilio@crisal.io
push dateFri, 22 Dec 2017 22:53:45 +0000
reviewersdbaron
bugs1426649
milestone59.0a1
Bug 1426649: Stop tracking DOM changes from painting. r?dbaron I'm pretty sure this is not a problem now, since we don't mutate the DOM from painting, and we don't have legacy extensions anymore. Just to confirm, I did a try run with a RELEASE_ASSERT(!CheckDOMModified()), and it passed. MozReview-Commit-ID: HTekD8tsz9v
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsPresContext.cpp
layout/base/nsPresContext.h
layout/painting/FrameLayerBuilder.cpp
layout/painting/FrameLayerBuilder.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -8827,22 +8827,16 @@ nsCSSFrameConstructor::CharacterDataChan
   }
 }
 
 void
 nsCSSFrameConstructor::BeginUpdate() {
   NS_ASSERTION(!nsContentUtils::IsSafeToRunScript(),
                "Someone forgot a script blocker");
 
-  nsRootPresContext* rootPresContext =
-    mPresShell->GetPresContext()->GetRootPresContext();
-  if (rootPresContext) {
-    rootPresContext->IncrementDOMGeneration();
-  }
-
 #ifdef DEBUG
   ++mUpdateCount;
 #endif
 }
 
 void
 nsCSSFrameConstructor::EndUpdate()
 {
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -3171,18 +3171,17 @@ nsPresContext::RebuildFontFeatureValues(
     if (NS_SUCCEEDED(rv)) {
       mPostedFlushFontFeatureValues = true;
     }
   }
 }
 
 nsRootPresContext::nsRootPresContext(nsIDocument* aDocument,
                                      nsPresContextType aType)
-  : nsPresContext(aDocument, aType),
-    mDOMGeneration(0)
+  : nsPresContext(aDocument, aType)
 {
 }
 
 nsRootPresContext::~nsRootPresContext()
 {
   NS_ASSERTION(mRegisteredPlugins.Count() == 0,
                "All plugins should have been unregistered");
   CancelAllDidPaintTimers();
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -1606,28 +1606,16 @@ public:
    * Transfer stored plugin geometry updates to the compositor. Called during
    * reflow, data is shipped over with layer updates. e10s specific.
    */
   void CollectPluginGeometryUpdates(mozilla::layers::LayerManager* aLayerManager);
 
   virtual bool IsRoot() override { return true; }
 
   /**
-   * Increment DOM-modification generation counter to indicate that
-   * the DOM has changed in a way that might lead to style changes/
-   * reflows/frame creation and destruction.
-   */
-  void IncrementDOMGeneration() { mDOMGeneration++; }
-
-  /**
-   * Get the current DOM generation counter.
-   */
-  uint32_t GetDOMGeneration() { return mDOMGeneration; }
-
-  /**
    * Add a runnable that will get called before the next paint. They will get
    * run eventually even if painting doesn't happen. They might run well before
    * painting happens.
    */
   void AddWillPaintObserver(nsIRunnable* aRunnable);
 
   /**
    * Run all runnables that need to get called before the next paint.
@@ -1670,17 +1658,16 @@ protected:
     nsCOMPtr<nsITimer> mTimer;
   };
   AutoTArray<NotifyDidPaintTimer, 4> mNotifyDidPaintTimers;
 
   nsCOMPtr<nsITimer> mApplyPluginGeometryTimer;
   nsTHashtable<nsRefPtrHashKey<nsIContent> > mRegisteredPlugins;
   nsTArray<nsCOMPtr<nsIRunnable> > mWillPaintObservers;
   nsRevocableEventPtr<RunWillPaintObservers> mWillPaintFallbackEvent;
-  uint32_t mDOMGeneration;
 };
 
 #ifdef MOZ_REFLOW_PERF
 
 #define DO_GLOBAL_REFLOW_COUNT(_name) \
   aPresContext->CountReflows((_name), (nsIFrame*)this);
 #else
 #define DO_GLOBAL_REFLOW_COUNT(_name)
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -116,17 +116,16 @@ static inline MaskLayerImageCache* GetMa
 
   return gMaskLayerImageCache;
 }
 
 FrameLayerBuilder::FrameLayerBuilder()
   : mRetainingManager(nullptr)
   , mContainingPaintedLayer(nullptr)
   , mInactiveLayerClip(nullptr)
-  , mDetectedDOMModification(false)
   , mInvalidateAllLayers(false)
   , mInLayerTreeCompressionMode(false)
   , mIsInactiveLayerManager(false)
   , mContainerLayerGeneration(0)
   , mMaxContainerLayerGeneration(0)
 {
   MOZ_COUNT_CTOR(FrameLayerBuilder);
 }
@@ -1786,19 +1785,16 @@ FrameLayerBuilder::Shutdown()
 void
 FrameLayerBuilder::Init(nsDisplayListBuilder* aBuilder, LayerManager* aManager,
                         PaintedLayerData* aLayerData,
                         bool aIsInactiveLayerManager,
                         const DisplayItemClip* aInactiveLayerClip)
 {
   mDisplayListBuilder = aBuilder;
   mRootPresContext = aBuilder->RootReferenceFrame()->PresContext()->GetRootPresContext();
-  if (mRootPresContext) {
-    mInitialDOMGeneration = mRootPresContext->GetDOMGeneration();
-  }
   mContainingPaintedLayer = aLayerData;
   mIsInactiveLayerManager = aIsInactiveLayerManager;
   mInactiveLayerClip = aInactiveLayerClip;
   aManager->SetUserData(&gLayerManagerLayerBuilder, this);
 }
 
 void
 FrameLayerBuilder::FlashPaint(gfxContext *aContext)
@@ -6027,19 +6023,16 @@ FrameLayerBuilder::PaintItems(nsTArray<C
       if (gfxEnv::DumpPaintItems()) {
         DebugPaintItem(aDrawTarget, aPresContext, cdi->mItem, aBuilder);
       } else
 #endif
       {
         cdi->mItem->Paint(aBuilder, aContext);
       }
     }
-
-    if (CheckDOMModified())
-      break;
   }
 
   if (currentClipIsSetInContext) {
     aContext->Restore();
   }
 }
 
 /**
@@ -6110,19 +6103,16 @@ FrameLayerBuilder::DrawPaintedLayer(Pain
   AUTO_PROFILER_LABEL("FrameLayerBuilder::DrawPaintedLayer", GRAPHICS);
 
   nsDisplayListBuilder* builder = static_cast<nsDisplayListBuilder*>
     (aCallbackData);
 
   FrameLayerBuilder *layerBuilder = aLayer->Manager()->GetLayerBuilder();
   NS_ASSERTION(layerBuilder, "Unexpectedly null layer builder!");
 
-  if (layerBuilder->CheckDOMModified())
-    return;
-
   PaintedLayerItemsEntry* entry = layerBuilder->mPaintedLayerItems.GetEntry(aLayer);
   NS_ASSERTION(entry, "We shouldn't be drawing into a layer with no items!");
   if (!entry->mContainerLayerFrame) {
     return;
   }
 
 
   PaintedDisplayItemLayerUserData* userData =
@@ -6228,34 +6218,16 @@ FrameLayerBuilder::DrawPaintedLayer(Pain
     }
   }
 
   if (!aRegionToInvalidate.IsEmpty()) {
     aLayer->AddInvalidRect(aRegionToInvalidate.GetBounds());
   }
 }
 
-bool
-FrameLayerBuilder::CheckDOMModified()
-{
-  if (!mRootPresContext ||
-      mInitialDOMGeneration == mRootPresContext->GetDOMGeneration())
-    return false;
-  if (mDetectedDOMModification) {
-    // Don't spam the console with extra warnings
-    return true;
-  }
-  mDetectedDOMModification = true;
-  // Painting is not going to complete properly. There's not much
-  // we can do here though. Invalidating the window to get another repaint
-  // is likely to lead to an infinite repaint loop.
-  NS_WARNING("Detected DOM modification during paint, bailing out!");
-  return true;
-}
-
 /* static */ void
 FrameLayerBuilder::DumpRetainedLayerTree(LayerManager* aManager, std::stringstream& aStream, bool aDumpHtml)
 {
   aManager->Dump(aStream, "", aDumpHtml);
 }
 
 nsDisplayItemGeometry*
 FrameLayerBuilder::GetMostRecentGeometry(nsDisplayItem* aItem)
--- a/layout/painting/FrameLayerBuilder.h
+++ b/layout/painting/FrameLayerBuilder.h
@@ -741,23 +741,16 @@ public:
    */
   void SetLayerTreeCompressionMode() { mInLayerTreeCompressionMode = true; }
   bool CheckInLayerTreeCompressionMode();
 
   void ComputeGeometryChangeForItem(DisplayItemData* aData);
 
 protected:
   /**
-   * Returns true if the DOM has been modified since we started painting,
-   * in which case we should bail out and not paint anymore. This should
-   * never happen, but plugins can trigger it in some cases.
-   */
-  bool CheckDOMModified();
-
-  /**
    * The layer manager belonging to the widget that is being retained
    * across paints.
    */
   LayerManager*                       mRetainingManager;
   /**
    * The root prescontext for the display list builder reference frame
    */
   RefPtr<nsRootPresContext>         mRootPresContext;
@@ -780,25 +773,16 @@ protected:
 
   /**
    * When building layers for an inactive layer, this stores the clip
    * of the display item that built the inactive layer.
    */
   const DisplayItemClip*              mInactiveLayerClip;
 
   /**
-   * Saved generation counter so we can detect DOM changes.
-   */
-  uint32_t                            mInitialDOMGeneration;
-  /**
-   * Set to true if we have detected and reported DOM modification during
-   * the current paint.
-   */
-  bool                                mDetectedDOMModification;
-  /**
    * Indicates that the entire layer tree should be rerendered
    * during this paint.
    */
   bool                                mInvalidateAllLayers;
 
   bool                                mInLayerTreeCompressionMode;
 
   bool                                mIsInactiveLayerManager;