1157669 - Possible null pointer dereference in BasicLayerManager::PaintSelfOrChildren

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Nicolas Silva [:nical]]

https://hg.mozilla.org/mozilla-central/file/a5af73b32ac8/gfx/layers/basic/BasicLayerManager.cpp#l793

aGroupTarget can be null, and we have at least one branch depending on whether it is null before we call PaintSelfOrchildren here: https://hg.mozilla.org/mozilla-central/file/a5af73b32ac8/gfx/layers/basic/BasicLayerManager.cpp#l914

We should either
* early-return if aGroupTarget is null.
* MOZ_ASSERT that aGroupTarget is not null and fix the callers of PaintSelfOrChildren.

Comment 1

[Amanda Sambath]

I would like to fix this bug.

Comment 2

[Amanda Sambath]

Attachment: Build successful

Comment 3

[Nicolas Silva [:nical]]

Comment on attachment 8608010 [details] [diff] [review]
Build successful

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

Looks good, it'd be nice if you could add a MOZ_ASSERT(aTarget) at the top of BasicLayerManager::PaintLayer, to make it explicit that removing that branch, and the assertion in PaintSelfOrChildren hold because we expect PaintLayer to be called with a non-null target (an no need to add an assertion message for a simple null-check like this one).

::: gfx/layers/basic/BasicLayerManager.cpp
@@ +792,5 @@
>  void
>  BasicLayerManager::PaintSelfOrChildren(PaintLayerContext& aPaintContext,
>                                         gfxContext* aGroupTarget)
>  {
> +  MOZ_ASSERT(aGroupTarget, "aGroupTarget must not be null");

nit: no need to paraphrase the test if the comment doesn't add information.
MOZ_ASSERT(aGroupTarget); will compile just fine.

Comment 4

[Amanda Sambath]

Attachment: 1157669.patch

Comment 5

[Nicolas Silva [:nical]]

try push https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdc245885097

Comment 6

[Pulsebot]

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

Comment 7

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/fd77898fed81