[Keyboard] Keyboard quit later than the app

VERIFIED FIXED

Status

Firefox OS
Gaia::Keyboard
P4
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: John Shih, Assigned: ttaubert)

Tracking

({polish})

unspecified
polish

Firefox Tracking Flags

(blocking-basecamp:-)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
## 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
(Reporter)

Updated

6 years ago
blocking-basecamp: --- → ?
(Assignee)

Updated

6 years ago
OS: Windows 7 → All
Hardware: x86_64 → All
I saw this too a few times - good work getting STR here.
Component: General → Gaia
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.
blocking-basecamp: ? → +
Priority: -- → P2
(Assignee)

Comment 3

6 years ago
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.
(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.
Assignee: nobody → ttaubert

Updated

6 years ago
Component: Gaia → Gaia::System::Keyboard
(Assignee)

Comment 5

6 years ago
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

6 years ago
Marking for C2, given this meets the criteria of known P1/P2 blocking-basecamp+ bugs at the end of C1.
Target Milestone: --- → B2G C2 (20nov-10dec)
(Assignee)

Comment 7

6 years ago
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.
Target Milestone: B2G C2 (20nov-10dec) → ---
(Assignee)

Updated

6 years ago
Target Milestone: --- → B2G C2 (20nov-10dec)
(Assignee)

Comment 8

6 years ago
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.
(Reporter)

Updated

6 years ago
Status: NEW → ASSIGNED
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).
blocking-, would take a patch for it but not blocking
blocking-basecamp: + → -
Keywords: polish
Priority: P2 → P4
Target Milestone: B2G C2 (20nov-10dec) → ---
Created attachment 687652 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

Pointer to Github pull-request
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.
Attachment #687652 - Flags: review?(21)
(Assignee)

Comment 13

6 years ago
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.
Attachment #687652 - Flags: feedback+
(Assignee)

Comment 14

6 years ago
Created attachment 687692 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6780

Pointer to Github pull-request
(Assignee)

Comment 15

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

Sorry, clicked the wrong button :(
Attachment #687692 - Attachment is obsolete: true
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+.
Attachment #687652 - Flags: review?(21) → review+
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.
Attachment #687652 - Flags: approval-gaia-master+
(Reporter)

Comment 19

6 years ago
verified on Unagi
2012-12-03
gaia master : 5d150b2b10472478e8841730abd9ae9597595206
mozilla-beta : 1e56f66d7ee9
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.