Closed Bug 967587 Opened 7 years ago Closed 7 years ago

Keyboard is laggy and keys don't get displayed.

Categories

(Core :: Panning and Zooming, defect)

30 Branch
x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla30
blocking-b2g 1.4+
Tracking Status
firefox28 --- unaffected
firefox29 --- fixed
firefox30 --- fixed
b2g-v1.3 --- unaffected
b2g-v1.4 --- fixed

People

(Reporter: Nildeala, Assigned: roc)

References

Details

(Keywords: perf, qablocker, regression)

Attachments

(2 files)

Attached image 2014-02-04-20-58-36.png
I am on master branch, using a 2014-02-04 build.

What happens : the keyboard, whatever the used layout, is really laggy, both at displaying, behaving, and typing.
I'm attaching a screenshot here to show most of the keys don't even display.
This is APZ issue. The keyboard is reflowing all the time.
Depends on: gaia-apzc-2
Workaround: disable APZ and restart phone
Status: UNCONFIRMED → NEW
Ever confirmed: true
Note - this is different than bug 963584, as this happens consistently on production devices.
Blocks: gaia-apzc-2
blocking-b2g: --- → 1.4?
No longer depends on: gaia-apzc-2
Component: Gaia::Keyboard → Panning and Zooming
Product: Firefox OS → Core
Version: unspecified → 28 Branch
Version: 28 Branch → 30 Branch
Actually this isn't the bug I'm seeing - I'm not even seeing the keyboard appearing. I'm filing a different bug for this.
blocking-b2g: 1.4? → ---
Actually change that - the keyboard is coming up after reflashing. I am however definitely seeing the lag however. Seems to be fixed by turning APZC off.
blocking-b2g: --- → 1.4?
Duplicate of this bug: 967971
(In reply to Jan Jongboom [:janjongboom] from comment #1)
> This is APZ issue. The keyboard is reflowing all the time.

APZ doesn't cause reflows as far as i'm aware. Do you have any evidence to back up this claim?
Well, with APZ we see an incredible amount of reflows. Without APZ it works as expected (with the reflow tools in Firefox OS Developer Tools).
Keywords: perf, qablocker
We need whatever caused this regression backed out. All of keyboard UI automated tests are blocked by this bug.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #7)
> (In reply to Jan Jongboom [:janjongboom] from comment #1)
> > This is APZ issue. The keyboard is reflowing all the time.
> 
> APZ doesn't cause reflows as far as i'm aware. Do you have any evidence to
> back up this claim?

There is reflows with/without APZ. But it seems like something in the heuristic to create layers has changed recently and if you are affected only if you have APZ turned on. 

I'm going to investigate but my current suspect is bug 946502 as it changed a few things, and has some displayport checks into it. What I know is that a build from the 30/01 does not expose this issue.

I'm also seeing an incredible number of layers on a few other apps. On the keypad for example, or in Calendar when switching between tabs.
Dale is going to manually verify a backout of that bug. If it works, then we'll issue a backout of that patch.
Vivien actually already verified that backing out bug 946502 fixes this bug. He's going to back this out in a sec.
Can we #ifndef MOZ_WIDGET_GONK that line instead? That bug fixed a bunch of stuff for Metro and it would be nice to not lose it. Perhaps roc will disagree with me though.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #15)
> Can we #ifndef MOZ_WIDGET_GONK that line instead? That bug fixed a bunch of
> stuff for Metro and it would be nice to not lose it. Perhaps roc will
> disagree with me though.

I'm fine with that fwiw but I wonder if Metro is not suffering from a memory regression too ? Needinfo'ed mbrubeck
Flags: needinfo?(mbrubeck)
(In reply to Jason Smith [:jsmith] from comment #14)
> Vivien actually already verified that backing out bug 946502 fixes this bug.
> He's going to back this out in a sec.

Actually the backing out stuff is on Dale's plate :)
Sorry for the delay, verifying the fix for https://bugzilla.mozilla.org/show_bug.cgi?id=968250 so I can get to verifying this one (ie waiting for builds)
(In reply to Dale Harvey (:daleharvey) from comment #18)
> Sorry for the delay, verifying the fix for
> https://bugzilla.mozilla.org/show_bug.cgi?id=968250 so I can get to
> verifying this one (ie waiting for builds)

Dale, can you at least back this out and test on another device?   QA is seeing today's builds on the Buri phone, show up with no keyboard.   this is blocking our testing today.
Flags: needinfo?(dale)
(In reply to Tony Chung [:tchung] from comment #19)
> Dale, can you at least back this out and test on another device?   QA is
> seeing today's builds on the Buri phone, show up with no keyboard.   this is
> blocking our testing today.

Is the keyboard not showing up at all for you? This bug from what I understand is about the keyboard showing up, but being laggy. Perhaps the issue you are seeing is different.
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #16)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #15)
> > Can we #ifndef MOZ_WIDGET_GONK that line instead? That bug fixed a bunch of
> > stuff for Metro and it would be nice to not lose it. Perhaps roc will
> > disagree with me though.

Bug 946502 fixed user-visible layout regressions on many sites, in both Metro and Android (and probably B2G also).  We can discuss options for backing it out and relanding it with this issue fixed, but it *is* a blocker for Android and Metro, so we cannot *just* back it out.  (In retrospect I wish we had just backed out bug 919144 right after it landed and caused the original regressions.)

> I'm fine with that fwiw but I wonder if Metro is not suffering from a memory
> regression too ? Needinfo'ed mbrubeck

I haven't noticed a memory regression (and I don't see anything obvious comparing about:memory between beta and aurora), but I might just not have the right test case.
Flags: needinfo?(mbrubeck)
(In reply to Tony Chung [:tchung] from comment #19)
> (In reply to Dale Harvey (:daleharvey) from comment #18)
> > Sorry for the delay, verifying the fix for
> > https://bugzilla.mozilla.org/show_bug.cgi?id=968250 so I can get to
> > verifying this one (ie waiting for builds)
> 
> Dale, can you at least back this out and test on another device?   QA is
> seeing today's builds on the Buri phone, show up with no keyboard.   this is
> blocking our testing today.

I agree with kats that the keyboard not showing up at all can be a different issue. Keyboard has suffered multiple different issues since 2 weeks that has confused a bit the situation. One possibility thought it that the keyboard is going OOM because of all those extra layers.
Flashing keon with latest gecko is working fine, this is with an old gonk, testing against latest gonk is problematic due to https://bugzilla.mozilla.org/show_bug.cgi?id=968250

https://bugzilla.mozilla.org/show_bug.cgi?id=963584 is related and traced back to a gonk/android issue, I am about to travel and not comfortable backing out a patch with genuine fixes without being sure its the actual bug
Flags: needinfo?(dale)
blocking-b2g: 1.4? → 1.4+
With enabled APZ keyboard is laggy or doesn't appear at all. Its unpredictable what will happen after restart (keyboard present but laggy or not keyboard at all). When disable APZ everything works fine. 

Tested on Alcatel Fire got from T-mobile Poland
Build Identifier: 20140206012914
Git Commit Info: 2014-02-05 21:37:14 5a5884c5
Platform version: 30.0a1

Build done from scratch after removing everything from out/ and objdir-gecko/.
Whiteboard: [status:Waiting on roc's input]
(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #12)
> Seems to be the right suspect. Commenting the line at
> http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.
> cpp#2170 removes the extra layers.

We can back out just this hunk.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #26)
> (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #12)
> > Seems to be the right suspect. Commenting the line at
> > http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.
> > cpp#2170 removes the extra layers.
> 
> We can back out just this hunk.

Okay. I'll work with Vivien tomorrow to get that backed out.
Flags: needinfo?(roc)
Whiteboard: [status:Waiting on roc's input] → [status:needs patch]
I believe this won't reintroduce bug 946502. Something like this change is needed to prevent other bugs, but we can fix that separately.
Assignee: nobody → roc
Attachment #8371935 - Flags: review?(matt.woodrow)
Attachment #8371935 - Flags: review?(matt.woodrow) → review+
Whiteboard: [status:needs patch] → [status:on inbound]
http://hg.mozilla.org/mozilla-central/rev/706a4e2b081c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Depends on: 969354
Target Milestone: --- → mozilla30
I think this bug happened because the keyboard's viewport is overflow:hidden. So its root scrollframe was treated as inactive, and the animated geometry root for all its elements was the viewport. Then the offending code marked each ThebesLayer as "drawn all above" so no other content could be merged into it, effectively putting every element in its own ThebesLayer.

I'll try to fix this properly in bug 969354.
This seems to be affecting memory consumption with tiled layers on Gecko 29...
Can we backport this backout to 29 release?
Comment on attachment 8371935 [details] [diff] [review]
partial backout of bug 946502 part 3

[Approval Request Comment]
Bug caused by (feature/regressing bug #946502): 
User impact if declined: Lower Memory consumption
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): don't know
String or IDL/UUID changes made by this patch: none
Attachment #8371935 - Flags: approval-mozilla-aurora?
Attachment #8371935 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.