1015643 - [Keyboard] A race-free and deterministic LayoutLoader and LayoutManager for keyboard start-up and current layout state

Status: RESOLVED FIXED

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

Description

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

The way keyboard currently rely on loadLayout() method to load the layout, itself is a non-deterministic function. It also does not take care of cancelling the previous switching layout request. We should put a LayoutLoader and a LayoutManager in layout_manager.js (renamed from layout.js) to take care of that.

I believe the patch will be rather similar with Part II and III of bug 1013155. I think for LayoutLoader, can simply be copy from InputMethodLoader rename a few variables.

LayoutManager, just like InputMethodManager, should take care of the current layout state (exposed as |currentLayout|). a |switchLayout()| method serves as single entry point to switch to a new layout. 

It would be a bit easier than InputMethod since there is no activate/deactivate nor glue etc. I however have not figured out what the start-up layout state should be. I don't know if there will be some side effect if |currentLayout| starts with null. More tests need to be done on the patch to figure out that.

George, please work on this bug only if you have wrapped build script features and we are free of unclaimed blocking bugs that you could help. Thanks!

Comment 1

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

I've changed my mind. George, please help Alive on bug 938045 first.

Comment 2

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:master PR#19811

Comment 3

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

Comment on attachment 8431401 [details] [review]
mozilla-b2g:master PR#19811

This patch is now ready for review.

- Because promise always run async even after the script loaded and resolves, for code launch is means a 50ms regression on Unagi. However, since I was finally told we should not measure performance against that phone, I would like to ignore that.
- Same regression (force async) happens on showing keyboard with layout already loaded, but it might not be observable at all. Creating a switchCurrentIMEngineSync() method could fix that but I don't want to do that in this bug.
- The event listeners and updateCurrentLayout() will be factor out to a StateManager in another bug. I didn't do that in this bug because that's too much change in one bug.

Comment 4

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

Comment on attachment 8431401 [details] [review]
mozilla-b2g:master PR#19811

r=me with some nits to be addressed.
Thanks.

Comment 5

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

Let's wait for 2.1. There is little benefit to land this on 2.0 unless we have blocking bugs depend on it.

Comment 6

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

review comments addressed (the ones I think we need to address)

https://travis-ci.org/mozilla-b2g/gaia/builds/27200429

Comment 7

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

master: https://github.com/mozilla-b2g/gaia/commit/8d0d5494010ad3a13709b4f0858f7bf2e6c61889