Fennec Native nightly crashes in mozilla::AndroidBridge::EnableLocationHighAccuracy when page requests GPS location

VERIFIED FIXED in Firefox 15

Status

()

Core
Widget: Android
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: KLB, Assigned: Arreth)

Tracking

(4 keywords)

14 Branch
mozilla14
ARM
Android
crash, regression, testcase, topcrash
Points:
---

Firefox Tracking Flags

(firefox15 verified, blocking-fennec1.0 +)

Details

(Whiteboard: [MTD][native-crash], crash signature, URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Web page or screen you were on when you saw the issue: 

When page requests user's location Fennec Native Nightly (2012-03-26) crashes

Steps to reproduce:
1. Go to http://envirochem.us/gpstest.htm
2. Click on start button.
3. Fennec crashes.

Reproduced on: Samsung Galaxy SII and Samsung Galaxy Tab 10.1

This does not affect Fennec Native Aurora.

Crash report ID (if applicable): I don't know how to provide this information.

Comment 1

5 years ago
(In reply to KLB from comment #0)
> Crash report ID (if applicable): I don't know how to provide this
> information.
Go to about:crashes and provide the latest crash ID (bp-...).
Severity: normal → critical
Whiteboard: [MTD] → [MTD][native-crash]
Reproduced with a Galaxy Nexus (Android 4.0.2), Nightly (03/27)

bp-b1a81ca0-6521-48b9-b7cf-942c92120327

Signature: [@ dalvik-LinearAlloc (deleted)@0x290466 ] (is this valid?)
Crash Signature: [@ dalvik-LinearAlloc (deleted)@0x290466 ]

Updated

5 years ago
status-firefox14: --- → affected

Updated

5 years ago
Crash Signature: [@ dalvik-LinearAlloc (deleted)@0x290466 ]

Updated

5 years ago
Crash Signature: [@ dalvik-LinearAlloc (deleted)@0x290466] [@ dvmCallMethodV]
Component: General → Widget: Android
Product: Fennec Native → Core
QA Contact: general → android
Summary: Fennec Native nightly crashes when page requests GPS location → Fennec Native nightly crashes in mozilla::AndroidBridge::EnableLocationHighAccuracy when page requests GPS location
Version: Firefox 14 → 14 Branch
(Reporter)

Comment 3

5 years ago
My crash profiles for this issue:

Samsung Galaxy Tab 10.1
http://crash-stats.mozilla.com/report/index/bp-635e89a3-f575-415c-af4b-925b72120327
http://crash-stats.mozilla.com/report/index/bp-1bee072d-18b1-45e5-9b4f-c59b32120327
http://crash-stats.mozilla.com/report/index/bp-36dcb31c-7474-4c89-952f-dadad2120327
http://crash-stats.mozilla.com/report/index/bp-9d2c56fb-f181-449d-8f60-5d44a2120327

Samsung Galaxy SII
http://crash-stats.mozilla.com/report/index/bp-b0c572e8-2662-4641-a543-1e11a2120327

Note: I only tested on the SII once to see if it was specific to my tablet.  Also, I first discovered this on a private site not publicly accessible site that I'm developing so I isolated the offending code to the demo page I linked to above.
Interesting.  Maps.google.com shares the location just fine.  Doesn't crash on desktop.
(Reporter)

Comment 5

5 years ago
Naoki,

I wonder if this is because my implementation keeps asking for location updates until the user "clicks" on the stop and record button.  Alternatively, it could be because I'm trying to force the browser to expire out existing location data and grab an updated location. 

I don't think most implementations of the location request will care that much about getting the most current/accurate data possible. In our implementation, we need as accurate of data as possible so we are having users grab a series of readings until they get multiple readings at the most accurate level their device is capable of and then averaging those readings. Ideally we want 5+ readings at 5 meter accuracy, but this isn't always possible.

Regardless of why I'm implementing the location functionality the way I am, I think we'll all agree that being able to crash the browser from a webpage is a bad thing that we wouldn't want to get past the nightly phase. If this were happening in a release version of Firefox, it would be considered a security vulnerability.

Hopefully my providing a demo page that isolates out my geolocation JS  helps eliminate code noise and allows for easier diagnosis of the problem.
Yes, I agree that crashing is bad; my comment was more of intrigue that it didn't crash on the geolocation from maps.google.com

The test page does help; we may just need to experiment further with the two scenarios that you had mentioned to narrow down the issue further.
Possible regression from bug 735011? Does a build from the 21st not crash?
(Reporter)

Comment 8

5 years ago
I ran some tests on the code and I found what causes the crashes. At line 95 in my script I have the following instruction:

navigator.geolocation.getCurrentPosition(geo_success, geo_error, {enableHighAccuracy:true, maximumAge:1000, timeout:240000});

If I change"enableHighAccuracy" to false, nightly stops crashing, which probably explains why Google Maps doesn't cause Nightly to crash.
Created attachment 609918 [details]
testcase

Thanks for finding out what causes the crash.
blocking-fennec1.0: --- → ?
Keywords: reproducible, testcase
Yes, excellent work, KLB!  Thank you for your report and analysis!
(Reporter)

Comment 11

5 years ago
My pleasure.  I can't fix the bugs, but I can certainly do my part to try and narrow down the the cause of bugs I report.

Updated

5 years ago
Keywords: reproducible

Comment 12

5 years ago
(In reply to Aaron Train [:aaronmt] from comment #7)
> Possible regression from bug 735011? Does a build from the 21st not crash?
Based on crash stats, it first appeared in 14.0a1/20120323, so it's indeed a regression from bug 735011.
Blocks: 735011
Crash Signature: [@ dalvik-LinearAlloc (deleted)@0x290466] [@ dvmCallMethodV] → [@ dvmCallMethodV] [@ dalvik-LinearAlloc (deleted)@0x28959e] [@ dalvik-LinearAlloc (deleted)@0x2895d6] [@ dalvik-LinearAlloc (deleted)@0x289756] [@ dalvik-LinearAlloc (deleted)@0x28ecc6] [@ dalvik-LinearAlloc (deleted)@0x290466] [@ dalvik-Lin…
Keywords: regression

Updated

5 years ago
Assignee: nobody → jdhaliwal
blocking-fennec1.0: ? → +

Comment 13

5 years ago
are there twitter or other social apps that tap into high accuracy gps?  those would make good test cases to confirm the fix.   In a few searches I spotted several survey sites where this might turn up as a blocker, but no popular social apps yet.
we think we know what the problem is. we can build a test for it.  josh is going to be working on a fix shortly.
(Assignee)

Comment 15

5 years ago
Created attachment 610436 [details] [diff] [review]
patch v1

From the different crash signatures that have been cited in this bug, this patch seems to contain the proper solution. Unfortunately (due to some reason I can't seem to figure out), the builds that I've made on this and other machines have been crashing with a completely different crash signature that I can't find a solution for. I didn't want to have everyone wait for me to figure this out, so if somebody else has time, if they could try using this patch and testing it out. Thanks in advance, and apologies.
Attachment #610436 - Flags: review?(doug.turner)
Comment on attachment 610436 [details] [diff] [review]
patch v1

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

::: widget/android/AndroidBridge.cpp
@@ +115,5 @@
>      jNotifyScreenShot = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "notifyScreenShot", "(Ljava/nio/ByteBuffer;III)V");
>      jAcknowledgeEventSync = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "acknowledgeEventSync", "()V");
>  
>      jEnableLocation = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "enableLocation", "(Z)V");
> +    jEnableLocation = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "enableLocationHighAccuracy", "(Z)V");

Shouldn't this be?

jEnableLocationHighAccuracy = (jmethodID) jEnv->GetStaticMethodID(jGeckoAppShellClass, "enableLocationHighAccuracy", "(Z)V");
Attachment #610436 - Flags: review?(doug.turner) → review-
(Assignee)

Comment 17

5 years ago
Created attachment 610501 [details] [diff] [review]
patch v2

:dougt - You are very right, my bad. Corrected this mistake.

Finally got my issue resolved. Tested, and crash no longer happens on my devices that I've tested on (Galaxy Nexus, and HTC Desire HD).
Attachment #610436 - Attachment is obsolete: true
Attachment #610501 - Flags: review?(doug.turner)

Updated

5 years ago
Crash Signature: [@ dvmCallMethodV] [@ dalvik-LinearAlloc (deleted)@0x28959e] [@ dalvik-LinearAlloc (deleted)@0x2895d6] [@ dalvik-LinearAlloc (deleted)@0x289756] [@ dalvik-LinearAlloc (deleted)@0x28ecc6] [@ dalvik-LinearAlloc (deleted)@0x290466] [@ dalvik-Lin… → [@ dvmCallMethodV] [@ _JNIEnv::CallStaticVoidMethod] [@ dalvik-LinearAlloc (deleted)@0x28959e] [@ dalvik-LinearAlloc (deleted)@0x2895d6] [@ dalvik-LinearAlloc (deleted)@0x28964e] [@ dalvik-LinearAlloc (deleted)@0x289756] [@ dalvik-LinearAlloc…
This crash has occurred on the latest Nightly build after I tapped on Share button on fandango.com:
https://crash-stats.mozilla.com/report/index/2b607e5a-2e98-4540-bf04-566bc2120329

--
Firefox 14.0a1 (2012-03-28)
Device: Samsung Galaxy S
OS: Android 2.2

Updated

5 years ago
Attachment #610501 - Flags: review?(doug.turner) → review+
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/bf171cf48355 for the XUL bustage in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=3e6a7f9fb34e (the native talos bustage was probably inherited from below, but you won't know until you push the next patch to Try).
Keywords: topcrash
(Assignee)

Comment 20

5 years ago
Created attachment 610711 [details] [diff] [review]
patch v3

updated patch to define enableLocationHighAccuracy in embedding/android/GeckoAppShell.java, so that XUL is not affected.
Attachment #610501 - Attachment is obsolete: true
Attachment #610711 - Flags: review?(doug.turner)

Updated

5 years ago
Crash Signature: [@ dvmCallMethodV] [@ _JNIEnv::CallStaticVoidMethod] [@ dalvik-LinearAlloc (deleted)@0x28959e] [@ dalvik-LinearAlloc (deleted)@0x2895d6] [@ dalvik-LinearAlloc (deleted)@0x28964e] [@ dalvik-LinearAlloc (deleted)@0x289756] [@ dalvik-LinearAlloc… → [@ dvmCallMethodV] [@ _JNIEnv::CallStaticVoidMethod] [@ dalvik-LinearAlloc (deleted)@0x28959e] [@ dalvik-LinearAlloc (deleted)@0x2895d6] [@ dalvik-LinearAlloc (deleted)@0x28964e] [@ dalvik-LinearAlloc (deleted)@0x289756] [@ dalvik-LinearAlloc…

Updated

5 years ago
Attachment #610711 - Flags: review?(doug.turner) → review+
Comment on attachment 610711 [details] [diff] [review]
patch v3

I removed: 
        mLocationHighAccuracy = enable;

Because, i don't think it will compile with this.
Attachment #610711 - Flags: review+ → review-
https://hg.mozilla.org/integration/mozilla-inbound/rev/33c419fc0fe4
https://hg.mozilla.org/mozilla-central/rev/33c419fc0fe4
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14

Updated

5 years ago
status-firefox14: affected → ---
No crash occurs if I perform the str from comment #0 on the latest Nightly. Closing bug as verified fixed on:

Firefox 15.0a1 (2012-05-23)
Device: Galaxy Nexus
OS: Android 4.0.2
Status: RESOLVED → VERIFIED
status-firefox15: --- → verified
You need to log in before you can comment on or make changes to this bug.