Closed Bug 1286709 Opened 4 years ago Closed 4 years ago

Missing return value check for GetApzc()

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: njn, Assigned: njn)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1363746)

Attachments

(1 file, 1 obsolete file)

ApplyAsyncTransformToScrollbarForContent() calls GetApzc() but doesn't null-check the result.
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: 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-
It was static analysis (Coverity).
Attachment #8770786 - Flags: review?(bugmail)
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+
https://hg.mozilla.org/mozilla-central/rev/994235fda74b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.