946532 - Crash on startup when using the basic compositor

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Nick Cameron [:nrc]]

And with d2d enabled.

Comment 1

[Nick Cameron [:nrc]]

Attachment: possible fix

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.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8342902 [details] [diff] [review]
possible fix

Thanks, crash seems to be gone!

Comment 3

[Matt Woodrow (:mattwoodrow)]

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?

Comment 4

[Nick Cameron [:nrc]]

(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.

Comment 5

[Nick Cameron [:nrc]]

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

Comment 6

[Nick Cameron [:nrc]]

And a fix: https://hg.mozilla.org/integration/mozilla-inbound/rev/ebc291b18b35

Comment 7

[Phil Ringnalda (:philor)]

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?

Comment 8

[Nick Cameron [:nrc]]

And again: https://hg.mozilla.org/integration/mozilla-inbound/rev/396ef872cb8b

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/396ef872cb8b

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Nick, can you give us steps to reproduce for QA? Thanks!

Comment 11

[Nick Cameron [:nrc]]

(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)".

Comment 12

[Bogdan Maris, Desktop Test Engineering]

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.