851180 - mAckNeeded may not always be initialized

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Turns out there's a lot of AndroidGeckoEvent constructors, and most of them don't initialize mAckNeeded, but it's always read. Oops.

The patch also removes an unused constructor.

Comment 1

[Geoff Brown [:gbrown]]

Comment on attachment 724986 [details] [diff] [review]
Patch

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

LGTM. 

It looks like there are a lot of other AndroidGeckoEvent members that are not consistently initialized -- any concerns there?

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Yeah I was kind of worried about that too, but from a quick look around I didn't see any others that were used without being initialized. If we can come with some way to always guarantee that it would be nice but I think trying to initialize everything in all of the constructors might be overkill unless we run into actual problems.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

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

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 724986 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 846774
User impact if declined: low risk of seemingly-random failures, because operations that are supposed to by synchronous could end up being not synchronous
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): low-risk, fennec only
String or UUID changes made by this patch: none

Comment 5

[Phil Ringnalda (:philor)]

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

Comment 6

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9e118768b522