812115 - [Keyboard] Keyboard quit later than the app

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect, P4)

Description

[John Shih]

## Environment :
Unagi phone, build 2012-11-15
Build info: 
* "gaia master" revision= ab2a7c7ae90a0a4764f51b5eabd1c2d5e6117c9c
* "mozilla-aurora" revision= 0d76df6f808d

Video:http://www.youtube.com/watch?v=tIo3GUNBcHY&feature=youtu.be

## Repro :
1. Launch the contact app
2. Press "+" to add a contact
3. Focus on arbitrary field to launch the keyboard
4. press the home key to quit the app

## Expected:
* Back to homescreen

## Actual:
* Back to homescreen with keyboard still shows up for a short time

Comment 1

[Jason Smith [:jsmith]]

I saw this too a few times - good work getting STR here.

Comment 2

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Keyboard will be hidden after keyboard app receives onfocuschange event with |type == 'blur'|. However, for OOP apps, we will introduce 40ms delay for hiding the keyboard.
http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/forms.js#109
https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/keyboard.js#L404

Maybe we can hide keyboard-frame first while we invoke |closeWindow| in window manager.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

We could let forms.js listen for 'appwillclose' and blur the currently focused element. That would instantly hide the keyboard. The only problem is that's a synthetic event and doesn't reach the content script (forms.js). Not sure how to work around that, yet.

Comment 4

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

(In reply to Tim Taubert [:ttaubert] from comment #3)
> We could let forms.js listen for 'appwillclose' and blur the currently
> focused element. That would instantly hide the keyboard. The only problem is
> that's a synthetic event and doesn't reach the content script (forms.js).
> Not sure how to work around that, yet.

Let's move some logic on the system app side if needed.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

So the problem here is I think the communication overhead between processes. I can't reproduce the problem all the time but maybe in 30% of all tries.

When pressing the home button, WindowManager.closeWindow() is called. Sometimes it takes up to 200+ms for the blur event to be handled in forms.js. 100-150ms later the async message arrives in Keyboard.jsm and Keyboard:FocusChange gets broadcasted. About 130ms after that the onfocuschange event is dispatched and the keyboard app starts a timeout with 20ms which is then fired 150-200ms(!) later.

That's why it takes up 600ms until the keyboard is hidden. We also fade it out which adds a couple of perceived time as well.

Comment 6

[Alex Keybl [:akeybl]]

Marking for C2, given this meets the criteria of known P1/P2 blocking-basecamp+ bugs at the end of C1.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

I suspect that GC is one reason for the huge lag. When putting an app into the background I see this:

GC(T+65.9) Total Time: 380.0ms, Compartments Collected: 33, Total Compartments: 33, MMU (20ms): 0%, MMU (50ms): 0%, SCC Sweep Total: 232.2ms, SCC Sweep Max Pause: 215.9ms, Max Pause: 309.3ms, Allocated: 5MB, +Chunks: 1, -Chunks: 0

This is followed by a couple of ~30ms GCs with "Reason: MEM_PRESSURE". When just opening the contacts app I see three GCs: 360ms, 330ms and 240ms. All because of MEM_PRESSURE.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Well, even without calling frame.setVisible(false) - which I think would activate GC, right? - the keyboard is still visible way too long after the app has disappeared. I don't see any CC/GC logs so maybe that's not it.

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Thanks for looking into this. One thing I can think off that contribute the delay is that we always do the keyboard hidden transition (regressed recently so it doesn't really look like a transition) regardless the reason of hiding the keyboard (because of real blur() or app close). That can be fixed in keyboard_manager.js along.

(In reply to Tim Taubert [:ttaubert] from comment #3)
> We could let forms.js listen for 'appwillclose' and blur the currently
> focused element. That would instantly hide the keyboard. The only problem is
> that's a synthetic event and doesn't reach the content script (forms.js).
> Not sure how to work around that, yet.

Please don't make chrome script listens to events generated in System app. We need to keep the two decoupled (although we already did that for a few cases).

Comment 10

[Dylan Oliver [:doliver]]

blocking-, would take a patch for it but not blocking

Comment 11

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

Pointer to Github pull-request

Comment 12

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 687652 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

1. Hide the keyboard when keyboard manager receives appwillclose event.
2. Remove redundant transition code in keyboard app.
3. I will add back the transitionend listener to keyboard manager so that the app resize would occur after the keyboard shows up.

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 687652 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

This looks really good to me when I tried it on the device. I'm not too familiar with the keyboard transition code, though.

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

Pointer to Github pull-request

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 687692 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

Sorry, clicked the wrong button :(

Comment 16

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

Comment on attachment 687652 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

It also works very well on my device! r+.

Comment 17

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

Comment on attachment 687652 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

Grr. This is not blocking anymore after has been a C2 bug and people has spent time on it.... Let's land what we have now.

Comment 18

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

https://github.com/mozilla-b2g/gaia/commit/e2814afbf12ef9500fe425a7746cb1cdd3cefc30

Comment 19

[John Shih]

verified on Unagi
2012-12-03
gaia master : 5d150b2b10472478e8841730abd9ae9597595206
mozilla-beta : 1e56f66d7ee9