Closed Bug 1374761 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: CSS Parsing and Computation, enhancement)

53 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(2 files)

Viewport, root scrollframe, etc.
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`?
> `EnsureSameStructsCached`?

Good catch!
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()|.
Attachment #8879688 - Flags: review?(cam) → review+
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?
> 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 on attachment 8879689 [details]
Bug 1374761 part 2.  Properly restyle the various paginated-presentation anonymous boxes.

https://reviewboard.mozilla.org/r/151036/#review156272
Attachment #8879689 - Flags: review?(cam) → review+
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.
Pushed by bzbarsky@mozilla.com:
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
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e990953fc64f
followup.  Adjust test expectations for now-passing tests.  r=orange
Assignee: nobody → bzbarsky
You need to log in before you can comment on or make changes to this bug.