Last Comment Bug 752539 - Some of the AutoLocalJNIFrame code usage could be more robust
: Some of the AutoLocalJNIFrame code usage could be more robust
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: -- normal (vote)
: Firefox 15
Assigned To: Kartikaya Gupta (
: Sebastian Kaspari (:sebastian)
Depends on:
  Show dependency treegraph
Reported: 2012-05-07 09:40 PDT by Kartikaya Gupta (
Modified: 2012-05-15 13:18 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (8.46 KB, patch)
2012-05-07 09:40 PDT, Kartikaya Gupta (
no flags Details | Diff | Splinter Review
Patch v2 (16.64 KB, patch)
2012-05-07 10:24 PDT, Kartikaya Gupta (
blassey.bugs: review+
blassey.bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Kartikaya Gupta ( 2012-05-07 09:40:14 PDT
Created attachment 621634 [details] [diff] [review]

:gcp pointed out that functions like AndroidGeckoLayerClient::ActivateProgram(JNIEnv *env) don't obviously check for exceptions after the JNI invocation; the caller is expected to do this. In order to make the code more robust it is better to pass in the AutoLocalJNIFrame to these functions and do the exception check there rather than relying on the caller to do it.

The new behaviour is also consistent with some of the other functions like AndroidGeckoSurfaceView::GetSoftwareDrawBitmap.

This change is a follow-up to bug 748531 and contains no actual functional changes, just code robustification.
Comment 1 User image Brad Lassey [:blassey] (use needinfo?) 2012-05-07 09:56:38 PDT
Comment on attachment 621634 [details] [diff] [review]

how about adding a GetEnv() function to AutoLocalJNIFrame and replacing the JNIEnv* arguments to these functions with a  AutoLocalJNIFrame*?
Comment 2 User image Kartikaya Gupta ( 2012-05-07 10:24:48 PDT
Created attachment 621650 [details] [diff] [review]
Patch v2

Sounds reasonable to me. We still need to do null checks on it before using it though.
Comment 3 User image Kartikaya Gupta ( 2012-05-08 06:37:41 PDT
Try was green:

Landed on inbound:
Comment 4 User image Ed Morley [:emorley] 2012-05-08 11:20:07 PDT
Comment 5 User image Kartikaya Gupta ( 2012-05-11 16:59:38 PDT
Comment on attachment 621650 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: none; future patches may not apply cleanly
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): android-only, low risk
String changes made by this patch: none
Comment 6 User image Kartikaya Gupta ( 2012-05-15 13:18:33 PDT

Note You need to log in before you can comment on or make changes to this bug.