Closed
Bug 951959
Opened 11 years ago
Closed 11 years ago
GeckoScreenOrientationListener is invoked hundreds of times during page load
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 30
People
(Reporter: rnewman, Assigned: blassey)
References
Details
Attachments
(1 file)
4.08 KB,
patch
|
rnewman
:
review+
|
Details | Diff | Splinter Review |
org.mozilla.gecko.GeckoScreenOrientationListener.updateScreenOrientation(GeckoScreenOrientationListener.java:152)
org.mozilla.gecko.GeckoScreenOrientationListener.access$000(GeckoScreenOrientationListener.java:18)
org.mozilla.gecko.GeckoScreenOrientationListener$OrientationEventListenerImpl.onOrientationChanged(GeckoScreenOrientationListener.java:28)
android.view.OrientationEventListener$SensorEventListenerImpl.onSensorChanged(OrientationEventListener.java:143)
android.hardware.SystemSensorManager$SensorEventQueue.dispatchSensorEvent(SystemSensorManager.java:423)
android.os.MessageQueue.nativePollOnce(Native Method)
android.os.MessageQueue.next(MessageQueue.java:137)
android.os.Looper.loop(Looper.java:135)
android.app.ActivityThread.main(ActivityThread.java:5851)
java.lang.reflect.Method.invokeNative(Native Method)
java.lang.reflect.Method.invoke(Method.java:515)
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1067)
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:883)
dalvik.system.NativeStart.main(Native Method)
This call path is invoked something like 90 times when a page loads. And then it keeps getting invoked. I opened Reddit and just sat there for a few seconds, and counted 272 calls, as close together as 50msec, and as far apart as 13 seconds.
Given that updateScreenOrientation does a context dance, a WindowManager dance, and sometimes sends an event to Gecko, we should probably investigate this further in case it's affecting page load times.
Reporter | ||
Comment 1•11 years ago
|
||
Apparently this is known.
"Basically, Screen.getRotation returns the same result for portrait and reverse portrait, checked in log, but the orientation returned by the listener is alright. Will this affect the performance as it's running all the time, getting called 10+ times every second?"
http://helpdesk.metaio.com/questions/17105/billboards-upside-down-in-reverse-portrait-configuration
We might want to specify a rate:
http://developer.android.com/reference/android/view/OrientationEventListener.html#OrientationEventListener%28android.content.Context,%20int%29
Reporter | ||
Comment 2•11 years ago
|
||
Secondly, perhaps we should just handle orientation in onConfigurationChanged, rather than watching for fine-grained rotations? Or do we need to provide that to the web layer?
Reporter | ||
Comment 3•11 years ago
|
||
Also, this class is wrong.
http://chrisrisner.com/31-Days-of-Android--Day-11%E2%80%93Device-Orientation
---
“You can safely assume that ROTATION_0 and ROTATION_180 are portrait and ROTATION_90 and ROTATION_270 are landscape.“…Not
on an HTC ChaCha (Blackberry form factor android with a normally landscape
screen). On this device, ROTATION_0 is actually landscape. Also
when you rotate the screen to be portrait, it says “Not Portrait, Landscape!” and “Rotation
90”.
---
Look what we do (after I've tidied up but otherwise not changed the code):
if (rotation == Surface.ROTATION_0) {
mOrientation = SCREEN_ORIENTATION_PORTRAIT_PRIMARY;
} else if (rotation == Surface.ROTATION_180) {
mOrientation = SCREEN_ORIENTATION_PORTRAIT_SECONDARY;
} else if (rotation == Surface.ROTATION_270) {
mOrientation = SCREEN_ORIENTATION_LANDSCAPE_SECONDARY;
} else if (rotation == Surface.ROTATION_90) {
mOrientation = SCREEN_ORIENTATION_LANDSCAPE_PRIMARY;
} else {
Log.e(LOGTAG, "Unexpected value received! (" + rotation + ")");
return;
}
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #1)
> We might want to specify a rate:
>
> http://developer.android.com/reference/android/view/OrientationEventListener.
> html#OrientationEventListener%28android.content.Context,%20int%29
The default is the slowest rate (I'm assuming UI is faster than NORMAL)
Assignee | ||
Comment 5•11 years ago
|
||
Does this look better in terms of time in your profile?
Assignee: nobody → blassey.bugs
Attachment #8360221 -
Flags: review?(rnewman)
Reporter | ||
Comment 6•11 years ago
|
||
Comment on attachment 8360221 [details] [diff] [review]
use_orientation.patch
Review of attachment 8360221 [details] [diff] [review]:
-----------------------------------------------------------------
I'll try to find time to test this out soon.
::: mobile/android/base/GeckoScreenOrientationListener.java
@@ +157,5 @@
> + }
> + updateScreenOrientation(rotation * 90);
> + }
> +
> + private void updateScreenOrientation(int aOrienation) {
Nit: aOrien*t*ation.
Reporter | ||
Comment 7•11 years ago
|
||
Interesting. On my Moto x86 phone, onOrientationChanged is only ever called (a) on first launch of the activity, (b) if the activity is resumed after a screen orientation event.
Will have to test on my HTC ARM device so I can repro.
Oh, Android.
Reporter | ||
Comment 8•11 years ago
|
||
Comment on attachment 8360221 [details] [diff] [review]
use_orientation.patch
Review of attachment 8360221 [details] [diff] [review]:
-----------------------------------------------------------------
This looks good to me; it drops the handling time for each occurrence from 1-5msec (avg 2-3) down to 0-1msec.
r+ with typo addressed.
Thanks for tackling this!
::: mobile/android/base/GeckoScreenOrientationListener.java
@@ +157,5 @@
> + }
> + updateScreenOrientation(rotation * 90);
> + }
> +
> + private void updateScreenOrientation(int aOrienation) {
Nit: aOrien*t*ation.
Attachment #8360221 -
Flags: review?(rnewman) → review+
Comment 9•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
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
•