900906 - [Keyboard] Wrap the pinyin IME engine with web worker to improve the user experience

Status: RESOLVED FIXED

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

Description

[Luke Chang [:lchang] (inactive)]

We want to prevent blocking user input when the IME engine is searching candidates.

Comment 1

[Luke Chang [:lchang] (inactive)]

After Bug 905530 is landed, we will meet a problem that indexedDB can't be accessed in web workers (see Bug 701634).

Since the current performance with "setTimeout" machanism looks good enough for user story,
I propose land user dictionary in v1.2 first and resolve this issue right after.

Comment 2

[Luke Chang [:lchang] (inactive)]

Attachment: Pull Request

We're unable to aware how many layouts in keyboard app are enabled from settings after the new keyboard manager is landed, therefore, we won't know when is the best timing to "uninit" an unused layout engine. In this case, I propose that put a "setTimeout" in "engine.deactivate()" to unload itself when it has been idle for a while (as "latin" engine did).

Comment 3

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

Comment on attachment 803534 [details]
Pull Request

Looks good.
Just leave some comments on github and please help set the review flag back when they are addressed.

Thanks, Luke.

Comment 4

[Luke Chang [:lchang] (inactive)]

Comment on attachment 803534 [details]
Pull Request

I've updated my patch.
Thanks for your comments, Rudy.

Comment 5

[Mike Lee [:mlee]]

Bruce,
Is the Systems Platform Team commiting to this for koi (1.2)? Please update the blocking-b2g flag to either "koi+" or "-" if you're not committing to 1.2 delivery.

Comment 6

[Dietrich Ayala (:dietrich)]

Definitely need to fix, but not blocking 1.2 on it as we're not shipping in China for this release.

Comment 7

[Dietrich Ayala (:dietrich)]

Ah and that requirement is up in the air, actually. Either way, it's "strongly desired" not "must have", so still not blocking+.

Comment 8

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

Comment on attachment 803534 [details]
Pull Request

r+, thanks for this improvement.

I personally would want this to get into v1.2 since this will improve the UI response when we switch the layout into Pinyin from other lang. layouts.

Comment 9

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

Luke,

Please help rebase your patch onto master to trigger travis and then you may merge this pull request yourself.

Thanks.

Comment 10

[Luke Chang [:lchang] (inactive)]

Rudy,

I've rebased my patch and will merge this pull request after travis passed.
Thanks for your review.

Comment 11

[Luke Chang [:lchang] (inactive)]

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

merged in gaia master:
https://github.com/mozilla-b2g/gaia/commit/4da866ea74c30cda886f6c306cfba9d27d193aa9

Comment 12

[Luke Chang [:lchang] (inactive)]

Ivan,

As Rudy mentioned, this patch could get noticeable improvements of UI response.
Do we need uplift this patch to v1.2?

Comment 13

[Luke Chang [:lchang] (inactive)]

Comment on attachment 803534 [details]
Pull Request

[Approval Request Comment]
This patch can improve the UI response when we switch the layout into Pinyin from other lang. layouts.

[Bug caused by]
v1.2 feature: pinyin input method.

[User impact] if declined:
We may need more than 2 seconds to switch layouts between pinyin and the others.

[Testing completed]:
Travis passed.

[Risk to taking this patch] (and alternatives if risky):
Risk is low. This patch works on pinyin engine only.

Comment 14

[Ivan Tsay (:ITsay)]

(In reply to Luke Chang [:lchang] from comment #13)
> Comment on attachment 803534 [details]
> Pull Request
> 
> [Approval Request Comment]
> This patch can improve the UI response when we switch the layout into Pinyin
> from other lang. layouts.
> 
> [Bug caused by]
> v1.2 feature: pinyin input method.
> 
> [User impact] if declined:
> We may need more than 2 seconds to switch layouts between pinyin and the
> others.
> 
> [Testing completed]:
> Travis passed.
> 
> [Risk to taking this patch] (and alternatives if risky):
> Risk is low. This patch works on pinyin engine only.

Plus it for the performance improvement on pinyin IME

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 4da866ea74c30cda886f6c306cfba9d27d193aa9 to:
v1.2: f15d846d6a510132272bc878dee47b6c330c00d1

Comment 16

[Preeti Raghunath(:Preeti)]

Comment on attachment 803534 [details]
Pull Request

Clearing approval nom flag. This bug has been uplifted to 1.2 via koi+

Comment 17

[Mike Lee [:mlee]]

Setting Target Milestone for all FxOS Perf 1.2 issues fixed for 10.25.