959842 - [OMTC][PGO] crash in mozilla::layers::DeprecatedContentClientDoubleBuffered::UpdateDestinationFrom(mozilla::layers::RotatedBuffer const&, nsIntRegion const&)

Status: VERIFIED FIXED

(Core :: Graphics: Layers, defect)

Description

[Alice0775 White]

This bug was filed from the Socorro interface and is 
report bp-b4298e3a-0ec2-4eac-b36c-063632140114.
=============================================================

Browser(Nightly29.0a1 and m-i PGO) crashes with OMTC enabled.

This crash happens only PGO build.

Steps To Reproduce:
1. Enable OMTC
   layers.offmainthreadcomposition.enabled = true
2. Restart Browser
3. Open http://www.youtube.com/watch?v=FEm8PZ_lUh8

Comment 1

[Alice0775 White]

Regression window(m-c pgo)
Good:
http://hg.mozilla.org/mozilla-central/rev/12d3ba62a599
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140112180004
Bad;
http://hg.mozilla.org/mozilla-central/rev/80a27198344a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140113060018
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=12d3ba62a599&tochange=80a27198344a


Regression window(m-i pgo)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/656e14ae9363
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140110090110
Crash
http://hg.mozilla.org/integration/mozilla-inbound/rev/7d5fcd8a3a99
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140110120002
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=656e14ae9363&tochange=7d5fcd8a3a99


Suspected : Bug 958375, Bug 958369, Bug 958437 ???

Comment 2

[Nick Cameron [:nrc]]

I think this needs a null check, I'll look into soon.

Comment 3

[Marc Auslander]

I've seen what appears to be the same crash in 2014_01_14 nightly.

https://bugzilla.mozilla.org/show_bug.cgi?id=959624

Comment 4

[Alice0775 White]

And also see Bug 960266

Comment 5

[Nick Cameron [:nrc]]

Attachment: patch

One of these I stole from one of your patches I reviewed in another bug, hope you don't mind!

Comment 6

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

Comment on attachment 8360667 [details] [diff] [review]
patch

Review of attachment 8360667 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me

Comment 7

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c040b200b336

Comment 8

[Nick Cameron [:nrc]]

This patch is necessary, but possibly not sufficient to fix the problem.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

I certainly get crashes still with the latest nightly.

Comment 11

[Mayank Bansal]

with the latest nightly, i get a crash while scrolling http://www.globalfundexchange.com/faqs/86-glossary/307-what-is-the-treynor-ratio with OMTC enabled.

https://crash-stats.mozilla.com/report/index/6b48d312-6a7c-43ca-bd40-0d6252140120

Comment 12

[Nick Cameron [:nrc]]

Was just testing the nightly and got a crash on the youtube link from the first comment, but the crash was in flash, not Firefox (https://crash-stats.mozilla.com/report/index/bb5b0483-4479-45ff-b280-706062140121)

Comment 15

[Nick Cameron [:nrc]]

Confirmed we are still crashing when visiting http://www.globalfundexchange.com/faqs/86-glossary/307-what-is-the-treynor-ratio at ReturnDrawTarget. I assume we just need a page with component alpha (and thus an OnWhite DT) to trigger this bug.

I don't see anything wrong here though. It is possible it is a PGO bug. Perhaps we could move ReturnDrawTarget from the header to the cpp so it is less likely to be inlined?

Comment 16

[Nick Cameron [:nrc]]

(In reply to Nick Cameron [:nrc] from comment #15)
> I don't see anything wrong here though. It is possible it is a PGO bug.
> Perhaps we could move ReturnDrawTarget from the header to the cpp so it is
> less likely to be inlined?

This did not help.

Comment 17

[Marc Auslander]

Are we sure that https://hg.mozilla.org/integration/mozilla-inbound/rev/c040b200b336 has made it into nightly?

I'm still seeing my version of the crash in the latest nightly as well.

Comment 18

[Nick Cameron [:nrc]]

(In reply to Marc Auslander from comment #17)
> Are we sure that
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c040b200b336 has made
> it into nightly?
> 
> I'm still seeing my version of the crash in the latest nightly as well.

Yes, I checked that.

Comment 19

[Nick Cameron [:nrc]]

However, I've also been spinning my own PGO builds to test my guesswork, and those didn't have the null-checks. Let me do another build...

(And thanks for the reminder!)

Comment 20

[Nick Cameron [:nrc]]

Attachment: patch

The renaming is not needed for the fix, I just found it awkward when fixing that the RotatedBuffer and ContentClient methods had the same name.

The bug is the missing '&' from the signature of ReturnDrawTargetToBuffer.

Comment 21

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8364824 [details] [diff] [review]
patch

Review of attachment 8364824 [details] [diff] [review]:
-----------------------------------------------------------------

Can you land this as two patches please :)

Comment 22

[Nick Cameron [:nrc]]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/2e93f0faa6ad
 https://hg.mozilla.org/integration/mozilla-inbound/rev/dc8f8287710b

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2e93f0faa6ad
https://hg.mozilla.org/mozilla-central/rev/dc8f8287710b

Comment 24

[Marc Auslander]

I'm still seeing the same crash with the 1/24 build of nightly.

Comment 25

[Nick Cameron [:nrc]]

(In reply to Marc Auslander from comment #24)
> I'm still seeing the same crash with the 1/24 build of nightly.

Give it another day, it might not have made it in time. (I have a working PGO build, so its possible I needed the .h -> .cpp changes too, but that seems unlikely)

Comment 26

[wojtekka]

Crashed again here
https://crash-stats.mozilla.com/report/index/0d2fab26-bc59-475a-a47f-cf5562140125
Latest nightly Built from http://hg.mozilla.org/mozilla-central/rev/9d650c07b547

Comment 27

[Alice0775 White]

(In reply to wojtekka from comment #26)
> Crashed again here
> https://crash-stats.mozilla.com/report/index/0d2fab26-bc59-475a-a47f-
> cf5562140125
> Latest nightly Built from
> http://hg.mozilla.org/mozilla-central/rev/9d650c07b547

Build ID is 20140124030216in your reports. It means not Latest nightly Built.
Try again with Latest nightly Build ID 20140125030205.

Comment 28

[Marc Auslander]

http://hg.mozilla.org/mozilla-central/rev/9e06d42c2a6a in buildconfig

problem appears fixed in my test profile

I'm turning omc back on for my working profile and report any troubles.

thanks.

Comment 29

[Manuela Muntean [Away]]

Verified as fixed with the STR from comment 0 on Win 7 x86, using latest Aurora.