1422654 - stylo: Avoid restyling the element if it has bindings without stylesheets.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P2)

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1422654: stylo: Avoid restyling XBL-bound element if the binding doesn't have stylesheets.

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

Comment 2

[Cameron McCormack (:heycam)]

Comment on attachment 8934035 [details]
Bug 1422654: stylo: Avoid restyling XBL-bound element if the binding doesn't have stylesheets.

https://reviewboard.mozilla.org/r/204964/#review210436

::: dom/xbl/nsXBLService.cpp:450
(Diff revision 1)
>    {
>      if (mHadData) {

MOZ_ASSERT(aResolveStyle) in here?

::: layout/base/ServoRestyleManager.h:247
(Diff revision 1)
>                                  ServoStyleSet& aStyleSet);
>  
>  public:
>    /**
> +   * Whether to clear all the style data (including the element itself), or just
> +   * the descendant's data.

descendants'

::: layout/base/nsCSSFrameConstructor.cpp:5904
(Diff revision 1)
> -      // See the comment in the similar-looking block in
> +      if (resolveStyle) {
> -      // ConstructDocElementFrame to see why we always re-fetch the style
> -      // context in Servo.
> -      if (styleContext->IsServo()) {
> +        if (styleContext->IsServo()) {
> -        styleContext =
> +          styleContext =
> -          mPresShell->StyleSet()->AsServo()->ResolveServoStyle(aContent->AsElement());
> +            mPresShell->StyleSet()->AsServo()->ResolveServoStyle(aContent->AsElement());
> -      } else if (resolveStyle) {
> +        } else {
> -        styleContext =
> +          styleContext =
> -          ResolveStyleContext(styleContext->AsGecko()->GetParent(),
> +            ResolveStyleContext(styleContext->AsGecko()->GetParent(),
> -                              aContent, &aState);
> +                                aContent, &aState);
> -      }
> +        }
> +      }

I'm not quite sure why we need to do different things in the two branches.  Can you explain in a comment?

Comment 3

[Emilio Cobos Álvarez [:emilio]]

(In reply to Cameron McCormack (:heycam) (away until Austin) from comment #2)
> Comment on attachment 8934035 [details]
> Bug 1422654: stylo: Avoid restyling XBL-bound element if the binding doesn't
> have stylesheets.
> 
> https://reviewboard.mozilla.org/r/204964/#review210436
> 
> ::: dom/xbl/nsXBLService.cpp:450
> (Diff revision 1)
> >    {
> >      if (mHadData) {
> 
> MOZ_ASSERT(aResolveStyle) in here?
> 
> ::: layout/base/ServoRestyleManager.h:247
> I'm not quite sure why we need to do different things in the two branches. 
> Can you explain in a comment?

The TL;DR is: No GetParent() for Servo.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8934035 [details]
Bug 1422654: stylo: Avoid restyling XBL-bound element if the binding doesn't have stylesheets.

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

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8934035 [details]
Bug 1422654: stylo: Avoid restyling XBL-bound element if the binding doesn't have stylesheets.

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50ccf635c731
stylo: Avoid restyling XBL-bound element if the binding doesn't have stylesheets. r=heycam

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/50ccf635c731