stylo: Implement PostRebuildAllStyleDataEvent

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: xidorn, Assigned: emilio)

Tracking

(Blocks: 1 bug)

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

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
@counter-style needs a mechanism to invalidate all style data (recompute all data in style structs) after rule changes. It currently uses PostRebuildAllStyleDataEvent in Gecko to do so asynchronously from nsPresContext::FlushCounterStyles. We probably need something similar in Stylo.
(Reporter)

Comment 1

2 years ago
emilio, could you provide some suggestion on how should we do this?

It seems to me RebuildAllStyleData in ServoRestyleManager is actually async somehow. I wonder whether we can just duplicate some of its code to Post*Event one...
Flags: needinfo?(emilio+bugs)
Duplicate of this bug: 1360805
(Assignee)

Comment 3

2 years ago
You mean async in the sense that the restyle is posted asynchronously? If so, yes.

If you want it synchronously you should be able to do:

    RebuildAllStyleData();
    ProcessPendingRestyles();

Would that work for you?
Flags: needinfo?(emilio+bugs)
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8867617 [details]
Bug 1364824: Implement ServoRestyleManager::PostRebuildAllStyleDataEvent.

https://reviewboard.mozilla.org/r/139188/#review142416

::: commit-message-72dbf:4
(Diff revision 1)
> +Bug 1364824: Implement ServoRestyleManager::PostRebuildAllStyleDataEvent. r?heycam
> +
> +MozReview-Commit-ID: I9HmM9ZMB2c
> +Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>

Probably don't need this line.
Attachment #8867617 - Flags: review?(cam) → review+
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8867624 [details]
Bug 1364824: Make RebuildAllStyleData's flush async too.

https://reviewboard.mozilla.org/r/139190/#review142418

OK, let's try.
Attachment #8867624 - Flags: review?(cam) → review+
(Reporter)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8867624 [details]
Bug 1364824: Make RebuildAllStyleData's flush async too.

https://reviewboard.mozilla.org/r/139190/#review142468

::: servo/components/style/gecko/data.rs:92
(Diff revision 1)
>  
>  impl PerDocumentStyleDataImpl {
>      /// Reset the device state because it may have changed.
>      ///
>      /// Implies also a stylesheet flush.
>      pub fn reset_device(&mut self, guard: &SharedRwLockReadGuard) {

> error: unused variable: `guard`
(Reporter)

Comment 9

2 years ago
It seems somehow the patch doesn't quite work :/

The first patch doesn't fix the issue of bug 1328319, and the second patch has startup crash.
(Assignee)

Comment 10

2 years ago
The second patch doesn't work because I forgot to force_dirty the stylesheets, and because we definitely rely on it being sync (some font-face tests assert without it).

I'll land the first patch because it fixes some font-face tests:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1f30074da4dec0e5f0cdf6fd10d36064d750440

Then file a bug to make the flush async, which requires a few more changes.
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8867624 - Attachment is obsolete: true

Comment 12

2 years ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fa66df85a979
Implement ServoRestyleManager::PostRebuildAllStyleDataEvent. r=heycam
(Assignee)

Updated

2 years ago
Blocks: 1364862

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fa66df85a979
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee: nobody → emilio
You need to log in before you can comment on or make changes to this bug.