Closed Bug 876208 Opened 7 years ago Closed 7 years ago

crash in gfxASurface::GetType() when switching tab after changing gfx.direct2d.disabled setting

Categories

(Core :: Graphics, defect, critical)

21 Branch
x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: maratreans, Assigned: milan)

Details

(Keywords: crash)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release)
Build ID: 20130511120803

Steps to reproduce:

1) Reset all gfx.* parameters to their defaults
2) Restart Firefox
3) Open this URL:
http://www.jstor.org/stable/pdfplus/1450272.pdf?acceptTC=true
4) In a new tab, go to about:config
5) Change gfx.direct2d.disabled to true
6) Switch back to pdf.js tab



Actual results:

Crash reports
bp-7cd7ec62-1b5a-4a02-bc4e-d50c92130526 
bp-715eea29-fea4-49a8-a59b-3c74c2130526 
bp-8c5dc82b-9150-4136-b769-18deb2130526 
bp-414501a3-7652-436d-9f7e-3378a2130526 

By following above steps I can reproduce crash at will


Expected results:

Firefox should not crash when changing about:config gfx settings
Why did not you restart browser after step 5)?

I think this bug should be wontfix.
(In reply to Alice0775 White from comment #1)
> I think this bug should be wontfix.
I think so.
Severity: normal → critical
Keywords: crash
Hardware: x86_64 → x86
Yes, after disabling HWA (in Options > Advanced or about:config), you need to restart the browser. In addition, there is no reason for the user to disable HWA and to continue browsing without restarting.

The only case I see is when the graphics driver crashes (which is rare).
If changing that setting requires a restart, would it not make more sense to make
it only take effect after restarting? Read the value at startup, store it
somewhere, and then subsequently use the stored value rather than the
about:config value.

The user doing stupid things should not make the application crash.
(In reply to Zachary W Martinez from comment #4)
> If changing that setting requires a restart, would it not make more sense to
> make
> it only take effect after restarting? Read the value at startup, store it
> somewhere, and then subsequently use the stored value rather than the
> about:config value.
> 
> The user doing stupid things should not make the application crash.

Yes, that would be the way to go.  It would also be nice if the user was given an indication as to needing to restart or not when they change the value.
Bas, is there a reason to allow Direct2D settings to be taken without restarting?  Is PDF.js a special case that doesn't work and the rest does and should?
Attachment #756190 - Flags: feedback?(bas)
Comment on attachment 756190 [details] [diff] [review]
Only evaluate gfx.direct2d.* preferences once, then require restart for any changes to take effect.

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

(In reply to Milan Sreckovic [:milan] from comment #6)
> Created attachment 756190 [details] [diff] [review]
> Only evaluate gfx.direct2d.* preferences once, then require restart for any
> changes to take effect.
> 
> Bas, is there a reason to allow Direct2D settings to be taken without
> restarting?  Is PDF.js a special case that doesn't work and the rest does
> and should?

Sadly, it's always 'kind of' worked but 'not really. I think I like this patch.

I'm not too fond of the implicit bool->int cast but even that I can live with as this patch is pretty simple which is nice.
Attachment #756190 - Flags: review+
Attachment #756190 - Flags: feedback?(bas)
Attachment #756190 - Flags: feedback+
Forgot to explicitly mention - this does stop the crash for me.
https://hg.mozilla.org/integration/mozilla-inbound/rev/574f888ef8de
Assignee: nobody → milan
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/574f888ef8de
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.