Skip to content

Speculative fix for crashes underneath StyleLengthResolution::adjustValueForPageZoom#57891

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
sammygill:eng/Mysterious-crashes-under-StyleLengthResolution-adjustValueForPageZoom
Feb 6, 2026
Merged

Speculative fix for crashes underneath StyleLengthResolution::adjustValueForPageZoom#57891
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
sammygill:eng/Mysterious-crashes-under-StyleLengthResolution-adjustValueForPageZoom

Conversation

@sammygill
Copy link
Contributor

@sammygill sammygill commented Feb 4, 2026

a7e0750

Speculative fix for crashes underneath StyleLengthResolution::adjustValueForPageZoom
https://bugs.webkit.org/show_bug.cgi?id=306989
rdar://168722605

Reviewed by Brent Fulgham.

Stability data seems to suggest that it is possible to hit some crashes
underneath StyleLengthResolution::adjustValueForPageZoom. These crashes
seem to be coming from the fact that we attempt to access the page zoom
factor via the RenderView on CSSToLengthConversionData.

It is not very clear how we can get into this state since code
inspection seems to indicate that we try very hard to make sure that
cleanup between these two objects is handled properly. An investigation
to attempt to reproduce this crash has also not been very fruitful since
it seems at least some of those who experienced this crash were not
aware that it happened, could not remember it occurring, or were not
able to get it to reproduce either by navigating through history.

In order to increase stability, and also hopefully be able to obtain
more actionable bug reports, we attempt a speculative fix for
addressing this crash. The main change is that in the constructor for
CSSToLengthConversionData, we use a new helper function to figure out
what we should use for the RenderView field. Since we cannot directly
check the existence of the LocalFrameView on the RenderView, we
look at the Document, which is the sole owner of the RenderView, to see
if it is still there. If the Document no longer has its LocalFrameView,
then we will return nullptr for the RenderView. Much of the other code
in CSSToLengthConversionData already performs a nullptr check for
RenderView, so we also need to start doing the same in
adjustValueForPageZoom since there is no guarantee that this pointer is
always non-null.

Canonical link: https://commits.webkit.org/306918@main

ed59ff6

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 💥 🛠 win loading 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 💥 🧪 win-tests ⏳ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe loading 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 wpe-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp
✅ 🛠 watch
✅ 🛠 watch-sim

@sammygill sammygill self-assigned this Feb 4, 2026
@sammygill sammygill added the CSS Cascading Style Sheets implementation label Feb 4, 2026
@sammygill sammygill force-pushed the eng/Mysterious-crashes-under-StyleLengthResolution-adjustValueForPageZoom branch from 25b8b23 to b14ab2f Compare February 4, 2026 23:01
@sammygill sammygill force-pushed the eng/Mysterious-crashes-under-StyleLengthResolution-adjustValueForPageZoom branch from b14ab2f to d1a385f Compare February 4, 2026 23:04
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 5, 2026
Copy link
Contributor

@brentfulgham brentfulgham left a comment

Choose a reason for hiding this comment

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

Thank you for adding this extra check. r=me!

// alive by checking it on the Document.
static RenderView* renderViewForDocument(const Document& document)
{
if (document.view())
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest:

if (document.view()) [Likely]

@sammygill sammygill removed the merging-blocked Applied to prevent a change from being merged label Feb 5, 2026
@sammygill sammygill force-pushed the eng/Mysterious-crashes-under-StyleLengthResolution-adjustValueForPageZoom branch from d1a385f to ed59ff6 Compare February 5, 2026 18:59
@sammygill sammygill added safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks merge-queue Applied to send a pull request to merge-queue and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Feb 5, 2026
…alueForPageZoom

https://bugs.webkit.org/show_bug.cgi?id=306989
rdar://168722605

Reviewed by Brent Fulgham.

Stability data seems to suggest that it is possible to hit some crashes
underneath StyleLengthResolution::adjustValueForPageZoom. These crashes
seem to be coming from the fact that we attempt to access the page zoom
factor via the RenderView on CSSToLengthConversionData.

It is not very clear how we can get into this state since code
inspection seems to indicate that we try very hard to make sure that
cleanup between these two objects is handled properly. An investigation
to attempt to reproduce this crash has also not been very fruitful since
it seems at least some of those who experienced this crash were not
aware that it happened, could not remember it occurring, or were not
able to get it to reproduce either by navigating through history.

In order to increase stability, and also hopefully be able to obtain
more actionable bug reports, we attempt a speculative fix for
addressing this crash. The main change is that in the constructor for
CSSToLengthConversionData, we use a new helper function to figure out
what we should use for the RenderView field. Since we cannot directly
check the existence of the LocalFrameView on the RenderView, we
look at the Document, which is the sole owner of the RenderView, to see
if it is still there. If the Document no longer has its LocalFrameView,
then we will return nullptr for the RenderView. Much of the other code
in CSSToLengthConversionData already performs a nullptr check for
RenderView, so we also need to start doing the same in
adjustValueForPageZoom since there is no guarantee that this pointer is
always non-null.

Canonical link: https://commits.webkit.org/306918@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Mysterious-crashes-under-StyleLengthResolution-adjustValueForPageZoom branch from ed59ff6 to a7e0750 Compare February 6, 2026 04:11
@webkit-commit-queue
Copy link
Collaborator

Committed 306918@main (a7e0750): https://commits.webkit.org/306918@main

Reviewed commits have been landed. Closing PR #57891 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit a7e0750 into WebKit:main Feb 6, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CSS Cascading Style Sheets implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants