# HG changeset patch # User Emilio Cobos Alvarez # Date 1517393556 -3600 # Node ID 6d21bdf7eab331e98bac5d4fa25faf6f41af647f # Parent 4289e3ea78859099eae1b595376582e945fdeb66 Bug 1434474: There's no need to rebuild font / counter styles / font feature values off a runnable. r=bholley Everything that needs them up-to-date will call flush appropriately, there should be no need to do it manually. This way we coalesce all the stylist updates until the next style flush in the best case, or until one of the consumers actually needs them. MozReview-Commit-ID: BVsxXxhtcKL diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1451,17 +1451,16 @@ nsIDocument::nsIDocument() mBFCacheDisallowed(false), mHasHadDefaultView(false), mStyleSheetChangeEventsEnabled(false), mIsSrcdocDocument(false), mDidDocumentOpen(false), mHasDisplayDocument(false), mFontFaceSetDirty(true), mGetUserFontSetCalled(false), - mPostedFlushUserFontSet(false), mDidFireDOMContentLoaded(true), mHasScrollLinkedEffect(false), mFrameRequestCallbacksScheduled(false), mIsTopLevelContentDocument(false), mIsContentDocument(false), mDidCallBeginLoad(false), mBufferingCSPViolations(false), mAllowPaymentRequest(false), @@ -3954,18 +3953,21 @@ nsDocument::CreateShell(nsPresContext* a MOZ_LOG(gDocumentLeakPRLog, LogLevel::Debug, ("DOCUMENT %p with PressShell %p and DocShell %p", this, shell.get(), docShell.get())); mExternalResourceMap.ShowViewers(); UpdateFrameRequestCallbackSchedulingState(); - // Now that we have a shell, we might have @font-face rules. - RebuildUserFontSet(); + // Now that we have a shell, we might have @font-face rules (the presence of a + // shell may change which rules apply to us). We don't need to do anything + // like EnsureStyleFlush or such, there's nothing to update yet and when stuff + // is ready to update we'll flush the font set. + MarkUserFontSetDirty(); return shell.forget(); } void nsIDocument::UpdateFrameRequestCallbackSchedulingState(nsIPresShell* aOldShell) { // If the condition for shouldBeScheduled changes to depend on some other @@ -4051,18 +4053,20 @@ nsDocument::DeleteShell() } // When our shell goes away, request that all our images be immediately // discarded, so we don't carry around decoded image data for a document we // no longer intend to paint. ImageTracker()->RequestDiscardAll(); // Now that we no longer have a shell, we need to forget about any FontFace - // objects for @font-face rules that came from the style set. - RebuildUserFontSet(); + // objects for @font-face rules that came from the style set. There's no need + // to call EnsureStyleFlush either, the shell is going away anyway, so there's + // no point on it. + MarkUserFontSetDirty(); if (mResizeObserverController) { mResizeObserverController->ShellDetachedFromDocument(); } nsIPresShell* oldShell = mPresShell; mPresShell = nullptr; UpdateFrameRequestCallbackSchedulingState(oldShell); @@ -12867,17 +12871,17 @@ gfxUserFontSet* nsIDocument::GetUserFontSet(bool aFlushUserFontSet) { // We want to initialize the user font set lazily the first time the // user asks for it, rather than building it too early and forcing // rule cascade creation. Thus we try to enforce the invariant that // we *never* build the user font set until the first call to // GetUserFontSet. However, once it's been requested, we can't wait // for somebody to call GetUserFontSet in order to rebuild it (see - // comments below in RebuildUserFontSet for why). + // comments below in MarkUserFontSetDirty for why). #ifdef DEBUG bool userFontSetGottenBefore = mGetUserFontSetCalled; #endif // Set mGetUserFontSetCalled up front, so that FlushUserFontSet will actually // flush. mGetUserFontSetCalled = true; if (mFontFaceSetDirty && aFlushUserFontSet) { // If this assertion fails, and there have actually been changes to @@ -12904,82 +12908,63 @@ void nsIDocument::FlushUserFontSet() { if (!mGetUserFontSetCalled) { return; // No one cares about this font set yet, but we want to be careful // to not unset our mFontFaceSetDirty bit, so when someone really // does we'll create it. } - if (mFontFaceSetDirty) { - if (gfxPlatform::GetPlatform()->DownloadableFontsEnabled()) { - nsTArray rules; - nsIPresShell* shell = GetShell(); - if (shell && !shell->StyleSet()->AppendFontFaceRules(rules)) { - return; - } - - bool changed = false; - - if (!mFontFaceSet && !rules.IsEmpty()) { - nsCOMPtr window = do_QueryInterface(GetScopeObject()); - mFontFaceSet = new FontFaceSet(window, this); - } - if (mFontFaceSet) { - changed = mFontFaceSet->UpdateRules(rules); - } - - // We need to enqueue a style change reflow (for later) to - // reflect that we're modifying @font-face rules. (However, - // without a reflow, nothing will happen to start any downloads - // that are needed.) - if (changed && shell) { - nsPresContext* presContext = shell->GetPresContext(); - if (presContext) { - presContext->UserFontSetUpdated(); - } - } - } - - mFontFaceSetDirty = false; - } -} - -void -nsIDocument::RebuildUserFontSet() + if (!mFontFaceSetDirty) { + return; + } + + mFontFaceSetDirty = false; + + if (gfxPlatform::GetPlatform()->DownloadableFontsEnabled()) { + nsTArray rules; + nsIPresShell* shell = GetShell(); + if (shell && !shell->StyleSet()->AppendFontFaceRules(rules)) { + return; + } + + + if (!mFontFaceSet && !rules.IsEmpty()) { + nsCOMPtr window = do_QueryInterface(GetScopeObject()); + mFontFaceSet = new FontFaceSet(window, this); + } + + bool changed = false; + if (mFontFaceSet) { + changed = mFontFaceSet->UpdateRules(rules); + } + + // We need to enqueue a style change reflow (for later) to + // reflect that we're modifying @font-face rules. (However, + // without a reflow, nothing will happen to start any downloads + // that are needed.) + if (changed && shell) { + if (nsPresContext* presContext = shell->GetPresContext()) { + presContext->UserFontSetUpdated(); + } + } + } +} + +void +nsIDocument::MarkUserFontSetDirty() { if (!mGetUserFontSetCalled) { // We want to lazily build the user font set the first time it's // requested (so we don't force creation of rule cascades too // early), so don't do anything now. return; } mFontFaceSetDirty = true; - if (nsIPresShell* shell = GetShell()) { - shell->SetNeedStyleFlush(); - } - - // Somebody has already asked for the user font set, so we need to - // post an event to rebuild it. Setting the user font set to be dirty - // and lazily rebuilding it isn't sufficient, since it is only the act - // of rebuilding it that will trigger the style change reflow that - // calls GetUserFontSet. (This reflow causes rebuilding of text runs, - // which starts font loads, whose completion causes another style - // change reflow). - if (!mPostedFlushUserFontSet) { - MOZ_RELEASE_ASSERT(NS_IsMainThread()); - nsCOMPtr ev = - NewRunnableMethod("nsIDocument::HandleRebuildUserFontSet", - this, - &nsIDocument::HandleRebuildUserFontSet); - if (NS_SUCCEEDED(Dispatch(TaskCategory::Other, ev.forget()))) { - mPostedFlushUserFontSet = true; - } - } } FontFaceSet* nsIDocument::Fonts() { if (!mFontFaceSet) { nsCOMPtr window = do_QueryInterface(GetScopeObject()); mFontFaceSet = new FontFaceSet(window, this); diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -3051,17 +3051,17 @@ public: if (weakNode) { mBlockedTrackingNodes.AppendElement(weakNode); } } gfxUserFontSet* GetUserFontSet(bool aFlushUserFontSet = true); void FlushUserFontSet(); - void RebuildUserFontSet(); // asynchronously + void MarkUserFontSetDirty(); mozilla::dom::FontFaceSet* GetFonts() { return mFontFaceSet; } // FontFaceSource mozilla::dom::FontFaceSet* Fonts(); bool DidFireDOMContentLoaded() const { return mDidFireDOMContentLoaded; } void SetDocumentUseCounter(mozilla::UseCounter aUseCounter) @@ -3274,21 +3274,16 @@ protected: nsCString GetContentTypeInternal() const { return mContentType; } mozilla::dom::XPathEvaluator* XPathEvaluator(); - void HandleRebuildUserFontSet() { - mPostedFlushUserFontSet = false; - FlushUserFontSet(); - } - // Update our frame request callback scheduling state, if needed. This will // schedule or unschedule them, if necessary, and update // mFrameRequestCallbacksScheduled. aOldShell should only be passed when // mPresShell is becoming null; in that case it will be used to get hold of // the relevant refresh driver. void UpdateFrameRequestCallbackSchedulingState(nsIPresShell* aOldShell = nullptr); // Helper for GetScrollingElement/IsScrollingElement. @@ -3533,19 +3528,16 @@ protected: bool mHasDisplayDocument : 1; // Is the current mFontFaceSet valid? bool mFontFaceSetDirty : 1; // Has GetUserFontSet() been called? bool mGetUserFontSetCalled : 1; - // Do we currently have an event posted to call FlushUserFontSet? - bool mPostedFlushUserFontSet : 1; - // True if we have fired the DOMContentLoaded event, or don't plan to fire one // (e.g. we're not being parsed at all). bool mDidFireDOMContentLoaded : 1; // True if ReportHasScrollLinkedEffect() has been called. bool mHasScrollLinkedEffect : 1; // True if we have frame request callbacks scheduled with the refresh driver. diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -34,16 +34,17 @@ #endif #include "gfxContext.h" #include "gfxPrefs.h" #include "gfxUserFontSet.h" #include "nsPresContext.h" #include "nsIContent.h" #include "nsIContentIterator.h" +#include "nsIPresShellInlines.h" #include "mozilla/dom/Element.h" #include "mozilla/dom/Event.h" // for Event::GetEventPopupControlState() #include "mozilla/dom/PointerEventHandler.h" #include "nsIDocument.h" #include "nsAnimationManager.h" #include "nsNameSpaceManager.h" // for Pref-related rule management (bugs 22963,20760,31816) #include "nsFrame.h" #include "FrameLayerBuilder.h" @@ -4528,21 +4529,22 @@ PresShell::ReconstructFrames() void nsIPresShell::RestyleForCSSRuleChanges() { if (mIsDestroying) { // We don't want to mess with restyles at this point return; } - mDocument->RebuildUserFontSet(); + EnsureStyleFlush(); + mDocument->MarkUserFontSetDirty(); if (mPresContext) { - mPresContext->RebuildCounterStyles(); - mPresContext->RebuildFontFeatureValues(); + mPresContext->MarkCounterStylesDirty(); + mPresContext->MarkFontFeatureValuesDirty(); } if (!mDidInitialize) { // Nothing to do here, since we have no frames yet return; } mStyleSet->InvalidateStyleForCSSRuleChanges(); diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp --- a/layout/base/nsPresContext.cpp +++ b/layout/base/nsPresContext.cpp @@ -305,19 +305,17 @@ nsPresContext::nsPresContext(nsIDocument mPendingMediaFeatureValuesChanged(false), mPrefChangePendingNeedsReflow(false), mIsEmulatingMedia(false), mIsGlyph(false), mUsesRootEMUnits(false), 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), @@ -1980,19 +1978,26 @@ nsPresContext::RebuildAllStyleData(nsCha mUsesExChUnits = false; if (mShell->StyleSet()->IsGecko()) { #ifdef MOZ_OLD_STYLE mShell->StyleSet()->AsGecko()->SetUsesViewportUnits(false); #else MOZ_CRASH("old style system disabled"); #endif } - mDocument->RebuildUserFontSet(); - RebuildCounterStyles(); - RebuildFontFeatureValues(); + + // TODO(emilio): It's unclear to me why would these three calls below be + // needed. In particular, RebuildAllStyleData doesn't rebuild rules or + // specified style information and such (note the comment in + // ServoRestyleManager::RebuildAllStyleData re. the funny semantics), so I + // don't know why should we rebuild the user font set / counter styles / + // etc... + mDocument->MarkUserFontSetDirty(); + MarkCounterStylesDirty(); + MarkFontFeatureValuesDirty(); RestyleManager()->RebuildAllStyleData(aExtraHint, aRestyleHint); } void nsPresContext::PostRebuildAllStyleDataEvent(nsChangeHint aExtraHint, nsRestyleHint aRestyleHint) { @@ -2328,37 +2333,24 @@ nsPresContext::FlushCounterStyles() RefreshDriver()->AddPostRefreshObserver( new CounterStyleCleaner(RefreshDriver(), mCounterStyleManager)); } mCounterStylesDirty = false; } } void -nsPresContext::RebuildCounterStyles() +nsPresContext::MarkCounterStylesDirty() { if (mCounterStyleManager->IsInitial()) { - // Still in its initial state, no need to reset. + // Still in its initial state, no need to touch anything. return; } mCounterStylesDirty = true; - if (mShell) { - mShell->SetNeedStyleFlush(); - } - if (!mPostedFlushCounterStyles) { - nsCOMPtr ev = - NewRunnableMethod("nsPresContext::HandleRebuildCounterStyles", - this, &nsPresContext::HandleRebuildCounterStyles); - nsresult rv = - Document()->Dispatch(TaskCategory::Other, ev.forget()); - if (NS_SUCCEEDED(rv)) { - mPostedFlushCounterStyles = true; - } - } } void nsPresContext::NotifyMissingFonts() { if (mMissingFonts) { mMissingFonts->Flush(); } @@ -3089,38 +3081,16 @@ nsPresContext::FlushFontFeatureValues() if (mFontFeatureValuesDirty) { StyleSetHandle styleSet = mShell->StyleSet(); mFontFeatureValuesLookup = styleSet->BuildFontFeatureValueSet(); mFontFeatureValuesDirty = false; } } -void -nsPresContext::RebuildFontFeatureValues() -{ - if (!mShell) { - return; // we've been torn down - } - - mFontFeatureValuesDirty = true; - mShell->SetNeedStyleFlush(); - - if (!mPostedFlushFontFeatureValues) { - nsCOMPtr ev = - NewRunnableMethod("nsPresContext::HandleRebuildFontFeatureValues", - this, &nsPresContext::HandleRebuildFontFeatureValues); - nsresult rv = - Document()->Dispatch(TaskCategory::Other, ev.forget()); - if (NS_SUCCEEDED(rv)) { - mPostedFlushFontFeatureValues = true; - } - } -} - nsRootPresContext::nsRootPresContext(nsIDocument* aDocument, nsPresContextType aType) : nsPresContext(aDocument, aType) { } nsRootPresContext::~nsRootPresContext() { diff --git a/layout/base/nsPresContext.h b/layout/base/nsPresContext.h --- a/layout/base/nsPresContext.h +++ b/layout/base/nsPresContext.h @@ -959,33 +959,36 @@ public: mPaintFlashing = aPaintFlashing; mPaintFlashingInitialized = true; } // This method should be used instead of directly accessing mPaintFlashing, // as that value may be out of date when mPaintFlashingInitialized is false. bool GetPaintFlashing() const; - bool SuppressingResizeReflow() const { return mSuppressResizeReflow; } + bool SuppressingResizeReflow() const { return mSuppressResizeReflow; } gfxUserFontSet* GetUserFontSet(bool aFlushUserFontSet = true); // Should be called whenever the set of fonts available in the user // font set changes (e.g., because a new font loads, or because the // user font set is changed and fonts become unavailable). void UserFontSetUpdated(gfxUserFontEntry* aUpdatedFont = nullptr); gfxMissingFontRecorder *MissingFontRecorder() { return mMissingFonts; } void NotifyMissingFonts(); void FlushCounterStyles(); - void RebuildCounterStyles(); // asynchronously + void MarkCounterStylesDirty(); void FlushFontFeatureValues(); - void RebuildFontFeatureValues(); // asynchronously + void MarkFontFeatureValuesDirty() + { + mFontFeatureValuesDirty = true; + } // Ensure that it is safe to hand out CSS rules outside the layout // engine by ensuring that all CSS style sheets have unique inners // and, if necessary, synchronously rebuilding all style data. void EnsureSafeToHandOutCSSRules(); // Mark an area as invalidated, associated with a given transaction id (allocated // by nsRefreshDriver::GetTransactionId). @@ -1226,26 +1229,16 @@ public: void InvalidatePaintedLayers(); protected: // May be called multiple times (unlink, destructor) void Destroy(); void AppUnitsPerDevPixelChanged(); - void HandleRebuildCounterStyles() { - mPostedFlushCounterStyles = false; - FlushCounterStyles(); - } - - void HandleRebuildFontFeatureValues() { - mPostedFlushFontFeatureValues = false; - FlushFontFeatureValues(); - } - bool HavePendingInputEvent(); // Can't be inline because we can't include nsStyleSet.h. 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 CreateTimer(nsTimerCallbackFunc aCallback, @@ -1426,23 +1419,19 @@ protected: // Does the associated document use ex or ch units? unsigned mUsesExChUnits : 1; // Has there been a change to the viewport's dimensions? unsigned mPendingViewportChange : 1; // Is the current mCounterStyleManager valid? unsigned mCounterStylesDirty : 1; - // Do we currently have an event posted to call FlushCounterStyles? - unsigned mPostedFlushCounterStyles: 1; // Is the current mFontFeatureValuesLookup valid? unsigned mFontFeatureValuesDirty : 1; - // Do we currently have an event posted to call FlushFontFeatureValues? - 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; diff --git a/layout/style/FontFaceSet.cpp b/layout/style/FontFaceSet.cpp --- a/layout/style/FontFaceSet.cpp +++ b/layout/style/FontFaceSet.cpp @@ -34,16 +34,17 @@ #include "nsIConsoleService.h" #include "nsIContentPolicy.h" #include "nsIContentSecurityPolicy.h" #include "nsIDocShell.h" #include "nsIDocument.h" #include "nsILoadContext.h" #include "nsINetworkPredictor.h" #include "nsIPresShell.h" +#include "nsIPresShellInlines.h" #include "nsIPrincipal.h" #include "nsISupportsPriority.h" #include "nsIWebNavigation.h" #include "nsNetUtil.h" #include "nsIProtocolHandler.h" #include "nsIInputStream.h" #include "nsLayoutUtils.h" #include "nsPresContext.h" @@ -493,17 +494,17 @@ FontFaceSet::Add(FontFace& aFontFace, Er FontFaceRecord* rec = mNonRuleFaces.AppendElement(); rec->mFontFace = &aFontFace; rec->mSheetType = SheetType::Unknown; // unused for mNonRuleFaces rec->mLoadEventShouldFire = aFontFace.Status() == FontFaceLoadStatus::Unloaded || aFontFace.Status() == FontFaceLoadStatus::Loading; mNonRuleFacesDirty = true; - RebuildUserFontSet(); + MarkUserFontSetDirty(); mHasLoadingFontFacesIsDirty = true; CheckLoadingStarted(); return this; } void FontFaceSet::Clear() { @@ -515,17 +516,17 @@ FontFaceSet::Clear() for (size_t i = 0; i < mNonRuleFaces.Length(); i++) { FontFace* f = mNonRuleFaces[i].mFontFace; f->RemoveFontFaceSet(this); } mNonRuleFaces.Clear(); mNonRuleFacesDirty = true; - RebuildUserFontSet(); + MarkUserFontSetDirty(); mHasLoadingFontFacesIsDirty = true; CheckLoadingFinished(); } bool FontFaceSet::Delete(FontFace& aFontFace) { FlushUserFontSet(); @@ -544,17 +545,17 @@ FontFaceSet::Delete(FontFace& aFontFace) } if (!removed) { return false; } aFontFace.RemoveFontFaceSet(this); mNonRuleFacesDirty = true; - RebuildUserFontSet(); + MarkUserFontSetDirty(); mHasLoadingFontFacesIsDirty = true; CheckLoadingFinished(); return true; } bool FontFaceSet::HasAvailableFontFace(FontFace* aFontFace) { @@ -1825,20 +1826,26 @@ void FontFaceSet::FlushUserFontSet() { if (mDocument) { mDocument->FlushUserFontSet(); } } void -FontFaceSet::RebuildUserFontSet() +FontFaceSet::MarkUserFontSetDirty() { if (mDocument) { - mDocument->RebuildUserFontSet(); + // Ensure we trigger at least a style flush, that will eventually flush the + // user font set. Otherwise the font loads that that flush may cause could + // never be triggered. + if (nsIPresShell* shell = mDocument->GetShell()) { + shell->EnsureStyleFlush(); + } + mDocument->MarkUserFontSetDirty(); } } nsPresContext* FontFaceSet::GetPresContext() { if (!mDocument) { return nullptr; @@ -1961,17 +1968,17 @@ FontFaceSet::UserFontSet::GetPrivateBrow } /* virtual */ void FontFaceSet::UserFontSet::DoRebuildUserFontSet() { if (!mFontFaceSet) { return; } - mFontFaceSet->RebuildUserFontSet(); + mFontFaceSet->MarkUserFontSetDirty(); } /* virtual */ already_AddRefed FontFaceSet::UserFontSet::CreateUserFontEntry( const nsTArray& aFontFaceSrcList, uint32_t aWeight, int32_t aStretch, uint8_t aStyle, diff --git a/layout/style/FontFaceSet.h b/layout/style/FontFaceSet.h --- a/layout/style/FontFaceSet.h +++ b/layout/style/FontFaceSet.h @@ -288,17 +288,17 @@ private: nsresult SyncLoadFontData(gfxUserFontEntry* aFontToLoad, const gfxFontFaceSrc* aFontFaceSrc, uint8_t*& aBuffer, uint32_t& aBufferLength); nsresult LogMessage(gfxUserFontEntry* aUserFontEntry, const char* aMessage, uint32_t aFlags, nsresult aStatus); - void RebuildUserFontSet(); + void MarkUserFontSetDirty(); void InsertRuleFontFace(FontFace* aFontFace, SheetType aSheetType, nsTArray& aOldRecords, bool& aFontSetModified); void InsertNonRuleFontFace(FontFace* aFontFace, bool& aFontSetModified); #ifdef DEBUG bool HasRuleFontFace(FontFace* aFontFace);