1405814 - Crash in mozilla::layers::ScrollMetadata::ScrollMetadata

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-74138075-a3db-4b31-9a5c-068f00170927.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	XUL 	mozilla::layers::ScrollMetadata::ScrollMetadata(mozilla::layers::ScrollMetadata const&) 	xpcom/ds/nsTArray.h:398
1 	XUL 	mozilla::layers::WebRenderScrollData::AddMetadata(mozilla::layers::ScrollMetadata const&) 	gfx/layers/FrameMetrics.h:778
2 	XUL 	mozilla::layers::WebRenderLayerScrollData::Initialize(mozilla::layers::WebRenderScrollData&, nsDisplayItem*, int, mozilla::ActiveScrolledRoot const*) 	gfx/layers/wr/WebRenderScrollData.cpp:89
3 	XUL 	mozilla::layers::WebRenderLayerManager::CreateWebRenderCommandsFromDisplayList(nsDisplayList*, nsDisplayListBuilder*, mozilla::layers::StackingContextHelper const&, mozilla::wr::DisplayListBuilder&, mozilla::wr::IpcResourceUpdateQueue&) 	gfx/layers/wr/WebRenderLayerManager.cpp:361
4 	XUL 	mozilla::layers::WebRenderLayerManager::EndTransactionInternal(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags, nsDisplayList*, nsDisplayListBuilder*) 	gfx/layers/wr/WebRenderLayerManager.cpp:784
5 	XUL 	nsDisplayList::PaintRoot(nsDisplayListBuilder*, gfxContext*, unsigned int) 	layout/painting/nsDisplayList.cpp:2173
6 	XUL 	nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) 	layout/base/nsLayoutUtils.cpp:3823
7 	XUL 	mozilla::PresShell::Paint(nsView*, nsRegion const&, unsigned int) 	layout/base/PresShell.cpp:6454
8 	XUL 	nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) 	view/nsViewManager.cpp:480
9 	XUL 	nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) 	view/nsViewManager.cpp:412
10 	XUL 	nsViewManager::ProcessPendingUpdates() 	view/nsViewManager.cpp:1102
11 	XUL 	nsRefreshDriver::Tick(long long, mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp:2082
12 	XUL 	mozilla::RefreshDriverTimer::TickRefreshDrivers(long long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) 	layout/base/nsRefreshDriver.cpp:337
13 	XUL 	mozilla::RefreshDriverTimer::Tick(long long, mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp:329
14 	XUL 	mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp:770
15 	XUL 	mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp:584
16 	XUL 	mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) 	layout/ipc/VsyncChild.cpp:67
17 	XUL 	mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) 	ipc/ipdl/PVsyncChild.cpp:155
18 	XUL 	mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp:2119
19 	XUL 	mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) 	ipc/glue/MessageChannel.cpp:2049
20 	XUL 	mozilla::ipc::MessageChannel::MessageTask::Run() 	ipc/glue/MessageChannel.cpp:1928
21 	XUL 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1039
22 	XUL 	NS_ProcessPendingEvents(nsIThread*, unsigned int) 	xpcom/threads/nsThreadUtils.cpp:466
23 	XUL 	nsBaseAppShell::NativeEventCallback() 	widget/nsBaseAppShell.cpp:99
24 	XUL 	nsAppShell::ProcessGeckoEvents(void*) 	widget/cocoa/nsAppShell.mm:436
...

reports with this signature seem to get more common with webrender - primarily on macos and fennec. is this related to bug 1384181?

Comment 1

[Kartikaya Gupta (email:[e-mail])]

(In reply to [:philipp] from comment #0)
> reports with this signature seem to get more common with webrender -
> primarily on macos and fennec. is this related to bug 1384181?

I don't see any reports of this on Fennec. That should be impossible since webrender is not (and now cannot be) enabled on fennec. But yes, I see crashes on all three desktop platforms. It's not obvious to me from looking at the stack what the problem is, but it's something we'll need to fix.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

I don't see any more crashes in crash-stats that match this.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Looks like it was a volume issue, as it has come back on beta:

https://crash-stats.mozilla.com/report/index/7de1fd33-fd10-4614-8542-1cf1c0181024
https://crash-stats.mozilla.com/report/index/6f1f9508-6b7e-47eb-9666-ebe2a0181024
https://crash-stats.mozilla.com/report/index/da64909f-0cea-4022-aa8c-bb0850181024

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

We are failing to copy construct this array:

https://searchfox.org/mozilla-central/rev/e0c879c86b95bdc752b1dbff6088169735674e4a/gfx/layers/FrameMetrics.h#827

ScrollMetadata::mSnapInfo::mScrollSnapCoordinates::mHdr seems to point to an invalid location in the source data that we want to copy. This ScrollMetadata structure actually lives on the stack, a little further up the call stack:

https://searchfox.org/mozilla-central/rev/e0c879c86b95bdc752b1dbff6088169735674e4a/gfx/layers/wr/WebRenderScrollData.cpp#68-74

The Maybe<ScrollMetadata> is probably Nothing, just like in the ClipManager crash in bug 1471671.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1405814 - Avoid crash with WebRender when the scroll metadata is unavailable.

Similar to bug 1471671, we are seeing missing scroll metadata in cases
we do not expect that, and have been observing low volume crashes in the
wild as a result. It appears that in the non-WR path, it skips such items,
so we should probably do the same thing with WebRender. If it is a real
problem, we will hopefully get a reproducible test case from a user if
scrolling fails for them.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1cb5cc2e2d5
Avoid crash with WebRender when the scroll metadata is unavailable. r=kats

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/b1cb5cc2e2d5

Comment 8

[Ryan VanderMeulen [:RyanVM]]

I'm confused - this bug predates WebRender and we see crashes with this signature without WR on the stack:
https://crash-stats.mozilla.com/report/index/4c96f62c-a7de-4ba0-932b-5553a0181030

Is there a bug tracking these original crashes somewhere still?

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Huh, nevermind. I guess this bug always *was* for the WR crash. Andrew, can you please nominate this for Beta approval since the volume there seems non-negligible?

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9020096 [details]
Bug 1405814 - Avoid crash with WebRender when the scroll metadata is unavailable.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: May experience crashes while using WebRender.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The change is not risky. The worst case scrolling is would not work as the user expects, versus today where it crashes mid scroll.

String changes made/needed: N/A

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9020096 [details]
Bug 1405814 - Avoid crash with WebRender when the scroll metadata is unavailable.

[Triage Comment]
Fixes a crash for users opted into the WebRender experiments on Beta. Approved for 64.0b7.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1041bb5b5556