script: No longer do explicit reflows for display#34599
Conversation
components/script/dom/window.rs
Outdated
| // parsing, as we need to interrupt it to update contents, but why is this | ||
| // necessary when parsing finishes? Not doing the synchronous update in that case | ||
| // causes iframe tests to become flaky. | ||
| with_script_thread(|script_thread| script_thread.update_the_rendering(false, can_gc)) |
There was a problem hiding this comment.
I'm not sure it is a good idea to run the full update here, as opposed to only doing a reflow, because finishing parsing a script tag seems to come with it's own approach to performing a microtask checkpoint. Would only doing a reflow here be sufficient? If so, I think it would be the approach with less possible side-effects, either now or if we make changes to the parser(which seems to lag behind the spec from a quick glance).
There was a problem hiding this comment.
How would you feel about running update the rendering, but without running a microtask checkpoint? I'm a bit worried about doing reflow of a single pipeline these days. I think it might be leading to early exits of the test runner / flaky result and I'd like to handle this a bit more thoroughly in update the rendering.
There was a problem hiding this comment.
Scratch that. I think the issue is related to our problems with <iframe> sizes. I'm working on that now and should hopefully be able to remove this final reflow (#14719).
mrobinson
force-pushed
the
remove-most-explicit-layouts
branch
from
677be21 to
8a0b9a8
Compare
These all happen now in *update the rendering*, typically after the message that triggered this code is processed, though in two cases reflow needs to be triggered explicitly. This makes `ReflowReason` redundant though perhaps `ReflowCondition` can be expanded later to give more insight into why the page is dirty. - Handling of the "reflow timer" concept has been explained a bit more via data structures and rustdoc comments. - Theme changes are cleaned up a little to simplify what happens during reflow and to avoid unecessary reflows when the theme doesn't change. Notably, layout queries and scrolling still trigger normal reflows and don't update the rendering. This needs more investigation as it's unclear to me currently whether or not they should update the rendering and simply delay event dispatch or only reflow. In general, this is a simplfication of the code. Fixes servo#31871. Signed-off-by: Martin Robinson <[email protected]>
mrobinson
force-pushed
the
remove-most-explicit-layouts
branch
from
8a0b9a8 to
e1008d8
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 participants
These all happen now in update the rendering, typically after the
message that triggered this code is processed, though in two cases
reflow needs to be triggered explicitly. This makes
ReflowReasonredundant though perhaps
ReflowConditioncan be expanded later to givemore insight into why the page is dirty.
data structures and rustdoc comments.
reflow and to avoid unecessary reflows when the theme doesn't change.
Notably, layout queries and scrolling still trigger normal reflows and
don't update the rendering. This needs more investigation as it's
unclear to me currently whether or not they should update the rendering
and simply delay event dispatch or only reflow.
In general, this is a simplfication of the code.
Fixes #31871.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsmay be different, but they do not introduce any changes to layout results as
far as I can tell.