1126918 - crash in mozilla::layers::CanvasClientSharedSurface::Update(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, mozilla::layers::ClientCanvasLayer*)

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

This bug was filed from the Socorro interface and is 
report bp-43df5924-e784-44cc-966c-bc01c2150121.
=============================================================

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

I'm blindly guessing bug 1066280 and failing to get a shared surface.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Guard against nullptr. r=jgilbert

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8555981 [details] [diff] [review]
Guard against nullptr. r=jgilbert

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

::: gfx/layers/client/CanvasClient.cpp
@@ +332,5 @@
>  static TemporaryRef<gl::ShSurfHandle>
>  CloneSurface(gl::SharedSurface* src, gl::SurfaceFactory* factory)
>  {
> +  RefPtr<gl::ShSurfHandle> dest = factory->NewShSurfHandle(src->mSize);
> +  if (dest) {

Generally, prefer early-outs for failure cases.

if (!dest)
  return nullptr;

// else if (dest)...

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Guard against nullptr. Carry r=jgilbert

Early exit instead.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=93a8b1b4ba9d

Comment 5

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8558151 [details] [diff] [review]
Guard against nullptr. Carry r=jgilbert

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

Thanks!

Comment 6

[Carsten Book [:Tomcat]]

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

Comment 7

[(PTO June 19-July 7) Ryan VanderMeulen]

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

Comment 8

[Robert Kaiser]

Kevin, this sounds like it is a fix to one of the topcrashers on 36 beta, we might want to get it into 37 at least.

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8558151 [details] [diff] [review]
Guard against nullptr. Carry r=jgilbert

I'll ask for Beta as well, but I'm happy with whatever we decide, if it's too late for 36.

Approval Request Comment
[Feature/regressing bug #]: 1066280
[User impact if declined]: As I undersetand, a high volume crash.
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Low.  We're checking a null pointer, and the caller handles it.
[String/UUID change made/needed]:

Comment 10

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

Comment on attachment 8558151 [details] [diff] [review]
Guard against nullptr. Carry r=jgilbert

Taking this for aurora but leaving this to Sylvestre to make the call on beta.

Comment 11

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

[Tracking Requested - why for this release]: Tracking this for 37.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8558151 [details] [diff] [review]
Guard against nullptr. Carry r=jgilbert

[Triage Comment]
Looks like super safe and if we can take an important top crash fix. It is great.

Comment 13

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-beta/rev/89cfa8ff9fc5

Queued for Aurora too once I get some others ready to push.

Comment 14

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-release/rev/89cfa8ff9fc5

Comment 15

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b8478050a2ee