1374761 - stylo: support incremental restyling of various root-related anonymous boxes

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

Viewport, root scrollframe, etc.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1374761 part 1. Properly restyle the viewport and its child anonymous box when we do a restyle.

The child anon box may be an nsHTMLScrollFrame, a nsRootBoxFrame, or a
nsSimplePageSequenceFrame.   nsHTMLScrollFrame already knows how to deal with
its anonymous box kids, nsRootBoxFrame doesn't have any, and the next changeset
will deal with anon box kids of nsSimplePageSequenceFrame.

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

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1374761 part 2. Properly restyle the various paginated-presentation anonymous boxes.

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8879688 [details]
Bug 1374761 part 1.  Properly restyle the viewport and its child anonymous box when we do a restyle.

https://reviewboard.mozilla.org/r/151034/#review155892

::: layout/generic/ViewportFrame.cpp:434
(Diff revision 1)
> +  // content to the change list.
> +  uint32_t equalStructs, samePointerStructs; // Not used.
> +  // We ignore the hint, since we can't very well queue it up anyway with a null
> +  // GetContent().  But we still want to CalcStyleDifference to reget lazy
> +  // structs as needed.
> +  Unused << oldContext->CalcStyleDifference(newContext, &equalStructs,

`EnsureSameStructsCached`?

Comment 4

[Boris Zbarsky [:bzbarsky]]

> `EnsureSameStructsCached`?

Good catch!

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8879688 [details]
Bug 1374761 part 1.  Properly restyle the viewport and its child anonymous box when we do a restyle.

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

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8879689 [details]
Bug 1374761 part 2.  Properly restyle the various paginated-presentation anonymous boxes.

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

Comment 7

[Cameron McCormack (:heycam)]

Comment on attachment 8879688 [details]
Bug 1374761 part 1.  Properly restyle the viewport and its child anonymous box when we do a restyle.

https://reviewboard.mozilla.org/r/151034/#review156186

::: layout/generic/nsFrame.cpp:10211
(Diff revision 2)
>                                      nsStyleChangeList& aChangeList,
>                                      nsChangeHint aHintForThisFrame)
>  {
>    MOZ_ASSERT(aChildFrame->GetParent() == this,
>               "This should only be used for children!");
> -  MOZ_ASSERT(aChildFrame->GetContent() == GetContent(),
> +  MOZ_ASSERT(!GetContent() /* We're the viewport */ ||

You could do |!GetContent() && IsViewportFrame()|.

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8879689 [details]
Bug 1374761 part 2.  Properly restyle the various paginated-presentation anonymous boxes.

https://reviewboard.mozilla.org/r/151036/#review156190

::: layout/base/nsCSSFrameConstructor.cpp:3048
(Diff revision 2)
> +  if (!prevPageContentFrame) {
> +    pageContentFrame->AddStateBits(NS_FRAME_OWNS_ANON_BOXES);
> +  }

Why only on the first one?

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Why only on the first one?

Because the anon box handling always walks continuations.  So we only set this flag on first continuations to avoid O(N^2) walks.  This is how it works for all frames that have this flag set.

Comment 10

[Cameron McCormack (:heycam)]

Comment on attachment 8879689 [details]
Bug 1374761 part 2.  Properly restyle the various paginated-presentation anonymous boxes.

https://reviewboard.mozilla.org/r/151036/#review156272

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8879688 [details]
Bug 1374761 part 1.  Properly restyle the viewport and its child anonymous box when we do a restyle.

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

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8879689 [details]
Bug 1374761 part 2.  Properly restyle the various paginated-presentation anonymous boxes.

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

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8879688 [details]
Bug 1374761 part 1.  Properly restyle the viewport and its child anonymous box when we do a restyle.

https://reviewboard.mozilla.org/r/151034/#review156186

> You could do |!GetContent() && IsViewportFrame()|.

Good idea, done.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74d8ec6397ee
part 1.  Properly restyle the viewport and its child anonymous box when we do a restyle.  r=heycam
https://hg.mozilla.org/integration/autoland/rev/272a41ef0e3d
part 2.  Properly restyle the various paginated-presentation anonymous boxes.  r=heycam

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e990953fc64f
followup.  Adjust test expectations for now-passing tests.  r=orange

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/74d8ec6397ee
https://hg.mozilla.org/mozilla-central/rev/272a41ef0e3d
https://hg.mozilla.org/mozilla-central/rev/e990953fc64f