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
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -173,26 +173,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)
{
@@ -298,17 +297,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),
@@ -2582,52 +2580,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 =
@@ -2669,32 +2672,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(
@@ -2729,62 +2728,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
@@ -1276,16 +1276,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]
@@ -1343,20 +1349,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;
@@ -1481,18 +1483,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;