Closed Bug 1181135 Opened 4 years ago Closed 4 years ago

[APZ] Crashing on pages while clicking-selecting-dragging text [@mozilla::DisplayItemClip::operator=(mozilla::DisplayItemClip const&), @mozilla::DisplayItemClip::IntersectWith(mozilla::DisplayItemClip const&)]

Categories

(Core :: Layout, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox42 --- disabled
firefox43 + fixed
firefox-esr38 --- unaffected

People

(Reporter: mayankleoboy1, Assigned: mstange)

References

Details

(5 keywords)

Crash Data

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20150706030206

Steps to reproduce:

 Using my normal profile.   e10s+APZ+OMTC on win7.

On any page, click a few times on text, select it, drag it a bit.
Reproduces intermittently.


Actual results:

getting crashes.

https://crash-stats.mozilla.com/report/index/c87115f3-ea1e-4286-a342-d16742150707
https://crash-stats.mozilla.com/report/index/8ed219c6-2e42-482b-bfb4-fe7c22150707
https://crash-stats.mozilla.com/report/index/ad7c841f-38bd-4158-9fb6-94ea02150707

and so on.
 Had about 7 crashes so far. Started happening with this :  https://hg.mozilla.org/mozilla-central/rev/cef11c3e86c3  . Didnt happen with the previous nightly.
Adapter Description: Intel(R) HD Graphics 5500
Adapter Drivers: igdumdim64 igd10iumd64 igd10iumd64 igdumdim32 igd10iumd32 igd10iumd32
Adapter RAM: Unknown
Asynchronous Pan/Zoom: wheel input enabled
ClearType Parameters: Gamma: 2200 Pixel Structure: R ClearType Level: 100 Enhanced Contrast: 50
Device ID: 0x1616
Direct2D Enabled: true
DirectWrite Enabled: true (6.2.9200.17292)
Driver Date: 5-22-2015
Driver Version: 10.18.14.4222
GPU #2 Active: false
GPU Accelerated Windows: 2/2 Direct3D 11 (OMTC)
Subsys ID: 2336103c
Supports Hardware H264 Decoding: true
Vendor ID: 0x8086
WebGL Renderer: Google Inc. -- ANGLE (Intel(R) HD Graphics 5500 Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote: true
AzureCanvasBackend: direct2d 1.1
AzureContentBackend: direct2d 1.1
AzureFallbackCanvasBackend: cairo
AzureSkiaAccelerated: 0
Component: General → Graphics: Layers
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
best STR I have is to go to https://groups.google.com/forum/#!topic/mozilla.dev.platform/-cDq6DqBZ2k , and start quickly clicking on some text such that it is selected-deselected.
appears to be APZ related. I disabled it, and havent reproduced teh crash yet.
Component: Graphics: Layers → Panning and Zooming
Whiteboard: [APZ]
The crash stack does look like it's running through APZ-related code, but on the layout side of things. Moving component and CC'ing markus.
Component: Panning and Zooming → Layout
Blocks: 1178745
Keywords: regression
i had APZ disabled for the last 2 weeks, and no crashes. On a lark, i enabled APZ, and within 5 minutes, i got a crash.
https://crash-stats.mozilla.com/report/index/1a07e512-13ee-44ae-bebb-331832150722

Is there any plan to fix this?
Yeah, I can take a look. I tried reproducing it on Linux - haven't seen this crash yet but following the STR I'm hitting a different crash with an assertion in FrameLayerBuilder. I'll file a separate bug for that.
(In reply to mayankleoboy1 from comment #5)
> Is there any plan to fix this?

But to answer this question, yes, this is blocking bug 1178298 so we will need to fix this before we can enable APZ on aurora and beyond.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Crashing on pages while clicking-selecting-dragging text → [APZ] Crashing on pages while clicking-selecting-dragging text
Whiteboard: [APZ]
Tracking since this is a regression - though if it isn't going to move to aurora this cycle, we can move the tracking to 44.   Note to self and anyone else... the pref to flip is layers.async-pan-zoom.enabled.
Bug 1181135 - Copy DisplayItemClips in order to avoid dangling pointers. r?tn
Attachment #8655656 - Flags: review?(tnikkel)
Blocks: 1200580
No longer blocks: 1200580
Crash Signature: [@ mozilla::DisplayItemClip::operator=(mozilla::DisplayItemClip const&)] [@ mozilla::DisplayItemClip::IntersectWith(mozilla::DisplayItemClip const&)]
Keywords: crash, topcrash-win
Summary: [APZ] Crashing on pages while clicking-selecting-dragging text → [APZ] Crashing on pages while clicking-selecting-dragging text [@mozilla::DisplayItemClip::operator=(mozilla::DisplayItemClip const&), @mozilla::DisplayItemClip::IntersectWith(mozilla::DisplayItemClip const&)]
Blocks: 1200580
Duplicate of this bug: 1198798
Comment on attachment 8655656 [details]
MozReview Request: Bug 1181135 - Copy DisplayItemClips in order to avoid dangling pointers. r?tn

Bug 1181135 - Copy DisplayItemClips in order to avoid dangling pointers. r?tn
Comment on attachment 8655656 [details]
MozReview Request: Bug 1181135 - Copy DisplayItemClips in order to avoid dangling pointers. r?tn

https://reviewboard.mozilla.org/r/17969/#review16199
Attachment #8655656 - Flags: review?(tnikkel) → review+
Assignee: nobody → mstange
I set the status of 42 and 43 to "disabled" to reflect the state it will be when those versions are released -- that is to reflect whether we need to backport fixes to those branches once we have them. Looking at the dependency list on bug 1178298 I'm assuming we won't let it ride the train in 43 with only another couple of weeks before uplift, but if we do we should flip status 43 back to "affected".
In my x86-64 Linux build, a DisplayItemClip is 32 bytes in size and it has
a nsTArray member which carries additional heap memory in the common case.
It seems a bit wasteful that mAncestorClip and mAncestorClipForCaret are
stored permanently on the scroll frame when they are only used during
painting (AFAICT).  It looks like these members are not actually used
by the scroll frame code itself, it just serves them to other code through
ComputeScrollClip and ComputeFrameMetrics.

Would it be possible store these members somewhere else such that
the memory is reclaimed after painting is done?
Yes. I am working on a patch that does exactly that; the patch in this bug is more of a stop-gap solution.
(My new patch will store the clip in a ScrollClip struct that's allocated into the display list builder arena, and each display item will have a member that points to its ScrollClip.)
Sounds great, thanks Markus.
https://hg.mozilla.org/mozilla-central/rev/1dace02633b3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8655656 [details]
MozReview Request: Bug 1181135 - Copy DisplayItemClips in order to avoid dangling pointers. r?tn

Approval Request Comment
[Feature/regressing bug #]: bug 1148582 and bug 1178745
[User impact if declined]: crashes when dragging text, if the user manually enabled layers.async-pan-zoom.enabled
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: low, touches a feature that's disabled by default
[String/UUID change made/needed]: none
Attachment #8655656 - Flags: approval-mozilla-aurora?
Markus, why should we take this patch if the feature is disabled? Thanks
Flags: needinfo?(mstange)
It's a very attractive pref because of the large perf win it gives, so I wouldn't be surprised if many people start testing it out. Also, even if I don't see how, it's conceivable that attackers might find a way to exploit the bug even when the pref is off.
Flags: needinfo?(mstange)
No longer blocks: 1200580
Duplicate of this bug: 1200580
Comment on attachment 8655656 [details]
MozReview Request: Bug 1181135 - Copy DisplayItemClips in order to avoid dangling pointers. r?tn

Ok, let's take it then!
Thanks for the input.
Attachment #8655656 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Blocks: 1200580
Blocks: 1198492
You need to log in before you can comment on or make changes to this bug.