1176184 - [Built-in&System Keyboard]After user locks and unlocks the device at "Select keyboards"view, the downloaded keyboard under "Select keyboards" item at "Keyboard" view will disappear.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Input Mgmt, defect)

Description

[SandKing]

Attachment: Bug log: logcat_1526.txt

[1.Description]:
[Flame v3.0][Nexus 5 v3.0][Built-in&System Keyboard]When user locks and unlocks the device in "Settings"->"Keyboards"->"Select keyboards" view, the downloaded keyboard is not under "Select  keyboards" item at "Keyboard" view, and it disappears.
See video:"1526.3GP"
See log:"logcat_1526.txt"
Found time:15:26

[2.Testing Steps]: 
1. Launch "Settings".
2. Connect a network.
3. Choose "Keyboards" item.
4. Tap "Build-in Keyboard".
5. Download a new keyboard.
6. After download finished, enable the keyboard.
7. Press "<"icon to back to "Keyboards" view.
**You can see the new keyboard appears is in the keyboard list at "Selected Keyboards" view.
8. Choose "Select Keyboards" item.
9. Press "Power" key to lock device.
10. Unlock the device.
11 Press "<"icon to back to "Keyboards" view.

[3.Expected Result]: 
11. The downloaded keyboard should be displayed under "Select  keyboards" item at "Keyboard" view.

[4.Actual Result]: 
11.The downloaded keyboard is not displayed under "Select  keyboards" item at "Keyboard" view.

[5.Reproduction build]: 
Device: Flame 2.2 (Unaffected)
Build ID               20150618002507
Gaia Revision          3414b07dc489976bf510fd8042c0af3b1192c160
Gaia Date              2015-06-16 22:04:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a2db74491088
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150618.035510
Firmware Date          Thu Jun 18 03:55:22 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (Affected)
Build ID               20150618010201
Gaia Revision          b404c41c5471c31610e64defb74ec066b411e724
Gaia Date              2015-06-17 17:01:15
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/a3f280b6f8d5
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150618.043514
Firmware Date          Thu Jun 18 04:35:26 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus5 2.2 (Unaffected)
Build ID               20150618002507
Gaia Revision          3414b07dc489976bf510fd8042c0af3b1192c160
Gaia Date              2015-06-16 22:04:56
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a2db74491088
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150618.034835
Firmware Date          Thu Jun 18 03:48:52 EDT 2015
Bootloader             HHZ12f

Device: Nexus5 3.0 (Affected)
Build ID               20150618160206
Gaia Revision          55bac3c151bff4f0ce0e8715962c4676fefb0887
Gaia Date              2015-06-18 16:49:24
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/656ea3bfbb4e
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150618.192142
Firmware Date          Thu Jun 18 19:22:00 EDT 2015
Bootloader             HHZ12f

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

Comment 1

[SandKing]

Attachment: Bug video: 1526.3GP

Comment 2

[Hermes Cheng (inactive after July 27, 2015)]

is it a regression?

Comment 3

[John Thomas [:Johnt]]

This issue DOES occur on the Flame 2.5. 

Flame: Newly downloaded selected keyboard will dissappear after locking and unlocking device.

Environmental Variables:
Device: Flame 2.5 KK Full Flash (319mb)
Build ID: 20150629134017
Gaia: 27fe0f4261e3685187769411f2f74cff19287b19
Gecko: c26dbd63604d
Gonk: a4f6f31d1fe213ac935ca8ede7d05e47324101a4
Version: 42.0a1 (2.5)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

========================================================

This issue does NOT occur on the Flame 2.2, 2.1 and 2.0.

Result: User does not have an option to manually download keyboards via WiFi.

