Closed
Bug 752539
Opened 13 years ago
Closed 13 years ago
Some of the AutoLocalJNIFrame code usage could be more robust
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox14 fixed, firefox15 fixed, blocking-fennec1.0 +)
RESOLVED
FIXED
Firefox 15
People
(Reporter: kats, Assigned: kats)
Details
Attachments
(1 file, 1 obsolete file)
16.64 KB,
patch
|
blassey
:
review+
blassey
:
approval-mozilla-aurora+
|
Details | Diff | Splinter 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.
Attachment #621634 -
Flags: review?(blassey.bugs)
Comment 1•13 years ago
|
||
Comment on attachment 621634 [details] [diff] [review]
Patch
how about adding a GetEnv() function to AutoLocalJNIFrame and replacing the JNIEnv* arguments to these functions with a AutoLocalJNIFrame*?
Assignee | ||
Comment 2•13 years ago
|
||
Sounds reasonable to me. We still need to do null checks on it before using it though.
Attachment #621634 -
Attachment is obsolete: true
Attachment #621634 -
Flags: review?(blassey.bugs)
Attachment #621650 -
Flags: review?(blassey.bugs)
Updated•13 years ago
|
Attachment #621650 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 3•13 years ago
|
||
Try was green: https://tbpl.mozilla.org/?tree=Try&rev=cc9d881fee80
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/3bc06857dcd2
status-firefox15:
--- → fixed
Target Milestone: --- → Firefox 15
Comment 4•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•13 years ago
|
||
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
Attachment #621650 -
Flags: approval-mozilla-aurora?
Updated•13 years ago
|
blocking-fennec1.0: --- → ?
Updated•13 years ago
|
Attachment #621650 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•13 years ago
|
blocking-fennec1.0: ? → +
Assignee | ||
Comment 6•13 years ago
|
||
status-firefox14:
--- → fixed
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•