1326292 - Crash in mozilla::widget::WinCompositorWidget::ClearTransparentWindow

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect, P4)

Description

[Ryan VanderMeulen [:RyanVM]]

+++ This bug was initially created as a clone of Bug #1284092 +++

Bug 1284092 apparently fixed one crash with this signature, but others remain. It's pretty low-frequency, though (~100 in the last 3 months). Most crashes are at 0x0, so maybe a null check is in order?

Comment 1

[Ryan VanderMeulen [:RyanVM]]

This missed the boat for 51 :(

Comment 2

[Jim Mathies [:jimm]]

We've been working on this as a side project, fwiw. Should have a fix up in the next couple weeks or so.

Comment 3

[Tracy Walker [:tracy]]

Attachment: null check of drawTarget WindowCompositorWidget

Should this simple null check be put through try?  If so, which tests should be run?

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 8834893 [details] [diff] [review]
null check of drawTarget WindowCompositorWidget

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

::: widget/windows/WinCompositorWidget.cpp
@@ +281,5 @@
>    IntSize size = mTransparentSurface->GetSize();
>    if (!size.IsEmpty()) {
>      RefPtr<DrawTarget> drawTarget =
>        gfxPlatform::CreateDrawTargetForSurface(mTransparentSurface, size);
> +    if (drawTarget == 0) {

nit-

if (!drawTarget) {
}

Comment 5

[Tracy Walker [:tracy]]

Attachment: null check of drawTarget WindowCompositorWidget(2)

fixed nit carryover r+ from :jimm

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/171570725b69
Add null check of drawTarget in WinCompositorWidget.cpp, to avoid crash. r=jimm

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/171570725b69

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora/Beta approval on this when you get a chance.

Comment 9

[Tracy Walker [:tracy]]

Comment on attachment 8834944 [details] [diff] [review]
null check of drawTarget WindowCompositorWidget(2)

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: chance of crashing
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no crashes had been reported against 54.0a1. we should see improvements on Aurora and Beta
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: it's a basic null dereference check
[String changes made/needed]: no

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8834944 [details] [diff] [review]
null check of drawTarget WindowCompositorWidget(2)

Fix a crash. Aurora53+.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/88fc18479321

Comment 12

[Julien Cristau [:jcristau] (back August 18)]

Comment on attachment 8834944 [details] [diff] [review]
null check of drawTarget WindowCompositorWidget(2)

avoid null dereference, beta52+

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ea512fbead9b

Comment 14

[Cornel Ionce [:noni]]

Setting qe-verify- per comment 9.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/ea512fbead9b