Closed Bug 1416868 Opened 4 years ago Closed 4 years ago
Crash in ns
TArray _Impl<T>::operator==<T> | mozilla::Mask Layer User Data::operator==
This bug was filed from the Socorro interface and is report bp-7883f933-ee6b-4169-b60a-534b30171113. ============================================================= Top 10 frames of crashing thread: 0 xul.dll nsTArray_Impl<mozilla::DisplayItemClip::RoundedRect, nsTArrayInfallibleAllocator>::operator==<nsTArrayInfallibleAllocator> xpcom/ds/nsTArray.h:969 1 xul.dll mozilla::MaskLayerUserData::operator== layout/painting/FrameLayerBuilder.cpp:1593 2 xul.dll mozilla::ContainerState::CreateMaskLayer layout/painting/FrameLayerBuilder.cpp:6363 3 xul.dll mozilla::ContainerState::SetupMaskLayer layout/painting/FrameLayerBuilder.cpp:6324 4 xul.dll mozilla::ContainerState::ProcessDisplayItems layout/painting/FrameLayerBuilder.cpp:4317 5 xul.dll mozilla::FrameLayerBuilder::BuildContainerLayerFor layout/painting/FrameLayerBuilder.cpp:5678 6 xul.dll nsDisplayOpacity::BuildLayer layout/painting/nsDisplayList.cpp:6261 7 xul.dll mozilla::FrameLayerBuilder::AddPaintedDisplayItem layout/painting/FrameLayerBuilder.cpp:4700 8 xul.dll mozilla::ContainerState::FinishPaintedLayerData<<lambda_593feee20c50667d87c0fe33c81e3e5b> > layout/painting/FrameLayerBuilder.cpp:3164 9 xul.dll mozilla::PaintedLayerDataNode::PopPaintedLayerData layout/painting/FrameLayerBuilder.cpp:2869 ============================================================= crash reports with this signature are rising in volume during the 58 cycle - most of the time they are happening in the content process.
Maybe we could have a nullptr checking before de-reference the userData. https://hg.mozilla.org/releases/mozilla-beta/annotate/1c336e874ae8/layout/painting/FrameLayerBuilder.cpp#l6363
Assignee: nobody → hshih
Status: NEW → ASSIGNED
nullptr check sounds great - should we add a gfxCriticalError/Note or some such if this is really unexpected? That way we won't crash, but we can see if something went wrong.
4 years ago
The gecko have two types of mask layer: css mask layer and the regular mask layer. The hash key of ContainerState::mRecycledMaskImageLayers doesn't contain mask type. So, we might get a css mask layer when we try to get a regular mask layer. Then, we will get a nullptr of userData. This patch add a userData checking in ContainerState::CreateOrRecycleMaskImageLayerFor() to avoid the problem. MozReview-Commit-ID: EEUhkctqwR2
Attachment #8928933 - Flags: review?(matt.woodrow)
Comment on attachment 8928933 [details] [diff] [review] make sure we could always the specific userData from mask layer. Review of attachment 8928933 [details] [diff] [review]: ----------------------------------------------------------------- The regular mask layer and css mask layer will be collected here: https://hg.mozilla.org/mozilla-central/annotate/fc194660762d1b92e1679d860a8bf41116d0f54f/layout/painting/FrameLayerBuilder.cpp#l4922 We might have the collision for these mask layers. Then, we can't get the correct user data.
Attachment #8928933 - Flags: review?(matt.woodrow) → review+
update the patch comment.
Attachment #8929288 - Flags: review+
Attachment #8928933 - Attachment is obsolete: true
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5d0fbb755131 make sure we could always get the specific userData from mask layer. r=mattwoodrow
Hi Jerry, Do you think this is worth uplifting to 58 if it's not too risky?
Yes, I don't see the crash from 59 after 11/18(comment 9).
Comment on attachment 8929288 [details] [diff] [review] make sure we could always get the specific userData from mask layer. v2 Approval Request Comment [Feature/Bug causing the regression]: Maybe it a regression from bug 1234485. It create a new type of mask. But that's landed in mozilla52. I don't know why the crash rate becomes higher in mozilla58. [User impact if declined]: Hit Crash at content side when gecko try to recycle a mask layer. [Is this code covered by automated tests?]: Yes. We have reftest for mask. [Has the fix been verified in Nightly?]: I think yes. I don't see the new crash after this patch landed. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: It add a nullptr checking for the result from a recycler. I think it doesn't change too much. [String changes made/needed]: none
Attachment #8929288 - Flags: approval-mozilla-beta?
Comment on attachment 8929288 [details] [diff] [review] make sure we could always get the specific userData from mask layer. v2 Fix a crash. Beta58+.
Attachment #8929288 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.