1084423 - Stumbler: list of scanned wifis needs proper size limit

Status: RESOLVED FIXED

(Android Background Services Graveyard :: Geolocation, defect)

Description

[:garvan]

In MozStumbler testing, we discovered a bug with the size limit on wifi scans. 
This can result in excessively large collections for a single location. The existing guard is faulty as it checks the guards before converting the scanned information array to JSON. The scanned info array is where there should be a size limit.

The equivalent PR in MozStumbler
https://github.com/mozilla/MozStumbler/pull/1004

Comment 1

[:garvan]

Attachment: stumbler-wifi-count-guard-fix.patch

This is the fix from MozStumbler, ported to Fennec's stumbler service. It guards the cell and wifi list from exceeding a fixed size.

Comment 2

[Victor Ng [:vng] [:crankycoder]]

Comment on attachment 8507000 [details] [diff] [review]
stumbler-wifi-count-guard-fix.patch

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

This looks ok to me.

The relevant unit tests in MozStumbler that exercise this patch are here:

https://github.com/mozilla/MozStumbler/blob/3c208dd90cbf8329ea3de711acec6d63d1d901d7/android/src/test/java/org/mozilla/mozstumbler/service/stumblerthread/datahandling/DataStorageManagerTest.java#L60

and here:

https://github.com/mozilla/MozStumbler/blob/3c208dd90cbf8329ea3de711acec6d63d1d901d7/android/src/test/java/org/mozilla/mozstumbler/service/stumblerthread/datahandling/DataStorageManagerTest.java#L81

Comment 3

[:garvan]

Try green:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=470b7d6a8c5b

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/fx-team/rev/e91747e0ee2f

Comment 5

[:garvan]

Approval Request Comment
[Feature/regressing bug #]: Related to Fennec stumbler (big 1038843), this is a buffer size guard that was missed. The bug was found in MozStumbler (which is the host project for this library used in Fennec): https://github.com/mozilla/MozStumbler/pull/1004
[User impact if declined]: Potential to consume excessive memory as buffer is allowed to grow too large.
[Describe test coverage new/current, TBPL]: Comment #2 describes the tests that happen in MozStumbler for this case.
[Risks and why]: Excessive memory consumption.
[String/UUID change made/needed]: None

Comment 6

[Carsten Book [:Tomcat]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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