Bug 1363596 part 1 - Merge nsCOMPtr<nsIAtom> into CounterStylePtr. r=heycam
MozReview-Commit-ID: D2fdlrC3yop
--- a/js/src/devtools/rootAnalysis/analyzeHeapWrites.js
+++ b/js/src/devtools/rootAnalysis/analyzeHeapWrites.js
@@ -86,16 +86,17 @@ function checkOverridableVirtualCall(ent
if (hasThreadsafeReferenceCounts(entry, /nsCOMPtr<T>::assign_with_AddRef.*?\[with T = (.*?)\]/))
return;
// Watch for raw addref/release.
var whitelist = [
"Gecko_AddRefAtom",
"Gecko_ReleaseAtom",
/nsPrincipal::Get/,
+ /CounterStylePtr::Reset/,
];
if (entry.matches(whitelist))
return;
dumpError(entry, location, "AddRef/Release on nsISupports");
}
function checkIndirectCall(entry, location, callee)
@@ -140,18 +141,18 @@ function treatAsSafeArgument(entry, varN
// RawGeckoBorrowedNode thread-mutable parameters.
["Gecko_SetNodeFlags", "aNode", null],
["Gecko_UnsetNodeFlags", "aNode", null],
// Various Servo binding out parameters. This is a mess and there needs
// to be a way to indicate which params are out parameters, either using
// an attribute or a naming convention.
["Gecko_CopyFontFamilyFrom", "dst", null],
- ["Gecko_SetListStyleType", "aList", null],
- ["Gecko_CopyListStyleTypeFrom", "aDst", null],
+ ["Gecko_SetCounterStyleToName", "aPtr", null],
+ ["Gecko_CopyCounterStyle", "aDst", null],
["Gecko_SetMozBinding", "aDisplay", null],
[/ClassOrClassList/, /aClass/, null],
["Gecko_GetAtomAsUTF16", "aLength", null],
["Gecko_CopyMozBindingFrom", "aDest", null],
["Gecko_SetNullImageValue", "aImage", null],
["Gecko_SetGradientImageValue", "aImage", null],
["Gecko_SetImageOrientation", "aVisibility", null],
["Gecko_SetImageOrientationAsFromImage", "aVisibility", null],
--- a/layout/style/CounterStyleManager.h
+++ b/layout/style/CounterStyleManager.h
@@ -145,18 +145,28 @@ private:
// managed by counter style manager.
class CounterStylePtr
{
public:
CounterStylePtr() : mRaw(0) {}
CounterStylePtr(const CounterStylePtr& aOther)
: mRaw(aOther.mRaw)
{
- if (IsAnonymous()) {
- AsAnonymous()->AddRef();
+ switch (GetType()) {
+ case eCounterStyle:
+ break;
+ case eAnonymousCounterStyle:
+ AsAnonymous()->AddRef();
+ break;
+ case eUnresolvedAtom:
+ AsAtom()->AddRef();
+ break;
+ case eMask:
+ MOZ_ASSERT_UNREACHABLE("Unknown type");
+ break;
}
}
CounterStylePtr(CounterStylePtr&& aOther)
: mRaw(aOther.mRaw)
{
aOther.mRaw = 0;
}
~CounterStylePtr() { Reset(); }
@@ -164,88 +174,139 @@ public:
CounterStylePtr& operator=(const CounterStylePtr& aOther)
{
if (this != &aOther) {
Reset();
new (this) CounterStylePtr(aOther);
}
return *this;
}
+ CounterStylePtr& operator=(CounterStylePtr&& aOther)
+ {
+ if (this != &aOther) {
+ Reset();
+ mRaw = aOther.mRaw;
+ aOther.mRaw = 0;
+ }
+ return *this;
+ }
CounterStylePtr& operator=(decltype(nullptr))
{
Reset();
return *this;
}
+ CounterStylePtr& operator=(already_AddRefed<nsIAtom> aAtom)
+ {
+ Reset();
+ if (nsIAtom* raw = aAtom.take()) {
+ AssertPointerAligned(raw);
+ mRaw = reinterpret_cast<uintptr_t>(raw) | eUnresolvedAtom;
+ }
+ return *this;
+ }
CounterStylePtr& operator=(AnonymousCounterStyle* aCounterStyle)
{
Reset();
if (aCounterStyle) {
CounterStyle* raw = do_AddRef(aCounterStyle).take();
AssertPointerAligned(raw);
- mRaw = reinterpret_cast<uintptr_t>(raw) | kAnonymousFlag;
+ mRaw = reinterpret_cast<uintptr_t>(raw) | eAnonymousCounterStyle;
}
return *this;
}
CounterStylePtr& operator=(CounterStyle* aCounterStyle)
{
Reset();
if (aCounterStyle) {
MOZ_ASSERT(!aCounterStyle->AsAnonymous());
AssertPointerAligned(aCounterStyle);
- mRaw = reinterpret_cast<uintptr_t>(aCounterStyle);
+ mRaw = reinterpret_cast<uintptr_t>(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
{ return mRaw == aOther.mRaw; }
bool operator!=(const CounterStylePtr& aOther) const
{ return mRaw != aOther.mRaw; }
+ bool IsResolved() const { return !IsUnresolved(); }
+ inline void Resolve(CounterStyleManager* aManager);
+
private:
CounterStyle* Get() const
{
- return reinterpret_cast<CounterStyle*>(mRaw & ~kAnonymousFlag);
+ MOZ_ASSERT(IsResolved());
+ return reinterpret_cast<CounterStyle*>(mRaw & ~eMask);
}
- void AssertPointerAligned(CounterStyle* aPointer)
+ template<typename T>
+ void AssertPointerAligned(T* aPointer)
{
// This can be checked at compile time via
- // > static_assert(alignof(CounterStyle) >= 2);
+ // > static_assert(alignof(CounterStyle) >= 4);
+ // > static_assert(alignof(nsIAtom) >= 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<uintptr_t>(aPointer) & kAnonymousFlag));
+ MOZ_ASSERT(!(reinterpret_cast<uintptr_t>(aPointer) & eMask));
}
- bool IsAnonymous() const { return !!(mRaw & kAnonymousFlag); }
+ enum Type : uintptr_t {
+ eCounterStyle = 0,
+ eAnonymousCounterStyle = 1,
+ eUnresolvedAtom = 2,
+ eMask = 3,
+ };
+
+ Type GetType() const { return static_cast<Type>(mRaw & eMask); }
+ bool IsUnresolved() const { return GetType() == eUnresolvedAtom; }
+ bool IsAnonymous() const { return GetType() == eAnonymousCounterStyle; }
+ nsIAtom* AsAtom()
+ {
+ MOZ_ASSERT(IsUnresolved());
+ return reinterpret_cast<nsIAtom*>(mRaw & ~eMask);
+ }
AnonymousCounterStyle* AsAnonymous()
{
MOZ_ASSERT(IsAnonymous());
return static_cast<AnonymousCounterStyle*>(
- reinterpret_cast<CounterStyle*>(mRaw & ~kAnonymousFlag));
+ reinterpret_cast<CounterStyle*>(mRaw & ~eMask));
}
void Reset()
{
- if (IsAnonymous()) {
- AsAnonymous()->Release();
+ switch (GetType()) {
+ case eCounterStyle:
+ break;
+ case eAnonymousCounterStyle:
+ AsAnonymous()->Release();
+ break;
+ case eUnresolvedAtom:
+ AsAtom()->Release();
+ break;
+ case eMask:
+ MOZ_ASSERT_UNREACHABLE("Unknown type");
+ break;
}
mRaw = 0;
}
- // mRaw contains the pointer, and its last bit is used for the flag.
- // If the flag is set, this pointer owns an AnonymousCounterStyle,
- // otherwise, it is a weak pointer referring a named counter style
+ // 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
+ // nsIAtom, and it needs to be resolved to a counter style before use.
+ // 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.
- static const uintptr_t kAnonymousFlag = 1;
uintptr_t mRaw;
};
class CounterStyleManager final
{
private:
~CounterStyleManager();
public:
@@ -290,11 +351,19 @@ public:
private:
void DestroyCounterStyle(CounterStyle* aCounterStyle);
nsPresContext* mPresContext;
nsDataHashtable<nsRefPtrHashKey<nsIAtom>, CounterStyle*> mStyles;
nsTArray<CounterStyle*> mRetiredStyles;
};
+void
+CounterStylePtr::Resolve(CounterStyleManager* aManager)
+{
+ if (IsUnresolved()) {
+ *this = aManager->BuildCounterStyle(AsAtom());
+ }
+}
+
} // namespace mozilla
#endif /* !defined(mozilla_CounterStyleManager_h_) */
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -1154,25 +1154,25 @@ Gecko_SetImageOrientationAsFromImage(nsS
void
Gecko_CopyImageOrientationFrom(nsStyleVisibility* aDst,
const nsStyleVisibility* aSrc)
{
aDst->mImageOrientation = aSrc->mImageOrientation;
}
void
-Gecko_SetListStyleType(nsStyleList* aList, nsIAtom* aName)
+Gecko_SetCounterStyleToName(CounterStylePtr* aPtr, nsIAtom* aName)
{
- aList->SetListStyleType(aName);
+ *aPtr = already_AddRefed<nsIAtom>(aName);
}
void
-Gecko_CopyListStyleTypeFrom(nsStyleList* aDst, const nsStyleList* aSrc)
+Gecko_CopyCounterStyle(CounterStylePtr* aDst, const CounterStylePtr* aSrc)
{
- aDst->CopyListStyleTypeFrom(*aSrc);
+ *aDst = *aSrc;
}
already_AddRefed<css::URLValue>
ServoBundledURI::IntoCssUrl()
{
if (!mURLString) {
return nullptr;
}
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -269,18 +269,20 @@ void Gecko_nsFont_Destroy(nsFont* dst);
void Gecko_SetImageOrientation(nsStyleVisibility* aVisibility,
double aRadians,
bool aFlip);
void Gecko_SetImageOrientationAsFromImage(nsStyleVisibility* aVisibility);
void Gecko_CopyImageOrientationFrom(nsStyleVisibility* aDst,
const nsStyleVisibility* aSrc);
// Counter style.
-void Gecko_SetListStyleType(nsStyleList* style_struct, nsIAtom* name);
-void Gecko_CopyListStyleTypeFrom(nsStyleList* dst, const nsStyleList* src);
+// This function takes an already addrefed nsIAtom
+void Gecko_SetCounterStyleToName(mozilla::CounterStylePtr* ptr, nsIAtom* name);
+void Gecko_CopyCounterStyle(mozilla::CounterStylePtr* dst,
+ const mozilla::CounterStylePtr* src);
// background-image style.
void Gecko_SetNullImageValue(nsStyleImage* image);
void Gecko_SetGradientImageValue(nsStyleImage* image, nsStyleGradient* gradient);
NS_DECL_THREADSAFE_FFI_REFCOUNTING(mozilla::css::ImageValue, ImageValue);
mozilla::css::ImageValue* Gecko_ImageValue_Create(ServoBundledURI aURI);
void Gecko_SetLayerImageImageValue(nsStyleImage* image,
mozilla::css::ImageValue* aImageValue);
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -339,16 +339,17 @@ structs-types = [
"RawGeckoServoStyleRuleList",
"RawGeckoURLExtraData",
"RefPtr",
"CSSPseudoClassType",
"CSSPseudoElementType",
"TraversalRestyleBehavior",
"TraversalRootBehavior",
"ComputedTimingFunction_BeforeFlag",
+ "CounterStylePtr",
"FontFamilyList",
"FontFamilyType",
"FontSizePrefs",
"GeckoFontMetrics",
"IterationCompositeOperation",
"Keyframe",
"ServoBundledURI",
"ServoElementSnapshot",
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -598,55 +598,49 @@ nsStyleOutline::CalcDifference(const nsS
// --------------------
// nsStyleList
//
nsStyleList::nsStyleList(const nsPresContext* aContext)
: mListStylePosition(NS_STYLE_LIST_STYLE_POSITION_OUTSIDE)
{
MOZ_COUNT_CTOR(nsStyleList);
if (aContext->StyleSet()->IsServo()) {
- mListStyleType = nsGkAtoms::disc;
+ mCounterStyle = do_AddRef(nsGkAtoms::disc);
} else {
mCounterStyle = aContext->
CounterStyleManager()->BuildCounterStyle(nsGkAtoms::disc);
}
SetQuotesInitial();
}
nsStyleList::~nsStyleList()
{
MOZ_COUNT_DTOR(nsStyleList);
}
nsStyleList::nsStyleList(const nsStyleList& aSource)
: mListStylePosition(aSource.mListStylePosition)
, mListStyleImage(aSource.mListStyleImage)
- , mListStyleType(aSource.mListStyleType)
, mCounterStyle(aSource.mCounterStyle)
, mQuotes(aSource.mQuotes)
, mImageRegion(aSource.mImageRegion)
{
MOZ_COUNT_CTOR(nsStyleList);
}
void
nsStyleList::FinishStyle(nsPresContext* aPresContext)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aPresContext->StyleSet()->IsServo());
if (mListStyleImage && !mListStyleImage->IsResolved()) {
mListStyleImage->Resolve(aPresContext);
}
- if (mListStyleType) {
- MOZ_ASSERT(!mCounterStyle);
- mCounterStyle = aPresContext->
- CounterStyleManager()->BuildCounterStyle(mListStyleType);
- mListStyleType = nullptr;
- }
+ mCounterStyle.Resolve(aPresContext->CounterStyleManager());
}
void
nsStyleList::SetQuotesInherit(const nsStyleList* aOther)
{
mQuotes = aOther->mQuotes;
}
@@ -704,18 +698,17 @@ nsStyleList::CalcDifference(const nsStyl
(mQuotes || aNewData.mQuotes) &&
GetQuotePairs() != aNewData.GetQuotePairs()) {
return nsChangeHint_ReconstructFrame;
}
if (mListStylePosition != aNewData.mListStylePosition) {
return nsChangeHint_ReconstructFrame;
}
if (DefinitelyEqualImages(mListStyleImage, aNewData.mListStyleImage) &&
- (mCounterStyle == aNewData.mCounterStyle ||
- mListStyleType != aNewData.mListStyleType)) {
+ mCounterStyle == aNewData.mCounterStyle) {
if (mImageRegion.IsEqualInterior(aNewData.mImageRegion)) {
return nsChangeHint(0);
}
if (mImageRegion.width == aNewData.mImageRegion.width &&
mImageRegion.height == aNewData.mImageRegion.height) {
return NS_STYLE_HINT_VISUAL;
}
}
@@ -3792,18 +3785,17 @@ nsStyleContentData::nsStyleContentData(c
}
bool
nsStyleContentData::
CounterFunction::operator==(const CounterFunction& aOther) const
{
return mIdent == aOther.mIdent &&
mSeparator == aOther.mSeparator &&
- mCounterStyle == aOther.mCounterStyle &&
- mCounterStyleName == aOther.mCounterStyleName;
+ mCounterStyle == aOther.mCounterStyle;
}
nsStyleContentData&
nsStyleContentData::operator=(const nsStyleContentData& aOther)
{
if (this == &aOther) {
return *this;
}
@@ -3835,23 +3827,18 @@ nsStyleContentData::Resolve(nsPresContex
switch (mType) {
case eStyleContentType_Image:
if (!mContent.mImage->IsResolved()) {
mContent.mImage->Resolve(aPresContext);
}
break;
case eStyleContentType_Counter:
case eStyleContentType_Counters: {
- CounterFunction* counters = mContent.mCounters;
- if (counters->mCounterStyleName) {
- MOZ_ASSERT(!counters->mCounterStyle);
- counters->mCounterStyle = aPresContext->CounterStyleManager()->
- BuildCounterStyle(counters->mCounterStyleName);
- counters->mCounterStyleName = nullptr;
- }
+ mContent.mCounters->
+ mCounterStyle.Resolve(aPresContext->CounterStyleManager());
break;
}
default:
break;
}
}
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -1463,45 +1463,26 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
imgRequestProxy* GetListStyleImage() const
{
return mListStyleImage ? mListStyleImage->get() : nullptr;
}
already_AddRefed<nsIURI> GetListStyleImageURI() const;
- // The following two methods are called from Servo code to maintain
- // list-style-type off main thread.
- void SetListStyleType(nsIAtom* aType)
- {
- mListStyleType = aType;
- mCounterStyle = nullptr;
- }
- void CopyListStyleTypeFrom(const nsStyleList& aOther)
- {
- mListStyleType = aOther.mListStyleType;
- mCounterStyle = aOther.mCounterStyle;
- }
-
const nsStyleQuoteValues::QuotePairArray& GetQuotePairs() const;
void SetQuotesInherit(const nsStyleList* aOther);
void SetQuotesInitial();
void SetQuotesNone();
void SetQuotes(nsStyleQuoteValues::QuotePairArray&& aValues);
uint8_t mListStylePosition; // [inherited]
RefPtr<nsStyleImageRequest> mListStyleImage; // [inherited]
- // mCounterStyle is the actual field for computed list-style-type.
- // mListStyleType is only used when we are off the main thread, so we
- // cannot safely construct CounterStyle object. FinishStyle() will
- // use it to setup mCounterStyle and then clear it. At any time, only
- // one of the following two fields should be non-null.
- nsCOMPtr<nsIAtom> mListStyleType;
mozilla::CounterStylePtr mCounterStyle; // [inherited]
private:
RefPtr<nsStyleQuoteValues> mQuotes; // [inherited]
nsStyleList& operator=(const nsStyleList& aOther) = delete;
public:
nsRect mImageRegion; // [inherited] the rect to use within an image
@@ -3043,36 +3024,33 @@ public:
return mContent.mString;
}
struct CounterFunction
{
nsString mIdent;
// This is only used when it is a counters() function.
nsString mSeparator;
- // One and only one of mCounterStyle and mCounterStyleName must be
- // non-null at any time.
mozilla::CounterStylePtr mCounterStyle;
- nsCOMPtr<nsIAtom> mCounterStyleName;
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CounterFunction)
bool operator==(const CounterFunction& aOther) const;
bool operator!=(const CounterFunction& aOther) const {
return !(*this == aOther);
}
private:
~CounterFunction() {}
};
CounterFunction* GetCounters() const
{
MOZ_ASSERT(mType == eStyleContentType_Counter ||
mType == eStyleContentType_Counters);
- MOZ_ASSERT(mContent.mCounters->mCounterStyle,
+ MOZ_ASSERT(mContent.mCounters->mCounterStyle.IsResolved(),
"Counter style should have been resolved");
return mContent.mCounters;
}
nsStyleImageRequest* GetImageRequest() const
{
MOZ_ASSERT(mType == eStyleContentType_Image);
return mContent.mImage;