Closed Bug 1364824 Opened 3 years ago Closed 3 years ago

stylo: Implement PostRebuildAllStyleDataEvent

Categories

(Core :: CSS Parsing and Computation, enhancement)

53 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: xidorn, Assigned: emilio)

References

Details

Attachments

(1 file, 1 obsolete file)

@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.
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
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 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 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+
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`
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.
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.
Attachment #8867624 - Attachment is obsolete: true
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fa66df85a979
Implement ServoRestyleManager::PostRebuildAllStyleDataEvent. r=heycam
Blocks: 1364862
https://hg.mozilla.org/mozilla-central/rev/fa66df85a979
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee: nobody → emilio
You need to log in before you can comment on or make changes to this bug.