942318 - Remove erroneous lock in BasicCompositor::DrawQuad

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

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

Attachment: cleanup-locking.patch

The lock in BasicCompositor::DrawQuad is unnecessary, it's already locked earlier in the callstack. Luckily Lock() doesn't actually lock anything for the object it's static_casting to, but the cast looks dubious anyway.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8337044 [details] [diff] [review]
cleanup-locking.patch

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

::: gfx/layers/basic/BasicCompositor.cpp
@@ +119,5 @@
>                                                            mThebesImage->Stride(),
>                                                            mSize,
>                                                            mFormat);
>      }
> +  }

This change will break video, since the YCbCrDeprecatedTextureHostBasic override of EnsureSurface only initializes mThebesSuface, not mSurface.

I'd recommend fixing that function, since this changes looks good.

You just need to replace the new gfxImageSurface() call with Factory::CreateDataSourceSurface, and convert the following uses of it to the slightly different API/enum types.

Comment 2

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

Attachment: v2

Comment 3

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/27669314f120

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/27669314f120