Closed
Bug 1072376
Opened 10 years ago
Closed 10 years ago
Regression: Open/Close animation of the tabs tray and menu is sluggish
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox35 verified, fennec35+)
VERIFIED
FIXED
Firefox 35
People
(Reporter: aaronmt, Assigned: wesj)
References
Details
(Keywords: regression, reproducible)
Attachments
(3 files)
This is currently on fx-team, opening and closing the tabs tray is incredibly slow.
Possibly a regression from
8329fcecefdc Martyn Haigh — Bug 1056920 - Create grid based layout for tabs (r=lucasr)
Martyn, do you see this on fx-team? Can you confirm if it's that recent landing?
Flags: needinfo?(mhaigh)
Comment 1•10 years ago
|
||
Yeah, that landed on morning of 24th GMT. However, eyeballing it - I'm not seeing any different between my dev build and beta running on an N10.
Flags: needinfo?(mhaigh)
Reporter | ||
Comment 2•10 years ago
|
||
Sluggish animation
Comment 3•10 years ago
|
||
(In reply to Martyn Haigh (:mhaigh) from comment #1)
> Yeah, that landed on morning of 24th GMT. However, eyeballing it - I'm not
> seeing any different between my dev build and beta running on an N10.
This is on phones.
Comment 4•10 years ago
|
||
Martyn, could you have a look at this to see what's causing the jank here?
Assignee: nobody → mhaigh
Reporter | ||
Comment 5•10 years ago
|
||
Another video from my S5 http://people.mozilla.org/~atrain/misc/video.mp4
Comment 6•10 years ago
|
||
This is what I'm seeing with this build : http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/fx-team-android/latest/fennec-35.0a1.en-US.android-arm.apk
Comment 7•10 years ago
|
||
I'm able to reproduce the janky animation in latest m-c on a Nexus 7. I backed out bug 1072376 locally and it doesn't make any difference. We need a regression window here. This is a serious regression.
Keywords: regressionwindow-wanted
Reporter | ||
Comment 8•10 years ago
|
||
Also the menu is affected.
Summary: Regression: Open/Close animation of the tabs tray is sluggish → Regression: Open/Close animation of the tabs tray and menu is sluggish
Comment 9•10 years ago
|
||
(In reply to Aaron Train [:aaronmt] from comment #8)
> Also the menu is affected.
What menu? How exactly?
Flags: needinfo?(aaron.train)
Reporter | ||
Comment 10•10 years ago
|
||
Last good revision: 29c35d8e3d90
First bad revision: f1a36cb2bba6
Pushlog:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=29c35d8e3d90&tochange=f1a36cb2bba6
wjohnston@mozilla.com
Mon Sep 22 06:01:28 2014 +0000 f1a36cb2bba6 Wes Johnston — Bug 815682 - Lockscreen widget for guest sessions. r=bnicholson, rnewman
mhammond@skippinet.com.au
Reporter | ||
Comment 11•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #9)
> (In reply to Aaron Train [:aaronmt] from comment #8)
> > Also the menu is affected.
>
> What menu? How exactly?
Browser menu, latency on opening and closing
Blocks: fennec-lockscreen
Flags: needinfo?(aaron.train)
Reporter | ||
Comment 12•10 years ago
|
||
Here's the inbound window too
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d47f0d824c2d&tochange=6c05e3dc15ac
Comment 13•10 years ago
|
||
Seems unrelated to mhaigh patch. Unassigning for now.
Assignee: mhaigh → nobody
Assignee | ||
Comment 14•10 years ago
|
||
There's a change there in GeckoProfile.get that will cause us to check the keyguard state. Maybe its getting hit here and delaying things? The rest of these changes are mostly in onCreate or onNewIntent. I don't have any devices that reproduce this on me right now (and I'm stuck home sick), so someone else will have to look into this (or at least back out the patch and see if it helps).
Comment 15•10 years ago
|
||
(In reply to Wesley Johnston (:wesj) from comment #14)
> There's a change there in GeckoProfile.get that will cause us to check the
> keyguard state. Maybe its getting hit here and delaying things? The rest of
> these changes are mostly in onCreate or onNewIntent. I don't have any
> devices that reproduce this on me right now (and I'm stuck home sick), so
> someone else will have to look into this (or at least back out the patch and
> see if it helps).
I've tracked this down: GuestSession.configureWindow() is the culprit here.
Assignee | ||
Comment 16•10 years ago
|
||
Well, configureWindow just sets some lock screen flags on the window dealing with the keyguard. I'm digging through the Android source a bit, but I have a strange feeling that doing so resets hardware acceleration for the window? Strangely, the dialer screen does this completely differently:
http://androidxref.com/4.2.2_r1/xref/packages/apps/Phone/src/com/android/phone/InCallScreen.java#468
If you can test, you might try adding FLAG_HARDWARE_ACCELERATED as well here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GuestSession.java#74
(those addFlags calls should just be one call anyway. I'm not sure why I used two).
Hardware acceleration listed in the manifest is set when the window is attached:
http://androidxref.com/4.2.2_r1/xref/frameworks/base/core/java/android/app/Activity.java#5090
All passing that does is set a flag in the window, which is only used when setting up a subwindow here:
http://androidxref.com/4.2.2_r1/xref/frameworks/base/core/java/android/view/Window.java#531
I don't know enough about windows and subwindows to know who that will affect though. The flags set via window.set/addFlags() seem to be stored separately, but trickle through to
http://androidxref.com/4.2.2_r1/xref/frameworks/base/core/java/android/view/WindowManagerGlobal.java#269
where they are applied to the root view of an activity. I don't understand the window/subwindow stuff well enough yet to see the hole, but at this point I'd probably just check that adding the hardware accel flag to configureWindow works to see if I'm on the right track at all. If you don't have time, I'll see if MTV QA has a device tomorrow.
Comment 17•10 years ago
|
||
Tried enabling hardware acceleration but it didn't make any difference. Logcat is showing a bunch of strictmode exceptions on every layout pass in the UI:
D/StrictMode( 3221): StrictMode policy violation; ~duration=25 ms: android.os.StrictMode$StrictModeDiskReadViolation: policy=287 violation=2
D/StrictMode( 3221): at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1135)
D/StrictMode( 3221): at android.database.sqlite.SQLiteConnection.applyBlockGuardPolicy(SQLiteConnection.java:1041)
D/StrictMode( 3221): at android.database.sqlite.SQLiteConnection.executeForCursorWindow(SQLiteConnection.java:842)
D/StrictMode( 3221): at android.database.sqlite.SQLiteSession.executeForCursorWindow(SQLiteSession.java:836)
D/StrictMode( 3221): at android.database.sqlite.SQLiteQuery.fillWindow(SQLiteQuery.java:62)
D/StrictMode( 3221): at android.database.sqlite.SQLiteCursor.fillWindow(SQLiteCursor.java:144)
D/StrictMode( 3221): at android.database.sqlite.SQLiteCursor.getCount(SQLiteCursor.java:133)
D/StrictMode( 3221): at android.database.AbstractCursor.moveToPosition(AbstractCursor.java:197)
D/StrictMode( 3221): at android.database.AbstractCursor.moveToFirst(AbstractCursor.java:237)
D/StrictMode( 3221): at com.android.server.LockSettingsService.readFromDb(LockSettingsService.java:420)
D/StrictMode( 3221): at com.android.server.LockSettingsService.getBoolean(LockSettingsService.java:202)
D/StrictMode( 3221): at com.android.internal.widget.ILockSettings$Stub.onTransact(ILockSettings.java:94)
D/StrictMode( 3221): at android.os.Binder.execTransact(Binder.java:404)
D/StrictMode( 3221): at android.os.BinderProxy.transact(Native Method)
D/StrictMode( 3221): at com.android.internal.policy.IKeyguardService$Stub$Proxy.isSecure(IKeyguardService.java:253)
D/StrictMode( 3221): at com.android.internal.policy.impl.keyguard.KeyguardServiceWrapper.isSecure(KeyguardServiceWrapper.java:53)
D/StrictMode( 3221): at com.android.internal.policy.impl.keyguard.KeyguardServiceDelegate.isSecure(KeyguardServiceDelegate.java:190)
D/StrictMode( 3221): at com.android.internal.policy.impl.PhoneWindowManager.isKeyguardSecure(PhoneWindowManager.java:4397)
D/StrictMode( 3221): at com.android.internal.policy.impl.PhoneWindowManager.finishPostLayoutPolicyLw(PhoneWindowManager.java:3534)
D/StrictMode( 3221): at com.android.server.wm.WindowManagerService.performLayoutAndPlaceSurfacesLockedInner(WindowManagerService.java:9026)
D/StrictMode( 3221): at com.android.server.wm.WindowManagerService.performLayoutAndPlaceSurfacesLockedLoop(WindowManagerService.java:8179)
D/StrictMode( 3221): at com.android.server.wm.WindowManagerService.performLayoutAndPlaceSurfacesLocked(WindowManagerService.java:8121)
D/StrictMode( 3221): at com.android.server.wm.WindowManagerService.relayoutWindow(WindowManagerService.java:3016)
D/StrictMode( 3221): at com.android.server.wm.Session.relayout(Session.java:190)
D/StrictMode( 3221): at android.view.IWindowSession$Stub.onTransact(IWindowSession.java:235)
D/StrictMode( 3221): at com.android.server.wm.Session.onTransact(Session.java:125)
D/StrictMode( 3221): at android.os.Binder.execTransact(Binder.java:404)
D/StrictMode( 3221): at dalvik.system.NativeStart.run(Native Method)
It seems the UI is making blocking calls on the KeyguardService in the main thread. FLAG_DISMISS_KEYGUARD is what's causing this regression btw.
Updated•10 years ago
|
Assignee: nobody → wjohnston
tracking-fennec: ? → 35+
Assignee | ||
Comment 18•10 years ago
|
||
There doesn't seem to be a Nexus 7 available in MTV right now (which is silly). I ordered one.
Assignee | ||
Comment 19•10 years ago
|
||
Finally managed to repro this. This makes us only set these flags when you're running over the keyguard. That can happen if your running in Guest mode, or if your keyguard is just not a secure one (i.e. swipe to unlock!).
In the later case, onResume is actually called twice, and the second time the keyguard is no longer showing so it actually removes these flags there (but the second onResume call never happens if we don't set the flags once). So really, you'll only see the jank if
1.) The keyguard is showing and
2.) You're in guest mode.
That's not perfect, but its better than what we had... (and it basically gets us back to parity with builds that didn't support running over the keyguard at all).
Attachment #8495550 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 20•10 years ago
|
||
I filed an Android issue for this too: https://code.google.com/p/android/issues/detail?id=76673&thanks=76673&ts=1411683474
Comment 21•10 years ago
|
||
Comment on attachment 8495550 [details] [diff] [review]
Patch
Review of attachment 8495550 [details] [diff] [review]:
-----------------------------------------------------------------
Have you tried always removing the DISMISS_KEYGUARD flags in onResume() to see if you can avoid the performance issues in all cases? DISMISS_KEYGUARD looks like a flags that will be only used when the activity starts. It's a dangerous assumption to make but maybe worth trying?
::: mobile/android/base/BrowserApp.java
@@ +750,5 @@
> + // is not showing since it affects Android's layout of the window.
> + if (manager.isKeyguardLocked() && (GeckoProfile.get(this).inGuestMode() || !manager.isKeyguardSecure())) {
> + GuestSession.configureWindow(getWindow());
> + } else {
> + GuestSession.unconfigureWindow(getWindow());
I remember you mentioned onResume being called twice when keyguard is not secure. Maybe document this here?
Attachment #8495550 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #21)
> Have you tried always removing the DISMISS_KEYGUARD flags in onResume() to
> see if you can avoid the performance issues in all cases? DISMISS_KEYGUARD
> looks like a flags that will be only used when the activity starts. It's a
> dangerous assumption to make but maybe worth trying?
I can try just that one. I removed both at one point (in DelayedStartup) and saw the keyguard reappear.
Assignee | ||
Comment 23•10 years ago
|
||
(In reply to Lucas Rocha (:lucasr) from comment #21)
> Have you tried always removing the DISMISS_KEYGUARD flags in onResume() to
> see if you can avoid the performance issues in all cases? DISMISS_KEYGUARD
> looks like a flags that will be only used when the activity starts. It's a
> dangerous assumption to make but maybe worth trying?
Flipping this causes the activity to be hidden. Will land as is for now.
Assignee | ||
Comment 24•10 years ago
|
||
Comment 25•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Reporter | ||
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
status-firefox35:
--- → verified
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
•