Closed Bug 1126918 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Graphics: Layers, defect)

All
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox35 --- unaffected
firefox36 + fixed
firefox37 + fixed
firefox38 --- fixed
fennec 36+ ---

People

(Reporter: blassey, Assigned: milan)

References

Details

(Keywords: crash, topcrash, topcrash-android-armv7)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-43df5924-e784-44cc-966c-bc01c2150121.
=============================================================
I'm blindly guessing bug 1066280 and failing to get a shared surface.
tracking-fennec: ? → 36+
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)...
Attachment #8555981 - Flags: review?(jgilbert) → review+
Comment on attachment 8558151 [details] [diff] [review]
Guard against nullptr. Carry r=jgilbert

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

Thanks!
https://hg.mozilla.org/mozilla-central/rev/ac6546e22203
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
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 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]:
Attachment #8558151 - Flags: approval-mozilla-beta?
Attachment #8558151 - Flags: approval-mozilla-aurora?
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.
Attachment #8558151 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
[Tracking Requested - why for this release]: Tracking this for 37.
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.
Attachment #8558151 - Flags: approval-mozilla-beta? → approval-mozilla-release+
https://hg.mozilla.org/releases/mozilla-beta/rev/89cfa8ff9fc5

Queued for Aurora too once I get some others ready to push.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: