Skip to content
This repository has been archived by the owner on Aug 4, 2022. It is now read-only.

Commit

Permalink
Bug 1525134 - Move image loads out of the style struct accessors. r=h…
Browse files Browse the repository at this point in the history 
…eycam

After this I can pass the document from the caller to
ResolveSameStructsAs, and get rid of the pres context pointer.

Differential Revision: https://phabricator.services.mozilla.com/D18600
  • Loading branch information
@emilio
emilio committed Feb 5, 2019
1 parent c6c108c commit e1610eb
Show file tree
Hide file tree
Showing 17 changed files with 116 additions and 254 deletions.
2 changes: 1 addition & 1 deletion dom/animation/KeyframeEffect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1537,7 +1537,7 @@ void KeyframeEffect::CalculateCumulativeChangeHint(

uint32_t equalStructs = 0;
nsChangeHint changeHint =
fromContext->CalcStyleDifference(toContext, &equalStructs);
fromContext->CalcStyleDifference(*toContext, &equalStructs);

mCumulativeChangeHint |= changeHint;
}
Expand Down
8 changes: 5 additions & 3 deletions layout/base/RestyleManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#endif

using mozilla::layers::AnimationInfo;
using mozilla::layout::ScrollAnchorContainer;

using namespace mozilla::dom;
using namespace mozilla::layers;
Expand Down Expand Up @@ -2439,7 +2440,7 @@ struct RestyleManager::TextPostTraversalState {
if (mShouldComputeHints) {
mShouldComputeHints = false;
uint32_t equalStructs;
mComputedHint = oldStyle->CalcStyleDifference(&aNewStyle, &equalStructs);
mComputedHint = oldStyle->CalcStyleDifference(aNewStyle, &equalStructs);
mComputedHint = NS_RemoveSubsumedHints(
mComputedHint, mParentRestyleState.ChangesHandledFor(aTextFrame));
}
Expand Down Expand Up @@ -2545,7 +2546,7 @@ static void UpdateOneAdditionalComputedStyle(nsIFrame* aFrame, uint32_t aIndex,

uint32_t equalStructs; // Not used, actually.
nsChangeHint childHint =
aOldContext.CalcStyleDifference(newStyle, &equalStructs);
aOldContext.CalcStyleDifference(*newStyle, &equalStructs);
if (!aFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW)) {
childHint = NS_RemoveSubsumedHints(childHint,
aRestyleState.ChangesHandledFor(aFrame));
Expand Down Expand Up @@ -2796,7 +2797,8 @@ bool RestyleManager::ProcessPostTraversal(Element* aElement,
// but it doesn't matter, since the only point of it is calling
// TriggerImageLoads on the relevant structs, and those don't matter for
// display: contents.
upToDateContext->ResolveSameStructsAs(oldOrDisplayContentsStyle);
upToDateContext->StartImageLoads(*mPresContext->Document(),
oldOrDisplayContentsStyle);

// We want to walk all the continuations here, even the ones with different
// styles. In practice, the only reason we get continuations with different
Expand Down
7 changes: 0 additions & 7 deletions layout/base/nsCSSFrameConstructor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2336,9 +2336,6 @@ nsIFrame* nsCSSFrameConstructor::ConstructDocElementFrame(
return nullptr;
}

// Make sure to start any background image loads for the root element now.
computedStyle->StartBackgroundImageLoads();

nsFrameConstructorSaveState docElementContainingBlockAbsoluteSaveState;
if (mHasRootAbsPosContainingBlock) {
// Push the absolute containing block now so we can absolutely position
Expand Down Expand Up @@ -5650,10 +5647,6 @@ void nsCSSFrameConstructor::ConstructFramesFromItem(
return;
}

// Start background loads during frame construction so that we're
// guaranteed that they will be started before onload fires.
computedStyle->StartBackgroundImageLoads();

AutoRestore<nsFrameState> savedStateBits(aState.mAdditionalStateBits);
if (item.mIsGeneratedContent) {
// Ensure that frames created here are all tagged with
Expand Down
6 changes: 0 additions & 6 deletions layout/generic/ViewportFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,12 +382,6 @@ void ViewportFrame::UpdateStyle(ServoRestyleState& aRestyleState) {
aRestyleState.StyleSet().ResolveInheritingAnonymousBoxStyle(pseudo,
nullptr);

// We're special because we have a null GetContent(), so don't call things
// like UpdateStyleOfOwnedChildFrame that try to append changes for the
// content to the change list. Nor do we computed a changehint, since we have
// no way to apply it anyway.
newStyle->ResolveSameStructsAs(Style());

MOZ_ASSERT(!GetNextContinuation(), "Viewport has continuations?");
SetComputedStyle(newStyle);

Expand Down
24 changes: 11 additions & 13 deletions layout/generic/nsBulletFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,19 +150,17 @@ bool nsBulletFrame::IsSelfEmpty() {
if (aOldComputedStyle) {
nsAccessibilityService* accService = nsIPresShell::AccService();
if (accService) {
const nsStyleList* oldStyleList = aOldComputedStyle->PeekStyleList();
if (oldStyleList) {
bool hadBullet = oldStyleList->GetListStyleImage() ||
!oldStyleList->mCounterStyle.IsNone();

const nsStyleList* newStyleList = StyleList();
bool hasBullet = newStyleList->GetListStyleImage() ||
!newStyleList->mCounterStyle.IsNone();

if (hadBullet != hasBullet) {
accService->UpdateListBullet(PresContext()->GetPresShell(), mContent,
hasBullet);
}
const nsStyleList* oldStyleList = aOldComputedStyle->StyleList();
bool hadBullet = oldStyleList->GetListStyleImage() ||
!oldStyleList->mCounterStyle.IsNone();

const nsStyleList* newStyleList = StyleList();
bool hasBullet = newStyleList->GetListStyleImage() ||
!newStyleList->mCounterStyle.IsNone();

if (hadBullet != hasBullet) {
accService->UpdateListBullet(PresContext()->GetPresShell(), mContent,
hasBullet);
}
}
}
Expand Down
59 changes: 24 additions & 35 deletions layout/generic/nsFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ void nsFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
NS_FRAME_MAY_BE_TRANSFORMED |
NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN));
} else {
mComputedStyle->StartImageLoads(*PresContext()->Document());
PresContext()->ConstructedFrame();
}
if (GetParent()) {
Expand Down Expand Up @@ -1066,8 +1067,8 @@ void nsIFrame::MarkNeedsDisplayItemRebuild() {
// We don't want to set the property if one already exists.
nsMargin oldValue(0, 0, 0, 0);
nsMargin newValue(0, 0, 0, 0);
const nsStyleMargin* oldMargin = aOldComputedStyle->PeekStyleMargin();
if (oldMargin && oldMargin->GetMargin(oldValue)) {
const nsStyleMargin* oldMargin = aOldComputedStyle->StyleMargin();
if (oldMargin->GetMargin(oldValue)) {
if (!StyleMargin()->GetMargin(newValue) || oldValue != newValue) {
if (!HasProperty(UsedMarginProperty())) {
AddProperty(UsedMarginProperty(), new nsMargin(oldValue));
Expand All @@ -1076,8 +1077,8 @@ void nsIFrame::MarkNeedsDisplayItemRebuild() {
}
}

const nsStylePadding* oldPadding = aOldComputedStyle->PeekStylePadding();
if (oldPadding && oldPadding->GetPadding(oldValue)) {
const nsStylePadding* oldPadding = aOldComputedStyle->StylePadding();
if (oldPadding->GetPadding(oldValue)) {
if (!StylePadding()->GetPadding(newValue) || oldValue != newValue) {
if (!HasProperty(UsedPaddingProperty())) {
AddProperty(UsedPaddingProperty(), new nsMargin(oldValue));
Expand All @@ -1086,20 +1087,16 @@ void nsIFrame::MarkNeedsDisplayItemRebuild() {
}
}

const nsStyleBorder* oldBorder = aOldComputedStyle->PeekStyleBorder();
if (oldBorder) {
oldValue = oldBorder->GetComputedBorder();
newValue = StyleBorder()->GetComputedBorder();
if (oldValue != newValue && !HasProperty(UsedBorderProperty())) {
AddProperty(UsedBorderProperty(), new nsMargin(oldValue));
}
const nsStyleBorder* oldBorder = aOldComputedStyle->StyleBorder();
oldValue = oldBorder->GetComputedBorder();
newValue = StyleBorder()->GetComputedBorder();
if (oldValue != newValue && !HasProperty(UsedBorderProperty())) {
AddProperty(UsedBorderProperty(), new nsMargin(oldValue));
}

const nsStyleDisplay* oldDisp = aOldComputedStyle->PeekStyleDisplay();
if (oldDisp &&
(oldDisp->mOverflowAnchor != StyleDisplay()->mOverflowAnchor)) {
if (ScrollAnchorContainer* container =
ScrollAnchorContainer::FindFor(this)) {
const nsStyleDisplay* oldDisp = aOldComputedStyle->StyleDisplay();
if (oldDisp->mOverflowAnchor != StyleDisplay()->mOverflowAnchor) {
if (auto* container = ScrollAnchorContainer::FindFor(this)) {
container->InvalidateAnchor();
}
if (nsIScrollableFrame* scrollableFrame = do_QueryFrame(this)) {
Expand All @@ -1109,20 +1106,19 @@ void nsIFrame::MarkNeedsDisplayItemRebuild() {

if (mInScrollAnchorChain) {
const nsStylePosition* oldPosition =
aOldComputedStyle->PeekStylePosition();
if (oldPosition &&
(oldPosition->mOffset != StylePosition()->mOffset ||
oldPosition->mWidth != StylePosition()->mWidth ||
oldPosition->mMinWidth != StylePosition()->mMinWidth ||
oldPosition->mMaxWidth != StylePosition()->mMaxWidth ||
oldPosition->mHeight != StylePosition()->mHeight ||
oldPosition->mMinHeight != StylePosition()->mMinHeight ||
oldPosition->mMaxHeight != StylePosition()->mMaxHeight)) {
aOldComputedStyle->StylePosition();
if (oldPosition->mOffset != StylePosition()->mOffset ||
oldPosition->mWidth != StylePosition()->mWidth ||
oldPosition->mMinWidth != StylePosition()->mMinWidth ||
oldPosition->mMaxWidth != StylePosition()->mMaxWidth ||
oldPosition->mHeight != StylePosition()->mHeight ||
oldPosition->mMinHeight != StylePosition()->mMinHeight ||
oldPosition->mMaxHeight != StylePosition()->mMaxHeight) {
needAnchorSuppression = true;
}

if (oldDisp && (oldDisp->mPosition != StyleDisplay()->mPosition ||
oldDisp->TransformChanged(*StyleDisplay()))) {
if (oldDisp->mPosition != StyleDisplay()->mPosition ||
oldDisp->TransformChanged(*StyleDisplay())) {
needAnchorSuppression = true;
}
}
Expand Down Expand Up @@ -10248,14 +10244,7 @@ void nsIFrame::UpdateStyleOfChildAnonBox(nsIFrame* aChildFrame,
// anonymous boxes directly.
uint32_t equalStructs; // Not used, actually.
nsChangeHint childHint = aChildFrame->Style()->CalcStyleDifference(
aNewComputedStyle, &equalStructs);

// CalcStyleDifference will handle caching structs on the new style, but only
// if we're not on a style worker thread.
MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal(),
"if we can get in here from style worker threads, then we need "
"a ResolveSameStructsAs call to ensure structs are cached on "
"aNewComputedStyle");
*aNewComputedStyle, &equalStructs);

// If aChildFrame is out of flow, then aRestyleState's "changes handled by the
// parent" doesn't apply to it, because it may have some other parent in the
Expand Down
76 changes: 34 additions & 42 deletions layout/style/ComputedStyle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ ComputedStyle::ComputedStyle(nsPresContext* aPresContext, nsAtom* aPseudoTag,
MOZ_ASSERT(ComputedData());
}

nsChangeHint ComputedStyle::CalcStyleDifference(ComputedStyle* aNewContext,
uint32_t* aEqualStructs) {
MOZ_ASSERT(aNewContext);
nsChangeHint ComputedStyle::CalcStyleDifference(const ComputedStyle& aNewStyle,
uint32_t* aEqualStructs) const {
AUTO_PROFILER_LABEL("ComputedStyle::CalcStyleDifference", LAYOUT);
static_assert(StyleStructConstants::kStyleStructCount <= 32,
"aEqualStructs is not big enough");
Expand Down Expand Up @@ -91,30 +90,25 @@ nsChangeHint ComputedStyle::CalcStyleDifference(ComputedStyle* aNewContext,
#define PEEK(struct_) ComputedData()->GetStyle##struct_()

#define EXPAND(...) __VA_ARGS__
#define DO_STRUCT_DIFFERENCE_WITH_ARGS(struct_, extra_args_) \
PR_BEGIN_MACRO \
const nsStyle##struct_* this##struct_ = PEEK(struct_); \
if (this##struct_) { \
structsFound |= STYLE_STRUCT_BIT(struct_); \
\
const nsStyle##struct_* other##struct_ = \
aNewContext->ThreadsafeStyle##struct_(); \
if (this##struct_ == other##struct_) { \
/* The very same struct, so we know that there will be no */ \
/* differences. */ \
*aEqualStructs |= STYLE_STRUCT_BIT(struct_); \
} else { \
nsChangeHint difference = \
this##struct_->CalcDifference(*other##struct_ EXPAND extra_args_); \
hint |= difference; \
if (!difference) { \
*aEqualStructs |= STYLE_STRUCT_BIT(struct_); \
} \
} \
} else { \
*aEqualStructs |= STYLE_STRUCT_BIT(struct_); \
} \
styleStructCount++; \
#define DO_STRUCT_DIFFERENCE_WITH_ARGS(struct_, extra_args_) \
PR_BEGIN_MACRO \
const nsStyle##struct_* this##struct_ = Style##struct_(); \
structsFound |= STYLE_STRUCT_BIT(struct_); \
\
const nsStyle##struct_* other##struct_ = aNewStyle.Style##struct_(); \
if (this##struct_ == other##struct_) { \
/* The very same struct, so we know that there will be no */ \
/* differences. */ \
*aEqualStructs |= STYLE_STRUCT_BIT(struct_); \
} else { \
nsChangeHint difference = \
this##struct_->CalcDifference(*other##struct_ EXPAND extra_args_); \
hint |= difference; \
if (!difference) { \
*aEqualStructs |= STYLE_STRUCT_BIT(struct_); \
} \
} \
styleStructCount++; \
PR_END_MACRO
#define DO_STRUCT_DIFFERENCE(struct_) \
DO_STRUCT_DIFFERENCE_WITH_ARGS(struct_, ())
Expand Down Expand Up @@ -179,8 +173,8 @@ nsChangeHint ComputedStyle::CalcStyleDifference(ComputedStyle* aNewContext,
// here, we add nsChangeHint_RepaintFrame hints (the maximum for
// things that can depend on :visited) for the properties on which we
// call GetVisitedDependentColor.
ComputedStyle* thisVis = GetStyleIfVisited();
ComputedStyle* otherVis = aNewContext->GetStyleIfVisited();
const ComputedStyle* thisVis = GetStyleIfVisited();
const ComputedStyle* otherVis = aNewStyle.GetStyleIfVisited();
if (!thisVis != !otherVis) {
// One style has a style-if-visited and the other doesn't.
// Presume a difference.
Expand All @@ -198,15 +192,14 @@ nsChangeHint ComputedStyle::CalcStyleDifference(ComputedStyle* aNewContext,
// due to change being true already or due to the old style not having a
// style-if-visited), but not the other way around.
#define STYLE_FIELD(name_) thisVisStruct->name_ != otherVisStruct->name_
#define STYLE_STRUCT(name_, fields_) \
if (PEEK(name_)) { \
const nsStyle##name_* thisVisStruct = thisVis->ThreadsafeStyle##name_(); \
const nsStyle##name_* otherVisStruct = otherVis->ThreadsafeStyle##name_(); \
if (MOZ_FOR_EACH_SEPARATED(STYLE_FIELD, (||), (), fields_)) { \
*aEqualStructs &= ~STYLE_STRUCT_BIT(name_); \
change = true; \
} \
}
#define STYLE_STRUCT(name_, fields_) { \
const nsStyle##name_* thisVisStruct = thisVis->Style##name_(); \
const nsStyle##name_* otherVisStruct = otherVis->Style##name_(); \
if (MOZ_FOR_EACH_SEPARATED(STYLE_FIELD, (||), (), fields_)) { \
*aEqualStructs &= ~STYLE_STRUCT_BIT(name_); \
change = true; \
} \
}
#include "nsCSSVisitedDependentPropList.h"
#undef STYLE_STRUCT
#undef STYLE_FIELD
Expand All @@ -231,15 +224,14 @@ nsChangeHint ComputedStyle::CalcStyleDifference(ComputedStyle* aNewContext,
// doesn't use Peek* functions to get the structs on the old
// context. But this isn't a big concern because these struct
// getters should be called during frame construction anyway.
const nsStyleDisplay* oldDisp = ThreadsafeStyleDisplay();
const nsStyleDisplay* newDisp = aNewContext->ThreadsafeStyleDisplay();
const nsStyleDisplay* oldDisp = StyleDisplay();
const nsStyleDisplay* newDisp = aNewStyle.StyleDisplay();
bool isFixedCB;
if (oldDisp->IsAbsPosContainingBlockForNonSVGTextFrames() ==
newDisp->IsAbsPosContainingBlockForNonSVGTextFrames() &&
(isFixedCB =
oldDisp->IsFixedPosContainingBlockForNonSVGTextFrames(*this)) ==
newDisp->IsFixedPosContainingBlockForNonSVGTextFrames(
*aNewContext) &&
newDisp->IsFixedPosContainingBlockForNonSVGTextFrames(aNewStyle) &&
// transform-supporting frames are a subcategory of non-SVG-text
// frames, so no need to test this if isFixedCB is true (both
// before and after the change)
Expand Down
Loading

0 comments on commit e1610eb

Please sign in to comment.