Comment on attachment 610338 [details] [diff] [review] Patch v1 Review of attachment 610338 [details] [diff] [review]: ----------------------------------------------------------------- You can go as is - but do file a follow up about the inconsistency in AndroidBridge.cpp. Assign to me or blassey. Or better yet, assign it to yourself and fix it ;) ::: widget/android/AndroidBridge.cpp @@ +2108,5 @@ > +void > +AndroidBridge::LockScreenOrientation(const dom::ScreenOrientationWrapper& aOrientation) > +{ > + ALOG_BRIDGE("AndroidBridge::LockScreenOrientation"); > + mJNIEnv->CallStaticVoidMethod(mGeckoAppShellClass, jLockScreenOrientation, aOrientation.orientation); JNIEnv *env = AndroidBridge::GetJNIEnv(); if (!env) return; AutoLocalJNIFrame jniFrame(env, 1); You want to be using ^^ to get the jni env. The other instances of this mistake in this file should be corrected (since you are there).

I've added an unlock function. Pretty basic patch. I will merge it with the other one before landing.

(In reply to Doug Turner (:dougt) from comment #1) > JNIEnv *env = AndroidBridge::GetJNIEnv(); > if (!env) > return; > > AutoLocalJNIFrame jniFrame(env, 1); I've been able to find a bug (bug 713803) that explains quite clearly why the three first lines. However, I do not understand why we need to call AutoLocalJNIFrame every time. I would be happy to write the patch but I would prefer to understand what I'm doing ;)

Comment on attachment 610419 [details] [diff] [review] Part 2 - unlock function Review of attachment 610419 [details] [diff] [review]: ----------------------------------------------------------------- Fix or follow up w/ a bug ::: widget/android/AndroidBridge.cpp @@ +2116,5 @@ > +void > +AndroidBridge::UnlockScreenOrientation() > +{ > + ALOG_BRIDGE("AndroidBridge::UnlockScreenOrientation"); > + mJNIEnv->CallStaticVoidMethod(mGeckoAppShellClass, jUnlockScreenOrientation); same thing here. you don't want to use mJNIEnv directly. Also you want to add a java stack frame like the other calls to CallStatic*Method