1374752 - stylo: UpdateStyleOfOwnedChildFrame lost aHintForThisFrame

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

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?

Comment 1

[Emilio Cobos Álvarez (:emilio)]

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.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1374752: Pack together the StyleSet, ChangeList and handled change hints, and use the latter with anonymous boxes while we're at it.

I need to update the docs of the UpdateStyleOfAnonBoxes functions, but I need to
run now. Will do before landing.

Review commit: https://reviewboard.mozilla.org/r/151064/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151064/

Comment 3

[Cameron McCormack (:heycam)]

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...

Comment 4

[Emilio Cobos Álvarez (:emilio)]

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 :)

Comment 5

[Emilio Cobos Álvarez (:emilio)]

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 6

[Emilio Cobos Álvarez (:emilio)]

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.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151064/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fdde73cf429c