Crash on startup when using the basic compositor

VERIFIED FIXED in Firefox 28

Status

()

defect
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: nrc, Assigned: nrc)

Tracking

Trunk
mozilla28
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox28 verified)

Details

Attachments

(1 attachment)

And with d2d enabled.
Posted patch possible fixSplinter Review
This works (fixes the crash). And I think I am doing the right thing, but I am not sure. It does rather change how we get our surfaces for remote drawing though. I'm also not really sure how to test this, other than doing a bit of browsing and testing menus, tooltips, etc. So a thorough review would be appreciated. Also anything I should make sure to test.
Attachment #8342902 - Flags: review?(matt.woodrow)
Attachment #8342902 - Flags: review?(dvander)
Comment on attachment 8342902 [details] [diff] [review]
possible fix

Thanks, crash seems to be gone!
Attachment #8342902 - Flags: review?(dvander) → review+
Blocks: 947037
Comment on attachment 8342902 [details] [diff] [review]
possible fix

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

::: widget/windows/nsWindow.cpp
@@ +3528,5 @@
> +    if (mTransparentSurface) {
> +      surf = mTransparentSurface;
> +    }
> +  } 
> +  

Whitespace!

@@ +3556,5 @@
>  void
>  nsWindow::EndRemoteDrawing()
>  {
> +  if (mTransparencyMode == eTransparencyTransparent) {
> +    MOZ_ASSERT(gfxWindowsPlatform::GetPlatform()->GetRenderMode() != gfxWindowsPlatform::RENDER_DIRECT2D || mTransparentSurface);

Isn't it always invalid to have d2d + transparent windows?
Attachment #8342902 - Flags: review?(matt.woodrow) → review+
(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> Comment on attachment 8342902 [details] [diff] [review]
> possible fix
> 
> Review of attachment 8342902 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> @@ +3556,5 @@
> >  void
> >  nsWindow::EndRemoteDrawing()
> >  {
> > +  if (mTransparencyMode == eTransparencyTransparent) {
> > +    MOZ_ASSERT(gfxWindowsPlatform::GetPlatform()->GetRenderMode() != gfxWindowsPlatform::RENDER_DIRECT2D || mTransparentSurface);
> 
> Isn't it always invalid to have d2d + transparent windows?

I think that we are always using Cairo, but Cairo may be backed by d2d in which case the render mode is RENDER_DIRECT2D, but I'll double check.
And a backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/eb2a2c686cc7

https://tbpl.mozilla.org/php/getParsedLog.php?id=31653121&tree=Mozilla-Inbound
c:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/widget/windows/nsWindow.cpp(3519) : error C3861: 'IsRenderMode': identifier not found

Just normal debug-only bustage, or `ac_add_options --disable-unified-compilation` catching non-unified bustage?
https://hg.mozilla.org/mozilla-central/rev/396ef872cb8b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Nick, can you give us steps to reproduce for QA? Thanks!
Flags: needinfo?(ncameron)
(In reply to Liz Henry :lizzard from comment #10)
> Nick, can you give us steps to reproduce for QA? Thanks!

If memory serves you should set the following prefs to true:

layers.offmainthreadcomposition.enabled
layers.offmainthreadcomposition.force-basic
layers.acceleration.disabled

And then just start Firefox. You can confirm you ahve the correct configuration by going to about:support, where "GPU Accelerated Windows" in the graphics section should say something like "0/1 basic (OMTC)".
Flags: needinfo?(ncameron)
I tested on Windows 7 64bit, Windows 8.1 32bit and Windows Vista 32bit using Firefox 28 beta 7. 
Changed the prefs from comment 11 and restarted Firefox several times. There were no crashes.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.