Closed Bug 1416868 Opened 4 years ago Closed 4 years ago

Crash in nsTArray_Impl<T>::operator==<T> | mozilla::MaskLayerUserData::operator==


(Core :: Graphics, defect, P3)

58 Branch



Tracking Status
firefox-esr52 --- wontfix
firefox57 --- wontfix
firefox58 --- fixed
firefox59 --- fixed


(Reporter: philipp, Assigned: jerry)


(Blocks 1 open bug)


(Keywords: crash, regression)

Crash Data


(1 file, 1 obsolete file)

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&lt;mozilla::DisplayItemClip::RoundedRect, nsTArrayInfallibleAllocator&gt;::operator==&lt;nsTArrayInfallibleAllocator&gt; 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&lt;&lt;lambda_593feee20c50667d87c0fe33c81e3e5b&gt; &gt; 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.
Assignee: nobody → hshih
Blocks: 1388995
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.
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:

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+
Attachment #8928933 - Attachment is obsolete: true
Pushed by
make sure we could always get the specific userData from mask layer. r=mattwoodrow
Keywords: checkin-needed
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Hi Jerry,
Do you think this is worth uplifting to 58 if it's not too risky?
Flags: needinfo?(hshih)
Yes, I don't see the crash from 59 after 11/18(comment 9).
Flags: needinfo?(hshih)
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]: 

[List of other uplifts needed for the feature/fix]:

[Is the change risky?]:

[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]:
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.