stylo: UpdateStyleOfOwnedChildFrame lost aHintForThisFrame

RESOLVED FIXED in Firefox 56

Status

()

RESOLVED FIXED
a year ago
2 months ago

People

(Reporter: bzbarsky, Assigned: emilio)

Tracking

(Blocks: 2 bugs)

53 Branch
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

This function was added in bug 1364871, refactoring existing code that .  It still has comments about aHintForThisFrame, but no longer has such an argument and no longer optimizes out the hints handled for descendants from aHintForThisFrame.

Though maybe this is really sort of fallout from bug 1302054 not cleaning up things enough?
Flags: needinfo?(emilio+bugs)
(Assignee)

Comment 1

a year ago
Yeah, I didn't remove any call to NS_RemoveSubsumedHints or anything like that in that bug.

That being said, it looks like we should add it back now I added the hints for descendants optimization.
Flags: needinfo?(emilio+bugs)
(Reporter)

Updated

a year ago
Blocks: 1243581, 1289864
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8879722 [details]
Bug 1374752: Pack together the StyleSet, ChangeList and handled change hints, and use the latter with anonymous boxes while we're at it.

https://reviewboard.mozilla.org/r/151064/#review156574

Looks good, thanks!

::: commit-message-9c95f:3
(Diff revision 1)
> +I need to update the docs of the UpdateStyleOfAnonBoxes functions, but I need to
> +run now. Will do before landing.

Don't forget to remove this from the commit message too. :-)

::: layout/base/ServoRestyleManager.h:35
(Diff revision 1)
> +{
> +  ServoStyleSet& mStyleSet;
> +  nsStyleChangeList& mChangeList;
> +  const nsChangeHint mChangesHandled;
> +
> +public:

Nit: would prefer the more conventional arrangement of

  public:
    ... member functions ...

  private:
    ... member variables ...

::: layout/base/ServoRestyleManager.cpp:182
(Diff revision 1)
> +private:
>    nsStyleContext& mParentContext;
> -  ServoStyleSet& mStyleSet;
> +  ServoRestyleState& mParentRestyleState;
>    RefPtr<nsStyleContext> mStyle;
>    bool mShouldPostHints;
>    bool mShouldComputeHints;
>    nsChangeHint mComputedHint;
> -  nsChangeHint mHintsHandled;
>  
> +public:

Nit: here too I guess.

::: layout/generic/nsFrame.cpp:10229
(Diff revision 1)
> +                                                                StyleContext());
>  
>    nsChangeHint childHint =
> -    UpdateStyleOfOwnedChildFrame(aChildFrame, newContext, aChangeList);
> +    UpdateStyleOfOwnedChildFrame(aChildFrame, newContext, aRestyleState);
>  
> +  ServoRestyleState childrenState(aRestyleState, childHint);

Nit: might look a little neater adding this below the comment...
Attachment #8879722 - Flags: review?(cam) → review+
(Assignee)

Comment 4

a year ago
mozreview-review
Comment on attachment 8879722 [details]
Bug 1374752: Pack together the StyleSet, ChangeList and handled change hints, and use the latter with anonymous boxes while we're at it.

https://reviewboard.mozilla.org/r/151064/#review156654

::: layout/base/ServoRestyleManager.h:35
(Diff revision 1)
> +{
> +  ServoStyleSet& mStyleSet;
> +  nsStyleChangeList& mChangeList;
> +  const nsChangeHint mChangesHandled;
> +
> +public:

Done.

::: layout/base/ServoRestyleManager.cpp:182
(Diff revision 1)
> +private:
>    nsStyleContext& mParentContext;
> -  ServoStyleSet& mStyleSet;
> +  ServoRestyleState& mParentRestyleState;
>    RefPtr<nsStyleContext> mStyle;
>    bool mShouldPostHints;
>    bool mShouldComputeHints;
>    nsChangeHint mComputedHint;
> -  nsChangeHint mHintsHandled;
>  
> +public:

Done too :)
(Assignee)

Comment 5

a year ago
mozreview-review-reply
Comment on attachment 8879722 [details]
Bug 1374752: Pack together the StyleSet, ChangeList and handled change hints, and use the latter with anonymous boxes while we're at it.

https://reviewboard.mozilla.org/r/151064/#review156574

> Nit: might look a little neater adding this below the comment...

Agreed, done.
Comment hidden (mozreview-request)

Comment 7

a year ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fdde73cf429c
Pack together the StyleSet, ChangeList and handled change hints, and use the latter with anonymous boxes while we're at it. r=heycam

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fdde73cf429c
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Reporter)

Updated

a year ago
Depends on: 1375674
Assignee: nobody → emilio
You need to log in before you can comment on or make changes to this bug.