565120 - [D2D] Shacknews.com fully loads but browser hangs eventually crashing after scrolling down the page.

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Brian Carpenter [:geeknik]]

I know that there was a D2D bug that was just fixed regarding the background not loading correctly on www.shacknews.com, and now the background loads, but the site does not and then the browser hangs and eventually crashes (usually within a minute of the hang).

[@ strcmp | nsPrefBranch::RemoveObserver(char const*, nsIObserver*) ] 
http://crash-stats.mozilla.com/report/index/bp-d36885c3-0c79-468d-a188-8ec862100511

[@ dtoa]
http://crash-stats.mozilla.com/report/index/63117337-a1a5-4f38-a0c4-17dfc2100511

HTML5=on, DW=on, D2D=on, OOPP=on

Comment 1

[Brian Carpenter [:geeknik]]

Latest nightly build doesn't crash on this site any more.

Comment 2

[Brian Carpenter [:geeknik]]

Okay, I lied. The page loads 100% now, but when I scroll down, the browser hangs and crashes. 

[@ mozjs.dll@0x249e4 ]
http://crash-stats.mozilla.com/report/index/a22be51f-d948-41a0-bba8-97c582100512

Comment 3

[Mike Shaver (:shaver emeritus)]

That crash has a strange build-id, which is why it doesn't have symbols.  Are you using an hourly?

Comment 4

[Brian Carpenter [:geeknik]]

Build 20100512060819 is what I got when I did a check for updates. Before that I was running the 5/11 Nightly.

Comment 5

[Brian Carpenter [:geeknik]]

Attachment: WinDbg Log File

Not sure where that came from, but I did a check for updates again and got Build 20100512035952 which is the right nightly build for 5/12 and got this crash:

[@ dtoa]
http://crash-stats.mozilla.com/report/index/fa55b1d2-50f7-4d3f-8f6e-30ef22100512

I know there is a bug for [@ dtoa] but I'm not getting the crash anywhere but on Shacknews.com ever since the background fix for it landed. And looking at the output from WinDebug, I think this is going to be a D2D regression from that patch and not a Javascript issue. I've attached the debug log.

Comment 6

[Mike Shaver (:shaver emeritus)]

Can you confirm that by turning D2D off?  Would be good, then we can get it over to the gfx folks.

Comment 7

[Brian Carpenter [:geeknik]]

Browser definitely does not crash when I turn off D2D/DW rendering. Moving this to the appropriate component now.

I believe the landing of the patch in https://bugzilla.mozilla.org/show_bug.cgi?id=550398 is what is causing this issue.

Comment 8

[Bas Schouten (:bas.schouten)]

Presumably the image downsampling might cause memory corruption because we're doing something wrong here. That's the only reason I can imagine it appears to be in JS. Will investigate.

Comment 9

[[not reading bugmail]]

Confirmed crash, I just tested also:

http://crash-stats.mozilla.com/report/pending/885676ab-c890-4aa6-a80d-0a4892100513

[@ memcpy | D3D10Level9.dll@0x10bae ]

Comment 10

[Bas Schouten (:bas.schouten)]

This is most likely a dupe of bug 560050.

Comment 11

[Brian Carpenter [:geeknik]]

Doubt it Bas. Phonedog.com doesn't hang my machine with D2D/DW enabled.

Comment 12

[Bas Schouten (:bas.schouten)]

(In reply to comment #11)
> Doubt it Bas. Phonedog.com doesn't hang my machine with D2D/DW enabled.

Be sure to scroll around actively! Using the mouse and the slider manually, using the page up/down, the scrollwheel, etc! Any machine I've found (that's reasonably fast) seems to be able to hang on phonedog. But this was a couple of weeks ago.

Comment 13

[Brian Carpenter [:geeknik]]

Yeah, I've gone around the entire site multiple times since the bug was first reported and haven't been able to replicate it, even a couple of weeks ago. I have an ATI Radeon 5870 (latest Catalyst drivers) and I'm willing to bet you and the Phonedog.com bug reporter have Nvidia cards. =)

Comment 14

[Bas Schouten (:bas.schouten)]

(In reply to comment #13)
> Yeah, I've gone around the entire site multiple times since the bug was first
> reported and haven't been able to replicate it, even a couple of weeks ago. I
> have an ATI Radeon 5870 (latest Catalyst drivers) and I'm willing to bet you
> and the Phonedog.com bug reporter have Nvidia cards. =)

Sadly, no Radeon 5850. Pretty much identical to yours chip-wise :-). I'll need to try and see if I can still repro it in current builds tomorrow.

Comment 15

[[not reading bugmail]]

(In reply to comment #9)
> Confirmed crash, I just tested also:
> 
> http://crash-stats.mozilla.com/report/pending/885676ab-c890-4aa6-a80d-0a4892100513
> 
> [@ memcpy | D3D10Level9.dll@0x10bae ]

I crash on this with latest nightly, Nvidia 7150 Win7 32bit but PhoneDog.com doesn't crash/hang for me either.

Comment 16

[Bas Schouten (:bas.schouten)]

I've found the cause of the crash. The issue is we have a source image surface pattern that lies outside of the destination surface. This causes my D2D code to get confused and do something silly, working on it.

Comment 17

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Do not needlessly upload partially in both dimensions

This patch will make us always just upload the entire surface in the dimension where it -will- fit. Making this situation closer to normal codepaths. This also happens to fix this bug but more fixing is required and coming up.

Comment 18

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Deal with large image source not touching destination

This patch fixes the actual problem. When a large image surface source after transformation does not touch the destination surface at all, just upload the complete row/column edge in that dimension so that EXTEND_PAD works correctly. 

Additionally trigger software fallback to software if EXTEND_REPEAT or EXTEND_REFLECT is used with a Large image surface. We'll need some additional code to deal with this but it is not high priority (we pretty much never user these extend modes). I'll open a separate bug for this.

Comment 19

[Jeff Muizelaar [:jrmuizel]]

Both of these patches make the code pretty unreadable. Can we clean this stuff up somehow?

Comment 20

[[not reading bugmail]]

Looks like that second patch is similar to the first.. but with changes/adds.

Comment 21

[Bas Schouten (:bas.schouten)]

(In reply to comment #19)
> Both of these patches make the code pretty unreadable. Can we clean this stuff
> up somehow?

Erm, I'll see what I can do.

Comment 22

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Do not needlessly upload partially in both dimensions v2

Factored out into a separate function to address review comments.

Comment 23

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Deal with large image source not touching destination v2

Comment 24

[Bas Schouten (:bas.schouten)]

Attachment: Deal with large image source not touching destination v3

Last patch was wrong.

Comment 25

[Bas Schouten (:bas.schouten)]

Pushed http://hg.mozilla.org/mozilla-central/rev/9cdb2c5ff1e9.
Pushed http://hg.mozilla.org/mozilla-central/rev/59150e307d88.