Environmental Variables:
Device: Flame 2.2 KK Full Flash (319mb)
Build ID: 20150629162503
Gaia: b39d4f5b4937592ded19ec65e113a74177ae1f86
Gecko: cefa70ef71e4
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Environmental Variables:
Device: Flame 2.1 KK Full Flash (319mb)
Build ID: 20150630001206
Gaia: 7080a7c28b0242f81d689d2339dfa1177e23f48f
Gecko: c201e76c63d1
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 34.0 (2.5)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Flame 2.5 KK Full Flash (319mb)
BuildID: 20150630001206
Gaia: 7080a7c28b0242f81d689d2339dfa1177e23f48f
Gecko: c201e76c63d1
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 34.0 (2.5) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 4

[John Thomas [:Johnt]]

Correction the last set of variables from comment 3 should read as "2.5", it should be the following with the same result.:

Environmental Variables:
Device: Flame 2.0 KK Full Flash (319mb)
Build ID: 20150630000202
Gaia: 5552bf529d3d6775a968942e9afa6c1d4037362c
Gecko: 88eedcc6fd9b
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 32.0 (2.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 5

[Annabelle]

b2g-inbound Regression Window:
Last Working Environmental Variables:
Build ID               20150506010043
Gaia Revision          5277356f4a4c5958a881618aef9e7e835cd1f28a
Gaia Date              2015-05-06 04:01:55
Gecko Revision         https://hg.mozilla.org/integration/b2g-inbound/rev/6f4f0bd2e3af
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150413.193139
Firmware Date          Mon Apr 13 19:31:50 EDT 2015
Bootloader             L1TC000118D0

First Broken Environmental Variables:
Build ID               20150506015042
Gaia Revision          b9f6aa017033a4d9c71ebd09788fc127fb11945e
Gaia Date              2015-05-06 07:21:51
Gecko Revision         https://hg.mozilla.org/integration/b2g-inbound/rev/25e13983cd82
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150413.193139
Firmware Date          Mon Apr 13 19:31:50 EDT 2015
Bootloader             L1TC000118D0

First Broken Gaia & Last Working Gecko - issue DOES repro:
Gaia Revision          b9f6aa017033a4d9c71ebd09788fc127fb11945e
Gecko Revision         https://hg.mozilla.org/integration/b2g-inbound/rev/6f4f0bd2e3af

First Broken Gecko & Last Working Gaia – issue DOES NOT repro:
Gecko Revision         https://hg.mozilla.org/integration/b2g-inbound/rev/25e13983cd82
Gaia Revision          5277356f4a4c5958a881618aef9e7e835cd1f28a

Gaia pushlog:
https://github.com/mozilla-b2g/gaia/compare/5277356f4a4c5958a881618aef9e7e835cd1f28a...b9f6aa017033a4d9c71ebd09788fc127fb11945e


Possibly caused by bug 1128396.
Note:The bug has existed, since the "Installed keyboards'&"More Keyboard"were added in Built-in keyboard.

Comment 6

[Hermes Cheng (inactive after July 27, 2015)]

Tim, could you take a look? It seems the patch of bug 1128396 results in this issue.

Comment 7

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

Thanks.

Comment 8

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

The cause of this bug is because InputAppList will modify DOMApplication#manifest in place but the object will be silently deleted when the screen is turned off (a new one will be created in place when query on the property again).

STR with Web Console:

1. Launch Settings app
2. Go to Keyboards panel
3. Type |var m = KeyboardHelper.inputAppList._inputApps[0].manifest;|
4. Do |m === KeyboardHelper.inputAppList._inputApps[0].manifest;| and verify it returns true
5. Press the power button to turn off the screen
6. Do |m === KeyboardHelper.inputAppList._inputApps[0].manifest;| again.

On step 6, it will return false.

Fabrice, I tried to read into Webapps.js but I couldn't find anything obvious that would cause this behavior. Maybe bug 903291 is related? 

Minded that the relevant Gaia only enabled only enabled with bug 1128396, so comment 0 can only help us find the range up to that point.

The alternative would be NOT to modify the manifest object in place, but that would change the internal interfaces between KeyboardHelper and Settings/System so I would like to avoid that.

Comment 9

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

Looks like manifestCache that makes DOMApplication shared the manifest (and the rules of clean the object and recreate it) was implemented all the way back in bug 834999 [1]. That said, getting rid of the manifest object on memory pressure was only added in bug 903291.

[1] https://hg.mozilla.org/mozilla-central/diff/aea13422f063/dom/apps/src/Webapps.js

Comment 10

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

The alternative Gaia fix would be always modify in place the manifest *again* whenever we return the object to other modules...

Comment 11

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

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #10)
> The alternative Gaia fix would be always modify in place the manifest
> *again* whenever we return the object to other modules...

However, the modification will vanish somewhere down the line if the caller decide to hold the objects... so this is not a bulletproof solution either.

Comment 12

[[:fabrice] Fabrice Desré]

DOMApplication.manifest is a readonly property (see https://mxr.mozilla.org/mozilla-central/source/dom/webidl/Apps.webidl#51). You should really not even try to modify these objects and expect them to stay the same.

Comment 13

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

(In reply to Fabrice Desré [:fabrice] from comment #12)
> DOMApplication.manifest is a readonly property (see
> https://mxr.mozilla.org/mozilla-central/source/dom/webidl/Apps.webidl#51).
> You should really not even try to modify these objects and expect them to
> stay the same.

We should have Object.freeze() everything if that's the case.... WebIDL doesn't do that for us automatically isn't?

Comment 14

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

Update: I have a patch now removing the in-place editing of DOMApplication#manifest. I am now working on completing the unit tests. It's not pretty but it should fix the problem.

The discussions on WebIDL binding and API design has been moved to dev-platform.

Comment 15

[GitHub Autolander Bot]

Attachment: [gaia] timdream:input-mgmt-inputapp > mozilla-b2g:master

Comment 16

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

Comment on attachment 8632741 [details] [review]
[gaia] timdream:input-mgmt-inputapp > mozilla-b2g:master

Setup for review since the tests are passed.

This patch introduces InputApp instances which associate to DOMApplication and a dict to dynamic input. InputAppList no longer return array of DOMApplications and instead returns InputApps. The surface is smaller than I think but this is still non-trivial.

It's sad that we have multiple interfaces that tried to represent the same thing ("Keyboard" and "Layout" being two another example in keyboard_context.js in Settings app), I wonder if it's worthy of taking some time reducing that.

Evelyn, could you review the Settings part? Alberto, could you review the System part? Thanks!

Comment 17

[Evelyn Hung]

Comment on attachment 8632741 [details] [review]
[gaia] timdream:input-mgmt-inputapp > mozilla-b2g:master

r+ on Settings part. Thanks.

Comment 18

[Alberto Pastor [:albertopq]]

Comment on attachment 8632741 [details] [review]
[gaia] timdream:input-mgmt-inputapp > mozilla-b2g:master

I'm not too familiar with this part of the code, but given that you are the only contributor to that file (so I can't forward the review), the code makes sense to me, and is well tested, I'm going to r+ this :) Thanks!

Comment 19

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

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

Comment 20

[SandKing]

This bug has been verified as pass on latest Nightly build of Flame master and Nexus5 master by the STR in Comment 0.

Actual results: The downloaded keyboard was displayed under "Select  keyboards" item at "Keyboard" view.
Reproduce rate: 0/5


Device: Nexus5 master(Pass):
Build ID               20150714160203
Gaia Revision          803d04e3829fd4fe9261211aa0ddca6b79d4e328
Gaia Date              2015-07-14 17:54:44
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/d5025c151d17
Gecko Version          42.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150714.194243
Firmware Date          Tue Jul 14 19:43:01 EDT 2015
Bootloader             HHZ12f

Device: Flame master(Pass):
Build ID               20150714160203
Gaia Revision          803d04e3829fd4fe9261211aa0ddca6b79d4e328
Gaia Date              2015-07-14 17:54:44
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/d5025c151d17
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150714.193145
Firmware Date          Tue Jul 14 19:31:57 EDT 2015
Bootloader             L1TC000118D0