From 30de825396eacfa6a80cf8316f8c5eacee4884f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 2 Feb 2019 19:41:44 +0100 Subject: [PATCH] Bug 1524328 - Don't resolve counter styles in the style system. r=xidorn Doing it during layout instead. This also has the nice side-effect of no longer needing to do a full restyle when counter-style rules are inserted. Differential Revision: https://phabricator.services.mozilla.com/D18343 --- layout/base/nsCounterManager.cpp | 8 +- layout/base/nsPresContext.cpp | 3 +- layout/generic/ReflowInput.cpp | 41 +++++---- layout/generic/nsBlockFrame.cpp | 9 +- layout/generic/nsBulletFrame.cpp | 122 +++++++++++++++------------ layout/generic/nsBulletFrame.h | 5 +- layout/style/CounterStyleManager.cpp | 45 +++------- layout/style/CounterStyleManager.h | 114 ++++++++++--------------- layout/style/GeckoBindings.cpp | 14 +-- layout/style/nsStyleStruct.cpp | 46 +++------- layout/style/nsStyleStruct.h | 2 - 11 files changed, 180 insertions(+), 229 deletions(-) diff --git a/layout/base/nsCounterManager.cpp b/layout/base/nsCounterManager.cpp index e5d17895e8349..8520e9fe3ac9d 100644 --- a/layout/base/nsCounterManager.cpp +++ b/layout/base/nsCounterManager.cpp @@ -78,13 +78,15 @@ void nsCounterUseNode::GetText(nsString& aResult) { } } - WritingMode wm = - mPseudoFrame ? mPseudoFrame->GetWritingMode() : WritingMode(); + WritingMode wm = mPseudoFrame->GetWritingMode(); + CounterStyle* style = + mPseudoFrame->PresContext()->CounterStyleManager()->ResolveCounterStyle( + mCounterStyle); for (uint32_t i = stack.Length() - 1;; --i) { nsCounterNode* n = stack[i]; nsAutoString text; bool isTextRTL; - mCounterStyle->GetCounterText(n->mValueAfter, wm, text, isTextRTL); + style->GetCounterText(n->mValueAfter, wm, text, isTextRTL); aResult.Append(text); if (i == 0) { break; diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp index 49490b5febb3b..83310728cf9db 100644 --- a/layout/base/nsPresContext.cpp +++ b/layout/base/nsPresContext.cpp @@ -1931,8 +1931,7 @@ void nsPresContext::FlushCounterStyles() { bool changed = mCounterStyleManager->NotifyRuleChanged(); if (changed) { PresShell()->NotifyCounterStylesAreDirty(); - PostRebuildAllStyleDataEvent(NS_STYLE_HINT_REFLOW, - eRestyle_ForceDescendants); + PostRebuildAllStyleDataEvent(NS_STYLE_HINT_REFLOW, nsRestyleHint(0)); RefreshDriver()->AddPostRefreshObserver( new CounterStyleCleaner(RefreshDriver(), mCounterStyleManager)); } diff --git a/layout/generic/ReflowInput.cpp b/layout/generic/ReflowInput.cpp index a524ab3b53c81..365cf69aeb582 100644 --- a/layout/generic/ReflowInput.cpp +++ b/layout/generic/ReflowInput.cpp @@ -136,21 +136,32 @@ static nscoord FontSizeInflationListMarginAdjustment(const nsIFrame* aFrame) { } float inflation = nsLayoutUtils::FontSizeInflationFor(aFrame); - if (inflation > 1.0f) { - auto listStyleType = aFrame->StyleList()->mCounterStyle->GetStyle(); - if (listStyleType != NS_STYLE_LIST_STYLE_NONE && - listStyleType != NS_STYLE_LIST_STYLE_DISC && - listStyleType != NS_STYLE_LIST_STYLE_CIRCLE && - listStyleType != NS_STYLE_LIST_STYLE_SQUARE && - listStyleType != NS_STYLE_LIST_STYLE_DISCLOSURE_CLOSED && - listStyleType != NS_STYLE_LIST_STYLE_DISCLOSURE_OPEN) { - // The HTML spec states that the default padding for ordered lists - // begins at 40px, indicating that we have 40px of space to place a - // bullet. When performing font inflation calculations, we add space - // equivalent to this, but simply inflated at the same amount as the - // text, in app units. - return nsPresContext::CSSPixelsToAppUnits(40) * (inflation - 1); - } + if (inflation <= 1.0f) { + return 0; + } + + // The HTML spec states that the default padding for ordered lists + // begins at 40px, indicating that we have 40px of space to place a + // bullet. When performing font inflation calculations, we add space + // equivalent to this, but simply inflated at the same amount as the + // text, in app units. + auto margin = nsPresContext::CSSPixelsToAppUnits(40) * (inflation - 1); + + auto* list = aFrame->StyleList(); + if (!list->mCounterStyle.IsAtom()) { + return margin; + } + + // NOTE(emilio): @counter-style can override some of the styles from this + // list, and we won't add margin to the counter. + // + // See https://github.com/w3c/csswg-drafts/issues/3584 + nsAtom* type = list->mCounterStyle.AsAtom(); + if (type != nsGkAtoms::none && type != nsGkAtoms::disc && + type != nsGkAtoms::circle && type != nsGkAtoms::square && + type != nsGkAtoms::disclosure_closed && + type != nsGkAtoms::disclosure_open) { + return margin; } return 0; diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp index 02ebf601a078d..e69c35db84bf9 100644 --- a/layout/generic/nsBlockFrame.cpp +++ b/layout/generic/nsBlockFrame.cpp @@ -6783,10 +6783,13 @@ void nsBlockFrame::CreateBulletFrameForListItem() { NS_BLOCK_FRAME_HAS_OUTSIDE_BULLET)) == 0, "How can we have a bullet already?"); - nsIPresShell* shell = PresShell(); + nsPresContext* pc = PresContext(); + nsIPresShell* shell = pc->PresShell(); const nsStyleList* styleList = StyleList(); + CounterStyle* style = + pc->CounterStyleManager()->ResolveCounterStyle(styleList->mCounterStyle); - CSSPseudoElementType pseudoType = styleList->mCounterStyle->IsBullet() + CSSPseudoElementType pseudoType = style->IsBullet() ? CSSPseudoElementType::mozListBullet : CSSPseudoElementType::mozListNumber; @@ -6817,7 +6820,7 @@ bool nsBlockFrame::BulletIsEmpty() const { HasOutsideBullet(), "should only care when we have an outside bullet"); const nsStyleList* list = StyleList(); - return list->mCounterStyle->IsNone() && !list->GetListStyleImage(); + return list->mCounterStyle.IsNone() && !list->GetListStyleImage(); } void nsBlockFrame::GetSpokenBulletText(nsAString& aText) const { diff --git a/layout/generic/nsBulletFrame.cpp b/layout/generic/nsBulletFrame.cpp index 4c683b6e8b359..dae07e1a57d3b 100644 --- a/layout/generic/nsBulletFrame.cpp +++ b/layout/generic/nsBulletFrame.cpp @@ -52,6 +52,7 @@ using namespace mozilla; using namespace mozilla::gfx; using namespace mozilla::image; using namespace mozilla::layout; +using mozilla::dom::Document; NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(FontSizeInflationProperty, float) @@ -65,6 +66,11 @@ NS_QUERYFRAME_TAIL_INHERITING(nsFrame) nsBulletFrame::~nsBulletFrame() {} +CounterStyle* nsBulletFrame::ResolveCounterStyle() { + return PresContext()->CounterStyleManager()->ResolveCounterStyle( + StyleList()->mCounterStyle); +} + void nsBulletFrame::DestroyFrom(nsIFrame* aDestructRoot, PostDestroyData& aPostDestroyData) { // Stop image loading first. @@ -87,7 +93,7 @@ nsresult nsBulletFrame::GetFrameName(nsAString& aResult) const { bool nsBulletFrame::IsEmpty() { return IsSelfEmpty(); } bool nsBulletFrame::IsSelfEmpty() { - return StyleList()->mCounterStyle->IsNone(); + return StyleList()->mCounterStyle.IsNone(); } /* virtual */ void nsBulletFrame::DidSetComputedStyle( @@ -147,11 +153,11 @@ bool nsBulletFrame::IsSelfEmpty() { const nsStyleList* oldStyleList = aOldComputedStyle->PeekStyleList(); if (oldStyleList) { bool hadBullet = oldStyleList->GetListStyleImage() || - !oldStyleList->mCounterStyle->IsNone(); + !oldStyleList->mCounterStyle.IsNone(); const nsStyleList* newStyleList = StyleList(); bool hasBullet = newStyleList->GetListStyleImage() || - !newStyleList->mCounterStyle->IsNone(); + !newStyleList->mCounterStyle.IsNone(); if (hadBullet != hasBullet) { accService->UpdateListBullet(PresContext()->GetPresShell(), mContent, @@ -650,7 +656,7 @@ void nsBulletFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, Maybe nsBulletFrame::CreateBulletRenderer( gfxContext& aRenderingContext, nsPoint aPt) { const nsStyleList* myList = StyleList(); - CounterStyle* listStyleType = myList->mCounterStyle; + CounterStyle* listStyleType = ResolveCounterStyle(); nsMargin padding = mPadding.GetPhysicalMargin(GetWritingMode()); if (myList->GetListStyleImage() && mImageRequest) { @@ -766,7 +772,7 @@ Maybe nsBulletFrame::CreateBulletRenderer( RefPtr fm = nsLayoutUtils::GetFontMetricsForFrame(this, GetFontSizeInflation()); nsAutoString text; - GetListItemText(text); + GetListItemText(listStyleType, GetWritingMode(), GetOrdinal(), text); WritingMode wm = GetWritingMode(); nscoord ascent = wm.IsLineInverted() ? fm->MaxDescent() : fm->MaxAscent(); aPt.MoveBy(padding.left, padding.top); @@ -835,25 +841,27 @@ int32_t nsBulletFrame::SetListItemOrdinal(int32_t aNextOrdinal, bool* aChanged, return nsCounterManager::IncrementCounter(mOrdinal, aIncrement); } -void nsBulletFrame::GetListItemText(nsAString& aResult) { - CounterStyle* style = StyleList()->mCounterStyle; - NS_ASSERTION(style->GetStyle() != NS_STYLE_LIST_STYLE_NONE && - style->GetStyle() != NS_STYLE_LIST_STYLE_DISC && - style->GetStyle() != NS_STYLE_LIST_STYLE_CIRCLE && - style->GetStyle() != NS_STYLE_LIST_STYLE_SQUARE && - style->GetStyle() != NS_STYLE_LIST_STYLE_DISCLOSURE_CLOSED && - style->GetStyle() != NS_STYLE_LIST_STYLE_DISCLOSURE_OPEN, - "we should be using specialized code for these types"); +void nsBulletFrame::GetListItemText(CounterStyle* aStyle, + mozilla::WritingMode aWritingMode, + int32_t aOrdinal, nsAString& aResult) { + NS_ASSERTION( + aStyle->GetStyle() != NS_STYLE_LIST_STYLE_NONE && + aStyle->GetStyle() != NS_STYLE_LIST_STYLE_DISC && + aStyle->GetStyle() != NS_STYLE_LIST_STYLE_CIRCLE && + aStyle->GetStyle() != NS_STYLE_LIST_STYLE_SQUARE && + aStyle->GetStyle() != NS_STYLE_LIST_STYLE_DISCLOSURE_CLOSED && + aStyle->GetStyle() != NS_STYLE_LIST_STYLE_DISCLOSURE_OPEN, + "we should be using specialized code for these types"); bool isRTL; nsAutoString counter, prefix, suffix; - style->GetPrefix(prefix); - style->GetSuffix(suffix); - style->GetCounterText(mOrdinal, GetWritingMode(), counter, isRTL); + aStyle->GetPrefix(prefix); + aStyle->GetSuffix(suffix); + aStyle->GetCounterText(aOrdinal, aWritingMode, counter, isRTL); aResult.Truncate(); aResult.Append(prefix); - if (GetWritingMode().IsBidiLTR() != isRTL) { + if (aWritingMode.IsBidiLTR() != isRTL) { aResult.Append(counter); } else { // RLM = 0x200f, LRM = 0x200e @@ -919,7 +927,8 @@ void nsBulletFrame::GetDesiredSize(nsPresContext* aCX, nscoord bulletSize; nsAutoString text; - switch (myList->mCounterStyle->GetStyle()) { + CounterStyle* style = ResolveCounterStyle(); + switch (style->GetStyle()) { case NS_STYLE_LIST_STYLE_NONE: finalSize.ISize(wm) = finalSize.BSize(wm) = 0; aMetrics.SetBlockStartAscent(0); @@ -952,7 +961,7 @@ void nsBulletFrame::GetDesiredSize(nsPresContext* aCX, break; default: - GetListItemText(text); + GetListItemText(style, GetWritingMode(), GetOrdinal(), text); finalSize.BSize(wm) = fm->MaxHeight(); finalSize.ISize(wm) = nsLayoutUtils::AppUnitWidthOfStringBidi( text, this, *fm, *aRenderingContext); @@ -1034,7 +1043,7 @@ static inline bool IsIgnoreable(const nsIFrame* aFrame, nscoord aISize) { return false; } auto listStyle = aFrame->StyleList(); - return listStyle->mCounterStyle->IsNone() && !listStyle->GetListStyleImage(); + return listStyle->mCounterStyle.IsNone() && !listStyle->GetListStyleImage(); } /* virtual */ void nsBulletFrame::AddInlineMinISize( @@ -1209,47 +1218,54 @@ already_AddRefed nsBulletFrame::GetImage() const { return nullptr; } +nscoord nsBulletFrame::GetListStyleAscent() const { + RefPtr fm = + nsLayoutUtils::GetFontMetricsForFrame(this, GetFontSizeInflation()); + auto* list = StyleList(); + if (list->mCounterStyle.IsNone()) { + return 0; + } + if (list->mCounterStyle.IsAnonymous()) { + return fm->MaxAscent(); + } + // NOTE(emilio): @counter-style can override most of the styles from this + // list, and we still return the changed ascent. Do we care about that? + // + // https://github.com/w3c/csswg-drafts/issues/3584 + nsAtom* style = list->mCounterStyle.AsAtom(); + if (style == nsGkAtoms::disc || style == nsGkAtoms::circle || + style == nsGkAtoms::square) { + nscoord ascent = fm->MaxAscent(); + nscoord baselinePadding = NSToCoordRound(float(ascent) / 8.0f); + ascent = std::max(nsPresContext::CSSPixelsToAppUnits(MIN_BULLET_SIZE), + NSToCoordRound(0.8f * (float(ascent) / 2.0f))); + return ascent + baselinePadding; + } + if (style == nsGkAtoms::disclosure_open || + style == nsGkAtoms::disclosure_closed) { + nscoord ascent = fm->EmAscent(); + nscoord baselinePadding = NSToCoordRound(0.125f * ascent); + ascent = std::max(nsPresContext::CSSPixelsToAppUnits(MIN_BULLET_SIZE), + NSToCoordRound(0.75f * ascent)); + return ascent + baselinePadding; + } + return fm->MaxAscent(); +} + nscoord nsBulletFrame::GetLogicalBaseline(WritingMode aWritingMode) const { - nscoord ascent = 0, baselinePadding; + nscoord ascent = 0; if (GetStateBits() & BULLET_FRAME_IMAGE_LOADING) { ascent = BSize(aWritingMode); } else { - RefPtr fm = - nsLayoutUtils::GetFontMetricsForFrame(this, GetFontSizeInflation()); - CounterStyle* listStyleType = StyleList()->mCounterStyle; - switch (listStyleType->GetStyle()) { - case NS_STYLE_LIST_STYLE_NONE: - break; - - case NS_STYLE_LIST_STYLE_DISC: - case NS_STYLE_LIST_STYLE_CIRCLE: - case NS_STYLE_LIST_STYLE_SQUARE: - ascent = fm->MaxAscent(); - baselinePadding = NSToCoordRound(float(ascent) / 8.0f); - ascent = std::max(nsPresContext::CSSPixelsToAppUnits(MIN_BULLET_SIZE), - NSToCoordRound(0.8f * (float(ascent) / 2.0f))); - ascent += baselinePadding; - break; - - case NS_STYLE_LIST_STYLE_DISCLOSURE_CLOSED: - case NS_STYLE_LIST_STYLE_DISCLOSURE_OPEN: - ascent = fm->EmAscent(); - baselinePadding = NSToCoordRound(0.125f * ascent); - ascent = std::max(nsPresContext::CSSPixelsToAppUnits(MIN_BULLET_SIZE), - NSToCoordRound(0.75f * ascent)); - ascent += baselinePadding; - break; - - default: - ascent = fm->MaxAscent(); - break; - } + ascent = GetListStyleAscent(); } return ascent + GetLogicalUsedMargin(aWritingMode).BStart(aWritingMode); } void nsBulletFrame::GetSpokenText(nsAString& aText) { - CounterStyle* style = StyleList()->mCounterStyle; + CounterStyle* style = + PresContext()->CounterStyleManager()->ResolveCounterStyle( + StyleList()->mCounterStyle); bool isBullet; style->GetSpokenCounterText(mOrdinal, GetWritingMode(), aText, isBullet); if (isBullet) { diff --git a/layout/generic/nsBulletFrame.h b/layout/generic/nsBulletFrame.h index 78ee775c5d5f5..6a0d672d62ef8 100644 --- a/layout/generic/nsBulletFrame.h +++ b/layout/generic/nsBulletFrame.h @@ -93,7 +93,8 @@ class nsBulletFrame final : public nsFrame { int32_t aIncrement); /* get list item text, with prefix & suffix */ - void GetListItemText(nsAString& aResult); + static void GetListItemText(mozilla::CounterStyle*, mozilla::WritingMode, + int32_t aOrdinal, nsAString& aResult); void GetSpokenText(nsAString& aText); @@ -139,6 +140,8 @@ class nsBulletFrame final : public nsFrame { int32_t mOrdinal; private: + mozilla::CounterStyle* ResolveCounterStyle(); + nscoord GetListStyleAscent() const; void RegisterImageRequest(bool aKnownToBeAnimated); void DeregisterAndCancelImageRequest(); diff --git a/layout/style/CounterStyleManager.cpp b/layout/style/CounterStyleManager.cpp index b01b15db1b1d1..8f1a3b99e755b 100644 --- a/layout/style/CounterStyleManager.cpp +++ b/layout/style/CounterStyleManager.cpp @@ -512,7 +512,8 @@ class BuiltinCounterStyle : public CounterStyle { constexpr BuiltinCounterStyle(int32_t aStyle, nsStaticAtom* aName) : CounterStyle(aStyle), mName(aName) {} - nsStaticAtom* GetStyleName() const final; + nsStaticAtom* GetStyleName() const { return mName; } + virtual void GetPrefix(nsAString& aResult) override; virtual void GetSuffix(nsAString& aResult) override; virtual void GetSpokenCounterText(CounterValue aOrdinal, @@ -534,21 +535,13 @@ class BuiltinCounterStyle : public CounterStyle { nsAString& aResult, bool& aIsRTL) override; protected: - BuiltinCounterStyle(const BuiltinCounterStyle& aOther) + constexpr BuiltinCounterStyle(const BuiltinCounterStyle& aOther) : CounterStyle(aOther.mStyle), mName(aOther.mName) {} private: - // The atom for the name of the builtin counter style. - // Extra indirection to point to nsGkAtoms members rather than the - // nsAtom, because members of nsGkAtoms are updated at runtime but - // we want to construct BuiltinCounterStyle at compile time. - nsStaticAtom* const mName; + nsStaticAtom* mName; }; -/* virtual */ nsStaticAtom* BuiltinCounterStyle::GetStyleName() const { - return mName; -} - /* virtual */ void BuiltinCounterStyle::GetPrefix(nsAString& aResult) { aResult.Truncate(); } @@ -909,7 +902,7 @@ class DependentBuiltinCounterStyle final : public BuiltinCounterStyle { // only case fallback is accessed is that they are extended. // Since extending styles will cache the data themselves, we need // not cache it here. - return mManager->BuildCounterStyle(nsGkAtoms::cjk_decimal); + return mManager->ResolveCounterStyle(nsGkAtoms::cjk_decimal); default: MOZ_ASSERT_UNREACHABLE("Not a valid dependent builtin style"); return BuiltinCounterStyle::GetFallback(); @@ -918,10 +911,9 @@ class DependentBuiltinCounterStyle final : public BuiltinCounterStyle { class CustomCounterStyle final : public CounterStyle { public: - CustomCounterStyle(nsAtom* aName, CounterStyleManager* aManager, + CustomCounterStyle(CounterStyleManager* aManager, const RawServoCounterStyleRule* aRule) : CounterStyle(NS_STYLE_LIST_STYLE_CUSTOM), - mName(aName), mManager(aManager), mRule(aRule), mRuleGeneration(Servo_CounterStyleRule_GetGeneration(aRule)), @@ -947,7 +939,6 @@ class CustomCounterStyle final : public CounterStyle { const RawServoCounterStyleRule* GetRule() const { return mRule; } uint32_t GetRuleGeneration() const { return mRuleGeneration; } - virtual nsAtom* GetStyleName() const override; virtual void GetPrefix(nsAString& aResult) override; virtual void GetSuffix(nsAString& aResult) override; virtual void GetSpokenCounterText(CounterValue aOrdinal, @@ -1011,8 +1002,6 @@ class CustomCounterStyle final : public CounterStyle { CounterStyle* GetExtends(); CounterStyle* GetExtendsRoot(); - RefPtr mName; - // CounterStyleManager should always overlive any CounterStyle as it // is owned by nsPresContext, and will be released after all nodes and // frames are released. @@ -1094,8 +1083,6 @@ void CustomCounterStyle::ResetDependentData() { } } -/* virtual */ nsAtom* CustomCounterStyle::GetStyleName() const { return mName; } - /* virtual */ void CustomCounterStyle::GetPrefix(nsAString& aResult) { if (!(mFlags & FLAG_PREFIX_INITED)) { mFlags |= FLAG_PREFIX_INITED; @@ -1252,7 +1239,7 @@ static inline bool IsRangeValueInfinite(const nsCSSValue& aValue) { if (!mFallback) { mFallback = CounterStyleManager::GetDecimalStyle(); if (nsAtom* fallback = Servo_CounterStyleRule_GetFallback(mRule)) { - mFallback = mManager->BuildCounterStyle(fallback); + mFallback = mManager->ResolveCounterStyle(fallback); } else if (IsExtendsSystem()) { mFallback = GetExtends()->GetFallback(); } @@ -1374,7 +1361,7 @@ void CustomCounterStyle::ComputeRawSpeakAs(uint8_t& aSpeakAs, break; case eCSSUnit_AtomIdent: aSpeakAs = NS_STYLE_COUNTER_SPEAKAS_OTHER; - aSpeakAsCounter = mManager->BuildCounterStyle(value.GetAtomValue()); + aSpeakAsCounter = mManager->ResolveCounterStyle(value.GetAtomValue()); break; case eCSSUnit_Null: { if (!IsExtendsSystem()) { @@ -1477,7 +1464,7 @@ CounterStyle* CustomCounterStyle::ComputeExtends() { } nsAtom* extended = Servo_CounterStyleRule_GetExtended(mRule); - CounterStyle* nextCounter = mManager->BuildCounterStyle(extended); + CounterStyle* nextCounter = mManager->ResolveCounterStyle(extended); CounterStyle* target = nextCounter; if (nextCounter->IsCustomStyle()) { mFlags |= FLAG_EXTENDS_VISITED; @@ -1556,10 +1543,6 @@ AnonymousCounterStyle::AnonymousCounterStyle(uint8_t aSystem, mSystem(aSystem), mSymbols(std::move(aSymbols)) {} -/* virtual */ nsAtom* AnonymousCounterStyle::GetStyleName() const { - return nullptr; -} - /* virtual */ void AnonymousCounterStyle::GetPrefix(nsAString& aResult) { aResult.Truncate(); } @@ -1800,7 +1783,7 @@ void CounterStyleManager::Disconnect() { mPresContext = nullptr; } -CounterStyle* CounterStyleManager::BuildCounterStyle(nsAtom* aName) { +CounterStyle* CounterStyleManager::ResolveCounterStyle(nsAtom* aName) { MOZ_ASSERT(NS_IsMainThread()); CounterStyle* data = GetCounterStyle(aName); if (data) { @@ -1813,7 +1796,7 @@ CounterStyle* CounterStyleManager::BuildCounterStyle(nsAtom* aName) { auto* rule = styleSet->CounterStyleRuleForName(aName); if (rule) { MOZ_ASSERT(Servo_CounterStyleRule_GetName(rule) == aName); - data = new (mPresContext) CustomCounterStyle(aName, this, rule); + data = new (mPresContext) CustomCounterStyle(this, rule); } else { for (const BuiltinCounterStyle& item : gBuiltinStyleTable) { if (item.GetStyleName() == aName) { @@ -1844,12 +1827,6 @@ CounterStyle* CounterStyleManager::BuildCounterStyle(nsAtom* aName) { return const_cast(&gBuiltinStyleTable[aStyle]); } -/* static */ nsAtom* CounterStyleManager::GetStyleNameFromType(int32_t aStyle) { - MOZ_ASSERT(0 <= aStyle && size_t(aStyle) < sizeof(gBuiltinStyleTable), - "Require a valid builtin style constant"); - return gBuiltinStyleTable[aStyle].GetStyleName(); -} - bool CounterStyleManager::NotifyRuleChanged() { bool changed = false; for (auto iter = mStyles.Iter(); !iter.Done(); iter.Next()) { diff --git a/layout/style/CounterStyleManager.h b/layout/style/CounterStyleManager.h index 6bf5dfddfb99b..4e366380be599 100644 --- a/layout/style/CounterStyleManager.h +++ b/layout/style/CounterStyleManager.h @@ -7,6 +7,7 @@ #define mozilla_CounterStyleManager_h_ #include "nsAtom.h" +#include "nsGkAtoms.h" #include "nsStringFwd.h" #include "nsDataHashtable.h" #include "nsHashKeys.h" @@ -48,7 +49,6 @@ class CounterStyle { // styles are dependent for fallback. bool IsDependentStyle() const; - virtual nsAtom* GetStyleName() const = 0; virtual void GetPrefix(nsAString& aResult) = 0; virtual void GetSuffix(nsAString& aResult) = 0; void GetCounterText(CounterValue aOrdinal, WritingMode aWritingMode, @@ -98,7 +98,6 @@ class AnonymousCounterStyle final : public CounterStyle { AnonymousCounterStyle(uint8_t aSystem, nsTArray aSymbols); explicit AnonymousCounterStyle(const nsCSSValue::Array* aValue); - virtual nsAtom* GetStyleName() const override; virtual void GetPrefix(nsAString& aResult) override; virtual void GetSuffix(nsAString& aResult) override; virtual bool IsBullet() override; @@ -138,16 +137,17 @@ class CounterStylePtr { public: CounterStylePtr() : mRaw(0) {} CounterStylePtr(const CounterStylePtr& aOther) : mRaw(aOther.mRaw) { + if (!mRaw) { + return; + } switch (GetType()) { - case eCounterStyle: - break; case eAnonymousCounterStyle: AsAnonymous()->AddRef(); break; - case eUnresolvedAtom: + case eAtom: AsAtom()->AddRef(); break; - case eMask: + default: MOZ_ASSERT_UNREACHABLE("Unknown type"); break; } @@ -176,50 +176,39 @@ class CounterStylePtr { Reset(); return *this; } + CounterStylePtr& operator=(nsStaticAtom* aStaticAtom) { + Reset(); + mRaw = reinterpret_cast(aStaticAtom) | eAtom; + return *this; + } CounterStylePtr& operator=(already_AddRefed aAtom) { Reset(); - if (nsAtom* raw = aAtom.take()) { - AssertPointerAligned(raw); - mRaw = reinterpret_cast(raw) | eUnresolvedAtom; - } + mRaw = reinterpret_cast(aAtom.take()) | eAtom; return *this; } CounterStylePtr& operator=(AnonymousCounterStyle* aCounterStyle) { Reset(); if (aCounterStyle) { CounterStyle* raw = do_AddRef(aCounterStyle).take(); - AssertPointerAligned(raw); mRaw = reinterpret_cast(raw) | eAnonymousCounterStyle; } return *this; } - CounterStylePtr& operator=(CounterStyle* aCounterStyle) { - Reset(); - if (aCounterStyle) { - MOZ_ASSERT(!aCounterStyle->AsAnonymous()); - AssertPointerAligned(aCounterStyle); - mRaw = reinterpret_cast(aCounterStyle) | eCounterStyle; - } - return *this; - } - operator CounterStyle*() const& { return Get(); } - operator CounterStyle*() const&& = delete; - CounterStyle* operator->() const { return Get(); } explicit operator bool() const { return !!mRaw; } bool operator!() const { return !mRaw; } bool operator==(const CounterStylePtr& aOther) const { + // FIXME(emilio): For atoms this is all right, but for symbols doesn't this + // cause us to compare as unequal all the time, even if the specified + // symbols didn't change? return mRaw == aOther.mRaw; } bool operator!=(const CounterStylePtr& aOther) const { return mRaw != aOther.mRaw; } - bool IsResolved() const { return !IsUnresolved(); } - inline void Resolve(CounterStyleManager* aManager); - nsAtom* AsAtom() const { - MOZ_ASSERT(IsUnresolved()); + MOZ_ASSERT(IsAtom()); return reinterpret_cast(mRaw & ~eMask); } AnonymousCounterStyle* AsAnonymous() const { @@ -228,58 +217,49 @@ class CounterStylePtr { reinterpret_cast(mRaw & ~eMask)); } - private: - CounterStyle* Get() const { - MOZ_ASSERT(IsResolved()); - return reinterpret_cast(mRaw & ~eMask); - } - template - void AssertPointerAligned(T* aPointer) { - // This can be checked at compile time via - // > static_assert(alignof(CounterStyle) >= 4); - // > static_assert(alignof(nsAtom) >= 4); - // but MSVC2015 doesn't support using alignof on an abstract class. - // Once we move to MSVC2017, we can replace this runtime check with - // the compile time check above. - MOZ_ASSERT(!(reinterpret_cast(aPointer) & eMask)); - } + bool IsAtom() const { return GetType() == eAtom; } + bool IsAnonymous() const { return GetType() == eAnonymousCounterStyle; } + bool IsNone() const { return IsAtom() && AsAtom() == nsGkAtoms::none; } + + private: enum Type : uintptr_t { - eCounterStyle = 0, - eAnonymousCounterStyle = 1, - eUnresolvedAtom = 2, - eMask = 3, + eAnonymousCounterStyle = 0, + eAtom = 1, + eMask = 1, }; + static_assert(alignof(CounterStyle) >= 1 << eMask, + "We're gonna tag the pointer, so it better fit"); + static_assert(alignof(nsAtom) >= 1 << eMask, + "We're gonna tag the pointer, so it better fit"); + Type GetType() const { return static_cast(mRaw & eMask); } - bool IsUnresolved() const { return GetType() == eUnresolvedAtom; } - bool IsAnonymous() const { return GetType() == eAnonymousCounterStyle; } void Reset() { + if (!mRaw) { + return; + } switch (GetType()) { - case eCounterStyle: - break; case eAnonymousCounterStyle: AsAnonymous()->Release(); break; - case eUnresolvedAtom: + case eAtom: AsAtom()->Release(); break; - case eMask: + default: MOZ_ASSERT_UNREACHABLE("Unknown type"); break; } mRaw = 0; } - // mRaw contains the pointer, and its last two bits are used for type - // of the pointer. - // If the type is eUnresolvedAtom, the pointer owns a reference to an - // nsAtom, and it needs to be resolved to a counter style before use. + // mRaw contains the pointer, and its last bit is used to store the type of + // the pointer. + // If the type is eAtom, the pointer owns a reference to an nsAtom + // (potentially null). // If the type is eAnonymousCounterStyle, it owns a reference to an - // anonymous counter style. - // Otherwise it is a weak pointer referring a named counter style - // managed by CounterStyleManager. + // anonymous counter style (never null). uintptr_t mRaw; }; @@ -304,7 +284,13 @@ class CounterStyleManager final { } // Same as GetCounterStyle but try to build the counter style object // rather than returning nullptr if that hasn't been built. - CounterStyle* BuildCounterStyle(nsAtom* aName); + CounterStyle* ResolveCounterStyle(nsAtom* aName); + CounterStyle* ResolveCounterStyle(const CounterStylePtr& aPtr) { + if (aPtr.IsAtom()) { + return ResolveCounterStyle(aPtr.AsAtom()); + } + return aPtr.AsAnonymous(); + } static CounterStyle* GetBuiltinStyle(int32_t aStyle); static CounterStyle* GetNoneStyle() { @@ -317,8 +303,6 @@ class CounterStyleManager final { return GetBuiltinStyle(NS_STYLE_LIST_STYLE_DISC); } - static nsAtom* GetStyleNameFromType(int32_t aStyle); - // This method will scan all existing counter styles generated by this // manager, and remove or mark data dirty accordingly. It returns true // if any counter style is changed, false elsewise. This method should @@ -341,12 +325,6 @@ class CounterStyleManager final { nsTArray mRetiredStyles; }; -void CounterStylePtr::Resolve(CounterStyleManager* aManager) { - if (IsUnresolved()) { - *this = aManager->BuildCounterStyle(AsAtom()); - } -} - } // namespace mozilla #endif /* !defined(mozilla_CounterStyleManager_h_) */ diff --git a/layout/style/GeckoBindings.cpp b/layout/style/GeckoBindings.cpp index 6eb080925f396..9f4b6125f0bec 100644 --- a/layout/style/GeckoBindings.cpp +++ b/layout/style/GeckoBindings.cpp @@ -1124,15 +1124,8 @@ void Gecko_CopyAlternateValuesFrom(nsFont* aDest, const nsFont* aSrc) { void Gecko_SetCounterStyleToName(CounterStylePtr* aPtr, nsAtom* aName, RawGeckoPresContextBorrowed aPresContext) { - // Try resolving the counter style if possible, and keep it unresolved - // otherwise. - CounterStyleManager* manager = aPresContext->CounterStyleManager(); RefPtr name = already_AddRefed(aName); - if (CounterStyle* style = manager->GetCounterStyle(name)) { - *aPtr = style; - } else { - *aPtr = name.forget(); - } + *aPtr = name.forget(); } void Gecko_SetCounterStyleToSymbols(CounterStylePtr* aPtr, uint8_t aSymbolsType, @@ -1156,10 +1149,7 @@ void Gecko_CopyCounterStyle(CounterStylePtr* aDst, } nsAtom* Gecko_CounterStyle_GetName(const CounterStylePtr* aPtr) { - if (!aPtr->IsResolved()) { - return aPtr->AsAtom(); - } - return (*aPtr)->GetStyleName(); + return aPtr->IsAtom() ? aPtr->AsAtom() : nullptr; } const AnonymousCounterStyle* Gecko_CounterStyle_GetAnonymous( diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index 3732dc75e7abf..435833691d666 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -459,7 +459,7 @@ nsStyleList::nsStyleList(const Document& aDocument) MOZ_COUNT_CTOR(nsStyleList); MOZ_ASSERT(NS_IsMainThread()); - mCounterStyle = CounterStyleManager::GetDiscStyle(); + mCounterStyle = nsGkAtoms::disc; mQuotes = Servo_Quotes_GetInitialValue().Consume(); } @@ -482,7 +482,6 @@ void nsStyleList::FinishStyle(nsPresContext* aPresContext, mListStyleImage->Resolve( aPresContext, aOldStyle ? aOldStyle->mListStyleImage.get() : nullptr); } - mCounterStyle.Resolve(aPresContext->CounterStyleManager()); } nsChangeHint nsStyleList::CalcDifference( @@ -3637,24 +3636,15 @@ bool nsStyleContentData::operator==(const nsStyleContentData& aOther) const { void nsStyleContentData::Resolve(nsPresContext* aPresContext, const nsStyleContentData* aOldStyle) { - switch (mType) { - case StyleContentType::Image: - if (!mContent.mImage->IsResolved()) { - const nsStyleImageRequest* oldRequest = - (aOldStyle && aOldStyle->mType == StyleContentType::Image) - ? aOldStyle->mContent.mImage - : nullptr; - mContent.mImage->Resolve(aPresContext, oldRequest); - } - break; - case StyleContentType::Counter: - case StyleContentType::Counters: { - mContent.mCounters->mCounterStyle.Resolve( - aPresContext->CounterStyleManager()); - break; - } - default: - break; + if (mType != StyleContentType::Image) { + return; + } + if (!mContent.mImage->IsResolved()) { + const nsStyleImageRequest* oldRequest = + (aOldStyle && aOldStyle->mType == StyleContentType::Image) + ? aOldStyle->mContent.mImage + : nullptr; + mContent.mImage->Resolve(aPresContext, oldRequest); } } @@ -3688,25 +3678,9 @@ nsStyleContent::nsStyleContent(const nsStyleContent& aSource) nsChangeHint nsStyleContent::CalcDifference( const nsStyleContent& aNewData) const { - // In ElementRestyler::Restyle we assume that if there's no existing - // ::before or ::after and we don't have to restyle children of the - // node then we can't end up with a ::before or ::after due to the - // restyle of the node itself. That's not quite true, but the only - // exception to the above is when the 'content' property of the node - // changes and the pseudo-element inherits the changed value. Since - // the code here triggers a frame change on the node in that case, - // the optimization in ElementRestyler::Restyle is ok. But if we ever - // change this code to not reconstruct frames on changes to the - // 'content' property, then we will need to revisit the optimization - // in ElementRestyler::Restyle. - // Unfortunately we need to reframe even if the content lengths are the same; // a simple reflow will not pick up different text or different image URLs, // since we set all that up in the CSSFrameConstructor - // - // Also note that we also rely on this to return ReconstructFrame when - // content changes to ensure that nsCounterUseNode wouldn't reference - // to stale counter stylex. if (mContents != aNewData.mContents || mIncrements != aNewData.mIncrements || mResets != aNewData.mResets) { return nsChangeHint_ReconstructFrame; diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h index 5ac9ba714b514..48f66e4aa3993 100644 --- a/layout/style/nsStyleStruct.h +++ b/layout/style/nsStyleStruct.h @@ -2397,8 +2397,6 @@ class nsStyleContentData { CounterFunction* GetCounters() const { MOZ_ASSERT(mType == StyleContentType::Counter || mType == StyleContentType::Counters); - MOZ_ASSERT(mContent.mCounters->mCounterStyle.IsResolved(), - "Counter style should have been resolved"); return mContent.mCounters; }