Closed
Bug 1290218
Opened 8 years ago
Closed 7 years ago
stylo: Cache servo stylesheets
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: bholley, Assigned: bradwerth)
References
Details
Attachments
(13 files, 8 obsolete files)
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
In css:Loader::DoSheetComplete, we skip the caching logic in the servo case. This presumably has a negative impact on performance and memory usage.
Updated•8 years ago
|
Priority: -- → P3
Comment 1•7 years ago
|
||
If there is anything we can do to support a global parsed stylesheet cache (bug 219276) that would be nice. It has some significant wins in e10s mode when navigating around within a site.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8832710 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8832711 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8835111 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8835112 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8832212 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8832708 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8835113 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8832213 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8835174 -
Flags: review?(cam)
Attachment #8835175 -
Flags: review?(cam)
Attachment #8835176 -
Flags: review?(cam)
Attachment #8835177 -
Flags: review?(cam)
Attachment #8835178 -
Flags: review?(cam)
Attachment #8835179 -
Flags: review?(cam)
Attachment #8835180 -
Flags: review?(cam)
Attachment #8835181 -
Flags: review?(cam)
Attachment #8835182 -
Flags: review?(cam)
Attachment #8832709 -
Flags: review?(cam)
Comment 40•7 years ago
|
||
mozreview-review |
Comment on attachment 8835174 [details] Bug 1290218 Part 1: Uplift IsModified into StyleSheet from CSSStyleSheet, and provide empty implementation for ServoStyleSheet. https://reviewboard.mozilla.org/r/110856/#review113258 ::: layout/style/CSSStyleSheet.h:138 (Diff revision 2) > already_AddRefed<CSSStyleSheet> Clone(CSSStyleSheet* aCloneParent, > css::ImportRule* aCloneOwnerRule, > nsIDocument* aCloneDocument, > nsINode* aCloneOwningNode) const; > > - bool IsModified() const { return mDirty; } > + bool IsModified() const final override { return mDirty; } https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style says this should just be "final" rather than "final override". ::: layout/style/ServoStyleSheet.h:71 (Diff revision 2) > css::Rule* GetDOMOwnerRule() const final; > > void WillDirty() {} > void DidDirty() {} > > + bool IsModified() const final override { return false; } Here too.
Attachment #8835174 -
Flags: review?(cam) → review+
Comment 41•7 years ago
|
||
mozreview-review |
Comment on attachment 8835175 [details] Bug 1290218 Part 2: Uplift mInner pointer from CSSStyleSheetInner into StyleSheetInfo. https://reviewboard.mozilla.org/r/110858/#review113266 r=me with these addressed. ::: layout/style/CSSStyleSheet.h:132 (Diff revision 2) > css::ImportRule* GetOwnerRule() const { return mOwnerRule; } > // Workaround overloaded-virtual warning in GCC. > using StyleSheet::GetOwnerRule; > > - nsXMLNameSpaceMap* GetNameSpaceMap() const { return mInner->mNameSpaceMap; } > + nsXMLNameSpaceMap* GetNameSpaceMap() const { > + return static_cast<CSSStyleSheetInner*>(mInner)->mNameSpaceMap; Instead of having static_asserts throughout various methods on CSSStyleSheet, how about we add a (private) method CSSStyleSheetInner* Inner() { return static_cast<CSSStyleSheetInner*>(mInner); } so we can avoid repeating it? Then it'll be clear enough not to have a separate |inner| local variable where we need it, and we can just operate directly on Inner(). ::: layout/style/CSSStyleSheet.cpp (Diff revision 2) > - mInner(aCopy.mInner), > mRuleProcessors(nullptr) > { > mParent = aParentToUse; > + mInner = aCopy.mInner; Any reason we have to do this? Probably better to keep it in the initializer list, if possible. ::: layout/style/StyleSheet.h:262 (Diff revision 2) > // the sense that if it's known-live then we're known-live). Always > // NotOwnedByDocument when mDocument is null. > DocumentAssociationMode mDocumentAssociationMode; > > + // Core information we get from parsed sheets, which are shared amongst > + // Stylesheet clones. Nit: "StyleSheet". ::: layout/style/StyleSheet.cpp:46 (Diff revision 2) > , mType(aCopy.mType) > , mDisabled(aCopy.mDisabled) > // We only use this constructor during cloning. It's the cloner's > // responsibility to notify us if we end up being owned by a document. > , mDocumentAssociationMode(NotOwnedByDocument) > + , mInner(nullptr) It might make sense to copy the mInner pointer here, and just leave the AddSheet call in CSSStyleSheet instead. If not, then let's add a comment in here pointing out that we copy mInner in the concrete class.
Attachment #8835175 -
Flags: review?(cam) → review+
Comment 42•7 years ago
|
||
mozreview-review |
Comment on attachment 8835175 [details] Bug 1290218 Part 2: Uplift mInner pointer from CSSStyleSheetInner into StyleSheetInfo. https://reviewboard.mozilla.org/r/110858/#review113282 ::: layout/style/CSSStyleSheet.cpp (Diff revision 2) > - mInner(aCopy.mInner), > mRuleProcessors(nullptr) > { > mParent = aParentToUse; > + mInner = aCopy.mInner; I realize of course that it's because it's on the superclass now.
Comment 43•7 years ago
|
||
mozreview-review |
Comment on attachment 8835176 [details] Bug 1290218 Part 3: Transition ServoStyleSheet to use mInner pointer, instead of maintaining a static StyleSheetInfo. https://reviewboard.mozilla.org/r/110860/#review113284
Attachment #8835176 -
Flags: review?(cam) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 54•7 years ago
|
||
mozreview-review |
Comment on attachment 8835175 [details] Bug 1290218 Part 2: Uplift mInner pointer from CSSStyleSheetInner into StyleSheetInfo. https://reviewboard.mozilla.org/r/110858/#review113540 ::: layout/style/CSSStyleSheet.h:132 (Diff revision 3) > css::ImportRule* GetOwnerRule() const { return mOwnerRule; } > // Workaround overloaded-virtual warning in GCC. > using StyleSheet::GetOwnerRule; > > - nsXMLNameSpaceMap* GetNameSpaceMap() const { return mInner->mNameSpaceMap; } > + nsXMLNameSpaceMap* GetNameSpaceMap() const { > + return static_cast<CSSStyleSheetInner*>(mInner)->mNameSpaceMap; This one can use Inner() too. ::: layout/style/nsCSSRuleProcessor.cpp:3676 (Diff revision 3) > - if (!aSheet->mInner->mOrderedRules.EnumerateForwards(CascadeRuleEnumFunc, > + CSSStyleSheetInner* inner = static_cast<CSSStyleSheetInner*>(aSheet->mInner); > + if (!inner->mOrderedRules.EnumerateForwards(CascadeRuleEnumFunc, Use aSheet->Inner() here too.
Comment 55•7 years ago
|
||
mozreview-review |
Comment on attachment 8835177 [details] Bug 1290218 Part 4: Implement shared mInners for ServoStyleSheets, and standardize calling of AddSheet into CSSStyleSheet and ServoStyleSheet constructors. https://reviewboard.mozilla.org/r/110862/#review113550 This generally looks good but a couple of things need fixing up. ::: layout/style/CSSStyleSheet.cpp:265 (Diff revision 3) > - mSheets.AppendElement(aSheet); > -} > + // Get some information now that we'll use if we survive the superclass > + // implementation of RemoveSheet. > + bool removingFirstOfSeveral = (aSheet == mSheets.ElementAt(0)) && > + (mSheets.Length() > 1); > > -void > -CSSStyleSheetInner::RemoveSheet(CSSStyleSheet* aSheet) > -{ > + StyleSheetInfo::RemoveSheet(aSheet); > + > + if (removingFirstOfSeveral) { I'm uneasy about duplicating the logic here of when RemoveSheet will end up deleting our object, and then doing work on the object after this call. What do you think about structuring the function like this: if (mSheets[0] == aSheet && mSheets.Length() > 1) { mOrderedRules.EnumerateForwards(..., mSheets[1]); ...::ReparentChildList(mSheets[1], ...); } StyleSheetInfo::RemoveSheet(aSheet); // WARNING: RemoveSheet might delete this object! ::: layout/style/CSSStyleSheet.cpp:398 (Diff revision 3) > mInRuleProcessorCache(false), > mScopeElement(nullptr), > mRuleProcessors(nullptr) > { > + MOZ_ASSERT(mInner, "We should have an mInner after copy."); > + MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us."); Is this assertion correct? I think we'll do a shallow copy of the mInner pointer up in the StyleSheet and do the AddSheet() call in the StyleSheet copy constructor. ::: layout/style/CSSStyleSheet.cpp:402 (Diff revision 3) > + MOZ_ASSERT(mInner, "We should have an mInner after copy."); > + MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us."); > + > mParent = aParentToUse; > > Inner()->AddSheet(this); This shouldn't be needed if we do the AddSheet in the StyleSheet copy constructor now. ::: layout/style/StyleSheet.cpp:167 (Diff revision 3) > +StyleSheetInfo::StyleSheetInfo(StyleSheetInfo& aCopy, > + StyleSheet* aPrimarySheet) > + : mSheetURI(aCopy.mSheetURI) > + , mOriginalSheetURI(aCopy.mOriginalSheetURI) > + , mBaseURI(aCopy.mBaseURI) > + , mPrincipal(aCopy.mPrincipal) > + , mCORSMode(aCopy.mCORSMode) > + , mReferrerPolicy(aCopy.mReferrerPolicy) > + , mIntegrity(aCopy.mIntegrity) > + , mComplete(aCopy.mComplete) > + , mFirstChild() // We don't rebuild the child because we're making a copy > + // without children. > +{ We should #ifdef DEBUG copy over mPrincipalSet too. ::: layout/style/StyleSheetInfo.h:60 (Diff revision 3) > #ifdef DEBUG > bool mPrincipalSet; > #endif > + > + AutoTArray<StyleSheet*, 8> mSheets; Nit: This is not too important, but if there's no other reason I'd keep the DEBUG-only bool at the end, so that mSheets is in the same position in the object in both debug and release builds.
Attachment #8835177 -
Flags: review?(cam) → review-
Comment 56•7 years ago
|
||
mozreview-review |
Comment on attachment 8835178 [details] Bug 1290218 Part 5: Subclass StyleSheetInfo to move the ServoStyleSheet raw sheet into its mInner. https://reviewboard.mozilla.org/r/110864/#review113558 r=me with that. ::: layout/style/ServoStyleSheet.h:70 (Diff revision 3) > - RawServoStyleSheet* RawSheet() const { return mSheet; } > + RawServoStyleSheet* RawSheet() const { > + ServoStyleSheetInner* inner = static_cast<ServoStyleSheetInner*>(mInner); > + return inner->mSheet; > + } > void SetSheetForImport(RawServoStyleSheet* aSheet) { > - MOZ_ASSERT(!mSheet); > + ServoStyleSheetInner* inner = static_cast<ServoStyleSheetInner*>(mInner); Here too I would have a private Inner() function that does the cast (and use it throughout the .cpp file).
Attachment #8835178 -
Flags: review?(cam) → review+
Comment 57•7 years ago
|
||
mozreview-review |
Comment on attachment 8835179 [details] Bug 1290218 Part 6: Generalize nsXULPrototypeCache to hold StyleSheet instead of CSSStyleSheet. https://reviewboard.mozilla.org/r/110866/#review113570
Attachment #8835179 -
Flags: review?(cam) → review+
Comment 58•7 years ago
|
||
mozreview-review |
Comment on attachment 8835180 [details] Bug 1290218 Part 7: Bypass XULPrototypeCache (which is a singleton) for non-Gecko Loaders. https://reviewboard.mozilla.org/r/110868/#review113564 r=me with that. ::: layout/style/Loader.cpp:1117 (Diff revision 3) > - if (IsChromeURI(aURI)) { > + // The XUL cache is a singleton that only holds Gecko-style sheets, so > + // only use the cache if the loader is also Gecko. > + if (IsChromeURI(aURI) && > + mStyleBackendType.isSome() && > + mStyleBackendType.value() == StyleBackendType::Gecko) { Can you file a followup bug to allow ServoStyleSheets to be stored in the nsXULPrototypeCache too? (It is probably important for performance.) I guess we can just have two hash tables for the two types of sheets. ::: layout/style/Loader.cpp:1120 (Diff revision 3) > + mStyleBackendType.isSome() && > + mStyleBackendType.value() == StyleBackendType::Gecko) { Use GetStyleBackendType() instead (which will call through to mDocument to find the type, when mStyleBackendType is Nothing). ::: layout/style/Loader.cpp:1966 (Diff revision 3) > + mStyleBackendType.isSome() && > + mStyleBackendType.value() == StyleBackendType::Gecko) { Same here. ::: layout/style/Loader.cpp:1966 (Diff revision 3) > + mStyleBackendType.isSome() && > + mStyleBackendType.value() == StyleBackendType::Gecko) { Here too.
Attachment #8835180 -
Flags: review?(cam) → review+
Comment 59•7 years ago
|
||
mozreview-review |
Comment on attachment 8835181 [details] Bug 1290218 Part 8: Implement ServoStyleSheet Clone. https://reviewboard.mozilla.org/r/110870/#review113572 r=me with this. ::: layout/style/ServoStyleSheet.h:24 (Diff revision 3) > class ServoCSSRuleList; > > namespace css { > class Loader; > class Rule; > +class ImportRule; Nit: two lines up to keep this (short) list sorted. ::: layout/style/ServoStyleSheet.h:92 (Diff revision 3) > + already_AddRefed<ServoStyleSheet> Clone(ServoStyleSheet* aCloneParent, > + css::ImportRule* aCloneOwnerRule, > + nsIDocument* aCloneDocument, > + nsINode* aCloneOwningNode) const; Should we just make Clone() virtual (and pure virtual on StyleSheet) and return already_AddRefed<StyleSheet>? That would make the calls to it simpler in CreateSheet in the next patch.
Attachment #8835181 -
Flags: review?(cam) → review+
Comment 60•7 years ago
|
||
mozreview-review |
Comment on attachment 8835182 [details] Bug 1290218 Part 9: Generalize stylesheet Loader to cache Servo sheets in the same way that it caches Gecko sheets. https://reviewboard.mozilla.org/r/110872/#review113574
Attachment #8835182 -
Flags: review?(cam) → review+
Comment 61•7 years ago
|
||
mozreview-review |
Comment on attachment 8832709 [details] Bug 1290218 Part 10: Add asserts to ServoStyleSets. https://reviewboard.mozilla.org/r/108900/#review113578
Attachment #8832709 -
Flags: review?(cam) → review+
Comment 62•7 years ago
|
||
Please also file a followup bug to handle CSSOM sheet modifications, doing something like the mDirty handling on CSSStyleSheet, otherwise we can end up having two sheets sharing an inner, and using CSSOM to modify one will end up modifying both.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 74•7 years ago
|
||
mozreview-review |
Comment on attachment 8835181 [details] Bug 1290218 Part 8: Implement ServoStyleSheet Clone. https://reviewboard.mozilla.org/r/110870/#review114240 ::: layout/style/ServoStyleSheet.h:92 (Diff revision 3) > + already_AddRefed<ServoStyleSheet> Clone(ServoStyleSheet* aCloneParent, > + css::ImportRule* aCloneOwnerRule, > + nsIDocument* aCloneDocument, > + nsINode* aCloneOwningNode) const; Did this, and it led to several new cases of new static_casts. I think the trade off is still worth it.
Assignee | ||
Comment 75•7 years ago
|
||
mozreview-review |
Comment on attachment 8835177 [details] Bug 1290218 Part 4: Implement shared mInners for ServoStyleSheets, and standardize calling of AddSheet into CSSStyleSheet and ServoStyleSheet constructors. https://reviewboard.mozilla.org/r/110862/#review113800 ::: layout/style/CSSStyleSheet.cpp:265 (Diff revision 3) > - mSheets.AppendElement(aSheet); > -} > + // Get some information now that we'll use if we survive the superclass > + // implementation of RemoveSheet. > + bool removingFirstOfSeveral = (aSheet == mSheets.ElementAt(0)) && > + (mSheets.Length() > 1); > > -void > -CSSStyleSheetInner::RemoveSheet(CSSStyleSheet* aSheet) > -{ > + StyleSheetInfo::RemoveSheet(aSheet); > + > + if (removingFirstOfSeveral) { I'll do this. I was being sensitive about reversing the order of operations between the reparenting and the removal of aSheet from mSheets. But I'll ensure that the code will still work with the operations in this order, and structure the code in the clearer way you propose. ::: layout/style/CSSStyleSheet.cpp:398 (Diff revision 3) > mInRuleProcessorCache(false), > mScopeElement(nullptr), > mRuleProcessors(nullptr) > { > + MOZ_ASSERT(mInner, "We should have an mInner after copy."); > + MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us."); At this point, the StyleSheet copy constructor has been called, and it calls AddSheet, so this assertion holds. ::: layout/style/CSSStyleSheet.cpp:402 (Diff revision 3) > + MOZ_ASSERT(mInner, "We should have an mInner after copy."); > + MOZ_ASSERT(mInner->mSheets.Contains(this), "Our mInner should include us."); > + > mParent = aParentToUse; > > Inner()->AddSheet(this); Yes, this was an erroneous carryover. AddSheet should stay in the StyleSheet copy constructor, so we don't have to override it in subclasses just to get this behavior. ::: layout/style/StyleSheetInfo.h:60 (Diff revision 3) > #ifdef DEBUG > bool mPrincipalSet; > #endif > + > + AutoTArray<StyleSheet*, 8> mSheets; Good idea.
Assignee | ||
Comment 76•7 years ago
|
||
mozreview-review |
Comment on attachment 8835175 [details] Bug 1290218 Part 2: Uplift mInner pointer from CSSStyleSheetInner into StyleSheetInfo. https://reviewboard.mozilla.org/r/110858/#review113394 ::: layout/style/CSSStyleSheet.h:132 (Diff revision 2) > css::ImportRule* GetOwnerRule() const { return mOwnerRule; } > // Workaround overloaded-virtual warning in GCC. > using StyleSheet::GetOwnerRule; > > - nsXMLNameSpaceMap* GetNameSpaceMap() const { return mInner->mNameSpaceMap; } > + nsXMLNameSpaceMap* GetNameSpaceMap() const { > + return static_cast<CSSStyleSheetInner*>(mInner)->mNameSpaceMap; Much better solution; thank you for the suggestion.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8837877 -
Flags: review?(cam)
Comment 88•7 years ago
|
||
mozreview-review |
Comment on attachment 8835177 [details] Bug 1290218 Part 4: Implement shared mInners for ServoStyleSheets, and standardize calling of AddSheet into CSSStyleSheet and ServoStyleSheet constructors. https://reviewboard.mozilla.org/r/110862/#review114756 ::: layout/style/CSSStyleSheet.cpp:265 (Diff revisions 3 - 5) > - // Get some information now that we'll use if we survive the superclass > + if ((aSheet == mSheets.ElementAt(0)) && (mSheets.Length() > 1)) { > - // implementation of RemoveSheet. > - bool removingFirstOfSeveral = (aSheet == mSheets.ElementAt(0)) && > - (mSheets.Length() > 1); > - > - StyleSheetInfo::RemoveSheet(aSheet); > - > - if (removingFirstOfSeveral) { > mOrderedRules.EnumerateForwards(SetStyleSheetReference, > - mSheets.ElementAt(0)); > + mSheets.ElementAt(1)); > > - ChildSheetListBuilder::ReparentChildList(mSheets[0], mFirstChild); > + ChildSheetListBuilder::ReparentChildList(mSheets[1], mFirstChild); > } Nit: if you like you can standardize on using [] instead of ElementAt(). (Probably this array used to be an nsCOMArray, which used not to have an operator[].)
Attachment #8835177 -
Flags: review?(cam) → review+
Comment 89•7 years ago
|
||
mozreview-review |
Comment on attachment 8837877 [details] Bug 1290218 Part 4b: Fixup cycle collector declarations for mInner moving into StyleSheetInfo. https://reviewboard.mozilla.org/r/112872/#review114758
Attachment #8837877 -
Flags: review?(cam) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 101•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=68e662ec0369
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8839715 -
Flags: review?(cam)
Comment 114•7 years ago
|
||
mozreview-review |
Comment on attachment 8839715 [details] Bug 1290218 Part 11: Update mochitest failure count, and mark a reftest as failing. https://reviewboard.mozilla.org/r/114292/#review115766
Attachment #8839715 -
Flags: review?(cam) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8840690 -
Flags: review?(cam)
Comment 140•7 years ago
|
||
mozreview-review |
Comment on attachment 8840690 [details] Bug 1290218 Part 12: Turn off a crashtest that leaks memory, see Bug 1342289. https://reviewboard.mozilla.org/r/115120/#review116598
Attachment #8840690 -
Flags: review?(cam) → review+
Comment 141•7 years ago
|
||
Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1b319a801847 Part 1: Uplift IsModified into StyleSheet from CSSStyleSheet, and provide empty implementation for ServoStyleSheet. r=heycam https://hg.mozilla.org/integration/autoland/rev/ea165fd4359a Part 2: Uplift mInner pointer from CSSStyleSheetInner into StyleSheetInfo. r=heycam https://hg.mozilla.org/integration/autoland/rev/5e9b9569c297 Part 3: Transition ServoStyleSheet to use mInner pointer, instead of maintaining a static StyleSheetInfo. r=heycam https://hg.mozilla.org/integration/autoland/rev/b433bbbd6228 Part 4: Implement shared mInners for ServoStyleSheets, and standardize calling of AddSheet into CSSStyleSheet and ServoStyleSheet constructors. r=heycam https://hg.mozilla.org/integration/autoland/rev/f42cbc7439d0 Part 4b: Fixup cycle collector declarations for mInner moving into StyleSheetInfo. r=heycam https://hg.mozilla.org/integration/autoland/rev/bc83f0653f30 Part 5: Subclass StyleSheetInfo to move the ServoStyleSheet raw sheet into its mInner. r=heycam https://hg.mozilla.org/integration/autoland/rev/6cb592ba64ec Part 6: Generalize nsXULPrototypeCache to hold StyleSheet instead of CSSStyleSheet. r=heycam https://hg.mozilla.org/integration/autoland/rev/cd6d1b2b9b6c Part 7: Bypass XULPrototypeCache (which is a singleton) for non-Gecko Loaders. r=heycam https://hg.mozilla.org/integration/autoland/rev/a72b5585f19c Part 8: Implement ServoStyleSheet Clone. r=heycam https://hg.mozilla.org/integration/autoland/rev/da2bee86bcb2 Part 9: Generalize stylesheet Loader to cache Servo sheets in the same way that it caches Gecko sheets. r=heycam https://hg.mozilla.org/integration/autoland/rev/fd354b31d6e3 Part 10: Add asserts to ServoStyleSets. r=heycam https://hg.mozilla.org/integration/autoland/rev/5b9d8ea1d448 Part 11: Update mochitest failure count, and mark a reftest as failing. r=heycam https://hg.mozilla.org/integration/autoland/rev/8a73e70ee40a Part 12: Turn off a crashtest that leaks memory, see Bug 1342289. r=heycam
Comment 142•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1b319a801847 https://hg.mozilla.org/mozilla-central/rev/ea165fd4359a https://hg.mozilla.org/mozilla-central/rev/5e9b9569c297 https://hg.mozilla.org/mozilla-central/rev/b433bbbd6228 https://hg.mozilla.org/mozilla-central/rev/f42cbc7439d0 https://hg.mozilla.org/mozilla-central/rev/bc83f0653f30 https://hg.mozilla.org/mozilla-central/rev/6cb592ba64ec https://hg.mozilla.org/mozilla-central/rev/cd6d1b2b9b6c https://hg.mozilla.org/mozilla-central/rev/a72b5585f19c https://hg.mozilla.org/mozilla-central/rev/da2bee86bcb2 https://hg.mozilla.org/mozilla-central/rev/fd354b31d6e3 https://hg.mozilla.org/mozilla-central/rev/5b9d8ea1d448 https://hg.mozilla.org/mozilla-central/rev/8a73e70ee40a
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•