967587 - Keyboard is laggy and keys don't get displayed.

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[William Dorffer [:Nildeala]]

Attachment: 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.

Comment 1

[Jan Jongboom [:janjongboom] (inactive)]

This is APZ issue. The keyboard is reflowing all the time.

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

Workaround: disable APZ and restart phone

Comment 3

[Jason Smith [:jsmith]]

Note - this is different than bug 963584, as this happens consistently on production devices.

Comment 4

[Jason Smith [:jsmith]]

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.

Comment 5

[Jason Smith [:jsmith]]

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.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

(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?

Comment 8

[Jan Jongboom [:janjongboom] (inactive)]

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).

Comment 10

[Jason Smith [:jsmith]]

We need whatever caused this regression backed out. All of keyboard UI automated tests are blocked by this bug.

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Kartikaya Gupta (email:[e-mail]) 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.

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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.

Comment 13

[Jason Smith [:jsmith]]

Dale is going to manually verify a backout of that bug. If it works, then we'll issue a backout of that patch.

Comment 14

[Jason Smith [:jsmith]]

Vivien actually already verified that backing out bug 946502 fixes this bug. He's going to back this out in a sec.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 16

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Kartikaya Gupta (email:[e-mail]) 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

Comment 17

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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 :)

Comment 18

[Dale Harvey (:daleharvey)]

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)

Comment 19

[Tony Chung [:tchung]]

(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.

Comment 20

[Stephen Donner [:stephend] Not actively reading bugmail]

This is also blocking the master.fps performance tests, which submit to Datazilla: http://qa-selenium.mv.mozilla.com:8080/job/b2g.hamachi.mozilla-central.master.perf.fps/1409/console

Comment 21

[Kartikaya Gupta (email:[e-mail])]

(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.

Comment 22

[Matt Brubeck (:mbrubeck)]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #16)
> (In reply to Kartikaya Gupta (email:[e-mail]) 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.

Comment 23

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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.

Comment 24

[Dale Harvey (:daleharvey)]

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

Comment 25

[mac]

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/.

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(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.

Comment 27

[Jason Smith [:jsmith]]

(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.

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: partial backout of bug 946502 part 3

I believe this won't reintroduce bug 946502. Something like this change is needed to prevent other bugs, but we can fix that separately.

Comment 29

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/706a4e2b081c

Comment 30

[Jason Smith [:jsmith]]

http://hg.mozilla.org/mozilla-central/rev/706a4e2b081c

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 32

[Oleg Romashin (:romaxa)]

This seems to be affecting memory consumption with tiled layers on Gecko 29...
Can we backport this backout to 29 release?

Comment 33

[Oleg Romashin (:romaxa)]

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

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/17dccd1aea37