Closed
Bug 1502454
Opened 6 years ago
Closed 6 years ago
Crash in mozilla::layers::ClipManager::PushOverrideForASR
Categories
(Core :: Graphics: WebRender, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox63 | --- | unaffected |
firefox64 | --- | fixed |
firefox65 | + | fixed |
People
(Reporter: jseward, Assigned: aosmond)
References
(Blocks 1 open bug)
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
This bug was filed from the Socorro interface and is report bp-9c63eb52-8c44-40c9-97d6-b64d30181026. ============================================================= This is topcrash #4 in the Windows nightly 20181025220044, with 6 crashes in 3 different installs. Top 10 frames of crashing thread: 0 xul.dll void mozilla::layers::ClipManager::PushOverrideForASR gfx/layers/wr/ClipManager.cpp:102 1 xul.dll void mozilla::layers::ClipManager::BeginList gfx/layers/wr/ClipManager.cpp:61 2 xul.dll mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList gfx/layers/wr/WebRenderCommandBuilder.cpp:1379 3 xul.dll nsDisplayTransform::CreateWebRenderCommands layout/painting/nsDisplayList.cpp:8865 4 xul.dll mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList gfx/layers/wr/WebRenderCommandBuilder.cpp:1456 5 xul.dll nsDisplayOwnLayer::CreateWebRenderCommands layout/painting/nsDisplayList.cpp:7082 6 xul.dll nsDisplayFixedPosition::CreateWebRenderCommands layout/painting/nsDisplayList.cpp:7512 7 xul.dll mozilla::layers::WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList gfx/layers/wr/WebRenderCommandBuilder.cpp:1456 8 xul.dll mozilla::layers::WebRenderCommandBuilder::BuildWebRenderCommands gfx/layers/wr/WebRenderCommandBuilder.cpp:1320 9 xul.dll mozilla::layers::WebRenderLayerManager::EndTransactionWithoutLayer gfx/layers/wr/WebRenderLayerManager.cpp:300 =============================================================
Reporter | ||
Updated•6 years ago
|
Flags: needinfo?(aosmond)
Assignee | ||
Updated•6 years ago
|
Blocks: wr-stability, stage-wr-trains
Assignee | ||
Updated•6 years ago
|
Component: Graphics: Layers → Graphics: WebRender
Priority: -- → P2
Assignee | ||
Comment 1•6 years ago
|
||
ClipManager::PushOverrideForASR assumes we will always get a scroll ID. Presumably this is because we call ClipManager::BeginItem for the display item that triggered the override, which would have run: https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/gfx/layers/wr/ClipManager.cpp#222 Which would have ensured we have a scroll ID. This suggests that we hit one of these cases: https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/gfx/layers/wr/ClipManager.cpp#282,297,303
Assignee | ||
Comment 2•6 years ago
|
||
This is a consequence of fixing bug 1471671. We got a little further along, that's all.
Assignee | ||
Comment 3•6 years ago
|
||
This suggests to me, then, that we should have a method in ClipManager that checks if there is a scroll ID for an ASR's view ID, and if not, repeat with its parent, until we either find a scroll ID, or we have no parent and use the null scroll ID. This would mirror the behaviour in DefineScrollLayers.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → aosmond
Status: NEW → ASSIGNED
Flags: needinfo?(aosmond)
Assignee | ||
Comment 4•6 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d72b330a53a8e902195934ec7e330c7dff55df6b
Assignee | ||
Comment 5•6 years ago
|
||
In ClipManager::DefineScrollLayers, we don't always create a scroll ID for each ASR. We may fail to get the scroll metadata, or the it may not be scrollable, in which case we should use the scroll ID of its ancestor (or the root scroll ID if there is no ancestor). This should fix a crash where we simply assumed the leaf of an ASR tree will always have a valid scroll ID.
Updated•6 years ago
|
status-firefox63:
--- → unaffected
status-firefox64:
--- → affected
status-firefox65:
--- → affected
status-firefox-esr60:
--- → unaffected
tracking-firefox65:
--- → +
Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/34035754be84 Ensure we are consistent in how we define and use scroll IDs with WebRender. r=kats
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/34035754be84
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Assignee | ||
Comment 8•6 years ago
|
||
Comment on attachment 9022099 [details] Bug 1502454 - Ensure we are consistent in how we define and use scroll IDs with WebRender. [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1471671 User impact if declined: May occasionally crash the content process if 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: Bug 1471671 Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): We were crashing before, now the theoretical worst case would be the scrolling on the page is flaky. Only affects WebRender. String changes made/needed: N/A
Attachment #9022099 -
Flags: approval-mozilla-beta?
Comment 9•6 years ago
|
||
Comment on attachment 9022099 [details] Bug 1502454 - Ensure we are consistent in how we define and use scroll IDs with WebRender. webrender crash fix, approved for 64.0b8
Attachment #9022099 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/a7b21829921c
Updated•6 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•