Skip to content

fix: don't force layout from LayoutSelection::Commit() - #52670

Open
codebytere wants to merge 2 commits into
mainfrom
fix-blink-selection-commit
Open

fix: don't force layout from LayoutSelection::Commit()#52670
codebytere wants to merge 2 commits into
mainfrom
fix-blink-selection-commit

Conversation

@codebytere

Copy link
Copy Markdown
Member

Description of Change

Backports https://chromium-review.googlesource.com/c/chromium/src/+/8187581 (in review upstream; the patch drops out when it merges and rolls).

LayoutSelection::Commit() runs during the compositing-inputs lifecycle phase and reaches FrameSelection::SelectionHasFocus(), which unconditionally forces Document::UpdateStyleAndLayout(). If the layout tree is still dirty at that point, the recalc runs while tree mutations are disallowed and the renderer dies on CHECK(Lifecycle().StateAllowsTreeMutations()) - in the field this is a plain drag-select mouse-up crash. The fix keeps the selection pending and schedules another frame, so the next frame's compositing-inputs step commits against a clean tree.

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • PR title follows semantic commit guidelines
  • PR release notes describe the change in a way relevant to app developers

Release Notes

Notes: Fixed a renderer crash when finishing a drag selection while a layout update was still pending.

@codebytere
codebytere requested a review from a team as a code owner August 5, 2026 20:37
@codebytere codebytere added semver/patch backwards-compatible bug fixes target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. target/44-x-y PR should also be added to the "44-x-y" branch. labels Aug 5, 2026
@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Aug 5, 2026

@deepak1556 deepak1556 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not much familiar with this code path, the upstream test doesn't seem to validate the pending selection gets committed on the next update. Is that intentional ?

If possible waiting for upstream review before merging would be good to get additional confidence.

Backports https://chromium-review.googlesource.com/c/chromium/src/+/8187581.

LayoutSelection::Commit() runs during the compositing-inputs lifecycle
phase and reaches FrameSelection::SelectionHasFocus(), which forces
Document::UpdateStyleAndLayout(). Style can be dirty at that point
without the lifecycle having rewound (a due animation timing update is
enough), and then the recalc runs while tree mutations are disallowed
and the renderer dies on CHECK(Lifecycle().StateAllowsTreeMutations())
- in the field a plain drag-select mouse-up crash. Keep the selection
pending and schedule another frame instead, so the next frame commits
against clean style.
@codebytere
codebytere force-pushed the fix-blink-selection-commit branch from 3e73d93 to 543d58c Compare August 6, 2026 18:27
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/patch backwards-compatible bug fixes target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. target/44-x-y PR should also be added to the "44-x-y" branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants