Missing return value check for GetApzc()

RESOLVED FIXED in Firefox 50

Status

()

Core
Graphics: Layers
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: njn, Assigned: njn)

Tracking

(Blocks: 1 bug, {coverity})

Trunk
mozilla50
coverity
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: CID 1363746)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

a year ago
ApplyAsyncTransformToScrollbarForContent() calls GetApzc() but doesn't null-check the result.
(Assignee)

Comment 1

a year ago
Created attachment 8770750 [details] [diff] [review]
Add a missing null check for a GetApzc() call

kats, I hope an early return is appropriate here, but I'm not certain.

All the other calls to GetApzc() are null-checked.
Attachment #8770750 - Flags: review?(bugmail)
(Assignee)

Updated

a year ago
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Comment on attachment 8770750 [details] [diff] [review]
Add a missing null check for a GetApzc() call

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

I think it should be safe to MOZ_ASSERT that it is not null there. The LayerMetrics passed in to this function must retyrn true from IsValid(), based on the call site, and if you trace backwards all the valid LayerMetrics that reach that point must have returned true from LayerIsScrollbarTarget. That function checks that the apzc is non-null.

Were you actually seeing a case where this caused a crash? If so I'll need to look more carefully but otherwise I would prefer the assert to an early return.
Attachment #8770750 - Flags: review?(bugmail) → review-
(Assignee)

Comment 3

a year ago
Created attachment 8770786 [details] [diff] [review]
Add a missing null check for a GetApzc() call

It was static analysis (Coverity).
Attachment #8770786 - Flags: review?(bugmail)
(Assignee)

Updated

a year ago
Attachment #8770750 - Attachment is obsolete: true
Comment on attachment 8770786 [details] [diff] [review]
Add a missing null check for a GetApzc() call

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

r=me, please update commit message also
Attachment #8770786 - Flags: review?(bugmail) → review+
(Assignee)

Comment 5

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/994235fda74bd1d05003546f98d23e46f6ceecef
Bug 1286709 - Assert the non-nullness of an GetApzc() call's return value. r=kats.

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/994235fda74b
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.