Bug 1429932 - Part 1: Remove mFireAfterPaintEvents and use mTransactions instead. r=tnikkel
☠☠ backed out by d32017c991aa ☠ ☠
authorMatt Woodrow <mwoodrow@mozilla.com>
Fri, 12 Jan 2018 11:41:16 +1300
changeset 748699 65b8996eb50c9930ceedcd9edb5eab6fca08f77e
parent 748698 2e9497af37293bd656016afa5474e7656adc1dff
child 748700 bb1394cba514d54d2585331acc143349b8440eb6
push id97228
push usersfraser@mozilla.com
push dateTue, 30 Jan 2018 10:21:04 +0000
reviewerstnikkel
bugs1429932
milestone60.0a1
Bug 1429932 - Part 1: Remove mFireAfterPaintEvents and use mTransactions instead. r=tnikkel This fixes a bug where EnsureEventualDidPaintEvent needs to be called separately for each transaction id, but we skip it since mFireAfterPaintEvents is still true from the previous paint. We now track the equivalent state by checking for the presence of mTransactions[aTransactionId], and correctly schedule an eventual didpaint for each id. MozReview-Commit-ID: JnRTycGEyom
layout/base/nsPresContext.cpp
layout/base/nsPresContext.h
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -174,26 +174,25 @@ nsPresContext::MakeColorPref(const nsStr
   }
 
   return result;
 }
 
 bool
 nsPresContext::IsDOMPaintEventPending()
 {
-  if (mFireAfterPaintEvents) {
+  if (!mTransactions.IsEmpty()) {
     return true;
   }
   nsRootPresContext* drpc = GetRootPresContext();
   if (drpc && drpc->mRefreshDriver->ViewManagerFlushIsPending()) {
     // Since we're promising that there will be a MozAfterPaint event
     // fired, we record an empty invalidation in case display list
     // invalidation doesn't invalidate anything further.
     NotifyInvalidation(drpc->mRefreshDriver->LastTransactionId() + 1, nsRect(0, 0, 0, 0));
-    NS_ASSERTION(mFireAfterPaintEvents, "Why aren't we planning to fire the event?");
     return true;
   }
   return false;
 }
 
 void
 nsPresContext::PrefChangedCallback(const char* aPrefName, void* instance_data)
 {
@@ -299,17 +298,16 @@ nsPresContext::nsPresContext(nsIDocument
     mUsesExChUnits(false),
     mPendingViewportChange(false),
     mCounterStylesDirty(true),
     mPostedFlushCounterStyles(false),
     mFontFeatureValuesDirty(true),
     mPostedFlushFontFeatureValues(false),
     mSuppressResizeReflow(false),
     mIsVisual(false),
-    mFireAfterPaintEvents(false),
     mIsChrome(false),
     mIsChromeOriginImage(false),
     mPaintFlashing(false),
     mPaintFlashingInitialized(false),
     mHasWarnedAboutPositionedTableParts(false),
     mHasWarnedAboutTooLargeDashedOrDottedRadius(false),
     mQuirkSheetAdded(false),
     mNeedsPrefUpdate(false),
@@ -2588,52 +2586,57 @@ nsPresContext::NotifyInvalidation(uint64
 
   nsRect rect(DevPixelsToAppUnits(clampedRect.x),
               DevPixelsToAppUnits(clampedRect.y),
               DevPixelsToAppUnits(clampedRect.width),
               DevPixelsToAppUnits(clampedRect.height));
   NotifyInvalidation(aTransactionId, rect);
 }
 
+nsPresContext::TransactionInvalidations*
+nsPresContext::GetInvalidations(uint64_t aTransactionId)
+{
+  for (TransactionInvalidations& t : mTransactions) {
+    if (t.mTransactionId == aTransactionId) {
+      return &t;
+    }
+  }
+  return nullptr;
+}
+
 void
 nsPresContext::NotifyInvalidation(uint64_t aTransactionId, const nsRect& aRect)
 {
   MOZ_ASSERT(GetContainerWeak(), "Invalidation in detached pres context");
 
   // If there is no paint event listener, then we don't need to fire
   // the asynchronous event. We don't even need to record invalidation.
   // MayHavePaintEventListener is pretty cheap and we could make it
   // even cheaper by providing a more efficient
   // nsPIDOMWindow::GetListenerManager.
 
   nsPresContext* pc;
   for (pc = this; pc; pc = pc->GetParentPresContext()) {
-    if (pc->mFireAfterPaintEvents)
+    TransactionInvalidations* transaction = pc->GetInvalidations(aTransactionId);
+    if (transaction) {
       break;
-    pc->mFireAfterPaintEvents = true;
+    } else {
+      transaction = pc->mTransactions.AppendElement();
+      transaction->mTransactionId = aTransactionId;
+    }
   }
   if (!pc) {
     nsRootPresContext* rpc = GetRootPresContext();
     if (rpc) {
       rpc->EnsureEventualDidPaintEvent(aTransactionId);
     }
   }
 
-  TransactionInvalidations* transaction = nullptr;
-  for (TransactionInvalidations& t : mTransactions) {
-    if (t.mTransactionId == aTransactionId) {
-      transaction = &t;
-      break;
-    }
-  }
-  if (!transaction) {
-    transaction = mTransactions.AppendElement();
-    transaction->mTransactionId = aTransactionId;
-  }
-
+  TransactionInvalidations* transaction = GetInvalidations(aTransactionId);
+  MOZ_ASSERT(transaction);
   transaction->mInvalidations.AppendElement(aRect);
 }
 
 /* static */ void
 nsPresContext::NotifySubDocInvalidation(ContainerLayer* aContainer,
                                         const nsIntRegion* aRegion)
 {
   ContainerLayerPresContext *data =
@@ -2675,32 +2678,28 @@ nsPresContext::SetNotifySubDocInvalidati
 nsPresContext::ClearNotifySubDocInvalidationData(ContainerLayer* aContainer)
 {
   aContainer->SetUserData(&gNotifySubDocInvalidationData, nullptr);
 }
 
 struct NotifyDidPaintSubdocumentCallbackClosure {
   uint64_t mTransactionId;
   const mozilla::TimeStamp& mTimeStamp;
-  bool mNeedsAnotherDidPaintNotification;
 };
 /* static */ bool
 nsPresContext::NotifyDidPaintSubdocumentCallback(nsIDocument* aDocument, void* aData)
 {
   NotifyDidPaintSubdocumentCallbackClosure* closure =
     static_cast<NotifyDidPaintSubdocumentCallbackClosure*>(aData);
   nsIPresShell* shell = aDocument->GetShell();
   if (shell) {
     nsPresContext* pc = shell->GetPresContext();
     if (pc) {
       pc->NotifyDidPaintForSubtree(closure->mTransactionId,
                                    closure->mTimeStamp);
-      if (pc->mFireAfterPaintEvents) {
-        closure->mNeedsAnotherDidPaintNotification = true;
-      }
     }
   }
   return true;
 }
 
 class DelayedFireDOMPaintEvent : public Runnable {
 public:
   DelayedFireDOMPaintEvent(
@@ -2735,62 +2734,58 @@ public:
 
 void
 nsPresContext::NotifyDidPaintForSubtree(uint64_t aTransactionId,
                                         const mozilla::TimeStamp& aTimeStamp)
 {
   if (IsRoot()) {
     static_cast<nsRootPresContext*>(this)->CancelDidPaintTimers(aTransactionId);
 
-    if (!mFireAfterPaintEvents) {
+    if (mTransactions.IsEmpty()) {
       return;
     }
   }
 
-  if (!PresShell()->IsVisible() && !mFireAfterPaintEvents) {
+  if (!PresShell()->IsVisible() && mTransactions.IsEmpty()) {
     return;
   }
 
   // Non-root prescontexts fire MozAfterPaint to all their descendants
   // unconditionally, even if no invalidations have been collected. This is
   // because we don't want to eat the cost of collecting invalidations for
   // every subdocument (which would require putting every subdocument in its
   // own layer).
 
   bool sent = false;
   uint32_t i = 0;
   while (i < mTransactions.Length()) {
     if (mTransactions[i].mTransactionId <= aTransactionId) {
-      nsCOMPtr<nsIRunnable> ev =
-        new DelayedFireDOMPaintEvent(this, &mTransactions[i].mInvalidations,
-                                     mTransactions[i].mTransactionId, aTimeStamp);
-      nsContentUtils::AddScriptRunner(ev);
-      sent = true;
+      if (!mTransactions[i].mInvalidations.IsEmpty()) {
+        nsCOMPtr<nsIRunnable> ev =
+          new DelayedFireDOMPaintEvent(this, &mTransactions[i].mInvalidations,
+                                       mTransactions[i].mTransactionId, aTimeStamp);
+        nsContentUtils::AddScriptRunner(ev);
+        sent = true;
+      }
       mTransactions.RemoveElementAt(i);
     } else {
       i++;
     }
   }
 
   if (!sent) {
     nsTArray<nsRect> dummy;
     nsCOMPtr<nsIRunnable> ev =
       new DelayedFireDOMPaintEvent(this, &dummy,
                                    aTransactionId, aTimeStamp);
     nsContentUtils::AddScriptRunner(ev);
   }
 
-  NotifyDidPaintSubdocumentCallbackClosure closure = { aTransactionId, aTimeStamp, false };
+  NotifyDidPaintSubdocumentCallbackClosure closure = { aTransactionId, aTimeStamp };
   mDocument->EnumerateSubDocuments(nsPresContext::NotifyDidPaintSubdocumentCallback, &closure);
-
-  if (!closure.mNeedsAnotherDidPaintNotification &&
-      mTransactions.IsEmpty()) {
-    // Nothing more to do for the moment.
-    mFireAfterPaintEvents = false;
-  }
 }
 
 bool
 nsPresContext::HasCachedStyleData()
 {
   if (!mShell) {
     return false;
   }
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -1281,16 +1281,22 @@ protected:
   bool HasCachedStyleData();
 
   // Creates a one-shot timer with the given aCallback & aDelay.
   // Returns a refcounted pointer to the timer (or nullptr on failure).
   already_AddRefed<nsITimer> CreateTimer(nsTimerCallbackFunc aCallback,
                                          const char* aName,
                                          uint32_t aDelay);
 
+  struct TransactionInvalidations {
+    uint64_t mTransactionId;
+    nsTArray<nsRect> mInvalidations;
+  };
+  TransactionInvalidations* GetInvalidations(uint64_t aTransactionId);
+
   // IMPORTANT: The ownership implicit in the following member variables
   // has been explicitly checked.  If you add any members to this class,
   // please make the ownership explicit (pinkerton, scc).
 
   nsPresContextType     mType;
   // the nsPresShell owns a strong reference to the nsPresContext, and is responsible
   // for nulling this pointer before it is destroyed
   nsIPresShell* MOZ_NON_OWNING_REF mShell;         // [WEAK]
@@ -1349,20 +1355,16 @@ protected:
 
   nsCOMPtr<nsITheme> mTheme;
   nsLanguageAtomService* mLangService;
   nsCOMPtr<nsIPrintSettings> mPrintSettings;
   nsCOMPtr<nsITimer>    mPrefChangedTimer;
 
   mozilla::UniquePtr<nsBidi> mBidiEngine;
 
-  struct TransactionInvalidations {
-    uint64_t mTransactionId;
-    nsTArray<nsRect> mInvalidations;
-  };
   AutoTArray<TransactionInvalidations, 4> mTransactions;
 
   // text performance metrics
   nsAutoPtr<gfxTextPerfMetrics>   mTextPerf;
 
   nsAutoPtr<gfxMissingFontRecorder> mMissingFonts;
 
   nsRect                mVisibleArea;
@@ -1487,18 +1489,16 @@ protected:
   unsigned              mPostedFlushFontFeatureValues: 1;
 
   // resize reflow is suppressed when the only change has been to zoom
   // the document rather than to change the document's dimensions
   unsigned              mSuppressResizeReflow : 1;
 
   unsigned              mIsVisual : 1;
 
-  unsigned              mFireAfterPaintEvents : 1;
-
   unsigned              mIsChrome : 1;
   unsigned              mIsChromeOriginImage : 1;
 
   // Should we paint flash in this context? Do not use this variable directly.
   // Use GetPaintFlashing() method instead.
   mutable unsigned mPaintFlashing : 1;
   mutable unsigned mPaintFlashingInitialized : 1;