1037284 - [NFC] Screen shows wired and NFC function broken when click home button while initial shrinking UI

Status: VERIFIED FIXED

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

Description

[Alison Shiue]

Attachment: click_home_button_before_shrinking_UI.mp4

Gaia      1bd6e8957ccf310b2f75ba5695b058a2e284df3a
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/f0e91a6bfd1b
BuildID   20140710134855
Version   32.0a2

Two phones with NFC

STR:
1. Phone A open browser
2. Tap two phones together
3. At the moment before show shrinking UI, click home button 

Expected result:
Nothing strange happen

Actual result:
1. Screen shows wired
2. Could not share browser again

Comment 1

[Alison Shiue]

http://youtu.be/RN43-Qc_Cs4

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

[Blocking Requested - why for this release]:

Comment 3

[howie [:howie]]

qawanted to check if this happens in 2.0

Comment 4

[Alison Shiue]

This case happens in 2.0

Comment 5

[Jason Smith [:jsmith]]

(In reply to ashiue from comment #4)
> This case happens in 2.0

Clearing QA flags per comment 4.

Comment 6

[Jason Smith [:jsmith]]

Yoshi - Did you mean to nominate this to 2.0, since this bug reproduces there?

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

[Blocking Requested - why for this release]:

If this also happens on 2.0 we should nominate to 2.0+.

Comment 8

[Vincent Chang[:vchang][changyihsin]]

Hi Greg, can you help to check this bug?

Comment 9

[Greg Weng [:snowmantw][:gweng][:λ]]

NI myself to remind to solve. I remember that there is one similar resolved bug, but needs time to survey to see if they're the same.

Comment 10

[Greg Weng [:snowmantw][:gweng][:λ]]

It's very hard to reproduce (at least for me)...either we can find a stable STR, or it's an very edge case and shouldn't be a blocker. Could you reproduce it easily?

Comment 11

[howie [:howie]]

Hi Alison, what's the reproduce rate here? thanks.

Comment 12

[Alison Shiue]

Always reproduce if you can tap the home button at the right moment, but it's not easy to get the moment.
(The moment is after NFC detect and before screen shows shrinking UI.)

Comment 13

[Greg Weng [:snowmantw][:gweng][:λ]]

Yoshi and I tested, and we usually failed to reproduce it. We success about 4 times while failed about 15 times. The moment is really hard to get.

According to the symptom, it looks like the 'back to home' event got triggered, even we're going to trigger shrinking UI and block the home key event. However, when it actually triggers the shrinking UI, the home key get blocked as it should be. So I suspect the problem is the timing to suspend and resume the home key event, but I still don't know what's the correct one.

Comment 14

[Greg Weng [:snowmantw][:gweng][:λ]]

Possible root cause: when 'shrinking-start' send from NFC manager, the 'home' event get sent immediately after it, too. So before NFC manager set the state as true (shrunk), the 'home' event can be passed without been blocked, and AWM handle it to back to home.

Comment 15

[Greg Weng [:snowmantw][:gweng][:λ]]

Or the first 'home' event pass and then 'shrinking-start' get fired, so that when AWM is handling the first event to switch to Homescreen, the shrinking start to performing. But I don't know whether it's possible that when AWM is handling Homescreen, the shrinking can do it's job.

Comment 16

[Greg Weng [:snowmantw][:gweng][:λ]]

One possible solution is ShrinkingUI knows whether the previous 'home' event get handled, if it's caused by the 'home'->'shrinking-start' case. Another possible solution is to block 'home' earlier, if it's caused by the 'shrinking-start'->'home' case. However, the timing to start to block the 'home' event now is so early, that I suspect if we can move it to another earlier moment.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #15)
> Or the first 'home' event pass and then 'shrinking-start' get fired, so that
> when AWM is handling the first event to switch to Homescreen, the shrinking
> start to performing. But I don't know whether it's possible that when AWM is
> handling Homescreen, the shrinking can do it's job.

It seems the problem is the whole shrinking procedure is not synchronous.
Could we listen to homescreenopened event to discard the shrinking state?

Comment 18

[Greg Weng [:snowmantw][:gweng][:λ]]

OK, I'll try with that. Thanks.

Comment 19

[Greg Weng [:snowmantw][:gweng][:λ]]

While I'm trying to patch it, I've found I can reproduce it with my WIP patch. So I now may dig deeper to see what's going on.

Comment 20

[Greg Weng [:snowmantw][:gweng][:λ]]

It looks like the Shrinking UI works as it should be: when it receive shrinking event, perform the UI, and block home key. The problem is now the NFC events fired too frequently in the case, so there is a missing 'home' event can be passed to AWM to make Homescreen appears. Debugging logs shows like this:

 JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:257 in su_start: >>>> ++++ TILING Started ++++
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:131 in su_handleEvent: >>>> NO HOME because of : true true false
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:131 in su_handleEvent: >>>> NO HOME because of : true true false
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:131 in su_handleEvent: >>>> NO HOME because of : true true false
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:263 in su_start/afterTilt<: >>>> ---- Stopped TILTING ----
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:136 in su_handleEvent: >>>> HOME because of : false false false
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/app_window_manager.js:529 in awm_handleEvent: >>>> <<<< >>>> <<<< HOME RECEIVIED
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:255 in su_start: >>>> iiiiii IN the start function:  false
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:257 in su_start: >>>> ++++ TILING Started ++++
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:131 in su_handleEvent: >>>> NO HOME because of : true true false
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:131 in su_handleEvent: >>>> NO HOME because of : true true false
E/GeckoConsole( 2146): Content JS LOG at app://system.gaiamobile.org/js/shrinking_ui.js:131 in su_handleEvent: >>>> NO HOME because of : true true false

You can see between the next home key blocking, there is one suddend stop tilting request (from the 'shrinking-stop' event). I guess that in such case the stopping animation would still perform, but meanwhile the AWM is doing to back to Homescreen, too.

Comment 21

[Greg Weng [:snowmantw][:gweng][:λ]]

Update: while in Homescreen, the modified patch can make the tilting window appears. It looks like the 'current' app in shrinking UI also get confused.

Comment 22

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #21)
> Update: while in Homescreen, the modified patch can make the tilting window
> appears. It looks like the 'current' app in shrinking UI also get confused.

I have an idea for maintaining AppWindows.

So in the past I think it makes sense to maintain a list of windows for every window manager,
but since the instance list in shrinkingUI has no difference from AppWindowManager,
I wish ShrinkingUI is a sub module of AppWindowManager.

That means, AppWindowManager will instantiate the ShrinkingUI and pass itself.
var shrinkingUI = new ShrinkingUI(this);

So in shrinkingUI we could access this.appWindowManager.getApps() and this.appWindowManaget.getActiveApp() directly.

Comment 23

[Greg Weng [:snowmantw][:gweng][:λ]]

I've fixed it with to set the current app as null while the 'home' event isn't blocked. I'll first re-organize my patch to let it works, and then to see if I can do some change as you said.

Comment 24

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch

Hi Alive, I want to minimize the risk of landing this patch and preventing Shrinking overreacting bug caused by the hardware issue. So I send this patch to fix this issue for master and (later) v2.0, and I would fire another bug to handle the second issue, which may require me to refactor Shrinking UI slightly.

Comment 25

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8470605 [details] [review]
Patch

r+ but we had better to have some test excluding UI part but only state management part..

Comment 26

[Greg Weng [:snowmantw][:gweng][:λ]]

The 'follow-up' bug to prevent Shrinking UI overreacting: Bug 1051665.

Yes, I've considered the test issue. However I'm not sure how to test this without UI part... The state I extracted from AppManager on my device, is the current app recorded in Shrinking UI would be the previous app, while the UI shows the current app should be Homescreen. My patch add the test at the 'home' event section.

Comment 27

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch v2.0

Comment 28

[Greg Weng [:snowmantw][:gweng][:λ]]

Waiting CI result.

Comment 29

[Greg Weng [:snowmantw][:gweng][:λ]]

The two patches only failed at one intermittent test:

https://tbpl.mozilla.org/?rev=5f7bf59496a8ac18380c1b23e5fd2f0f784dcb2e&tree=Gaia-Try
https://tbpl.mozilla.org/?rev=3f8e625a1809a7af423db6dd9299cae70d531185&tree=Gaia-Try

So I would land the patch.

Comment 30

[Greg Weng [:snowmantw][:gweng][:λ]]

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

Comment 31

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch v2.0

Correct the v2.0 patch.

Comment 32

[Greg Weng [:snowmantw][:gweng][:λ]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/e09129b1c6f48cd641f5bae8b572fa4d653dc6b9

Comment 33

[Alison Shiue]

Verified on

[2.0]
Gaia      1144cdc3a544f81c9bf71598aba1cb67d6c95a29
Gecko     https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/6495a7bd61ed
BuildID   20140812000205
Version   32.0

[2.1]
Gaia      8f955d80d175e52283275d3197e4eae2574b389f
Gecko     https://hg.mozilla.org/mozilla-central/rev/391f42c733fc
BuildID   20140811160202
Version   34.0a1