998111 - [NFC] Shrinking UI on Nfc Gaia app (that registered for 'peerfound') remains tilted sometimes when performing peer discovery and peer lost between two NFC devices

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Garner Lee]

Per bug comment: https://bugzilla.mozilla.org/show_bug.cgi?id=959059#c20

Split possible remaining ShrinkUI timing bug into separate bug for investigation.

What happens:
The P2P UI can glitch if two NFC devices are placed very near, and rapidly brought together and removed. Done fast enough, the visible animation end states be in some odd intermediate transition state.

If the patch in Bug 959059 lands, the UI still seems to have an animation cancel timing bug that is greatly reduced if the animation transition time is set to some very small human fast time like 0.001s.

Expected:
UI matches the physical reality: It dismisses itself.
And possibly, no redraw flicker, and maybe a real time update on the tilted screen snapshot (This may be unrelated, or out of scope for the UX spec).

Comment 1

[Ken Chang[:kenkai]]

I wonder if we could prevent this situation in Gecko/Deamon? It seems not a good way to let Gaia to handle this kind of bulk event changing?

Comment 2

[Ken Chang[:kenkai]]

Sid, I wonder if you can handle this on NFC manager. Or where do you prefer to filter bulk events?

Comment 3

[psiddh]

Attachment: 0001-Bug-998111-Fix-race-condition-while-starting-and-sto.patch

Requesting Greg to provide feedback.
I tested this patch against race conditions that used to occur earlier. This patch seems to be fixing all race conditions surrounding shrinkingUI

Comment 4

[psiddh]

Reason for race condition:-
I think, the 'shrinking state' (setState) in start() is done in the callback.
http://lxr.mozilla.org/gaia/source/apps/system/js/shrinking_ui.js#215

Before this callback gets fired, there is a chance that stop could be called, resulting in such issues.

Fix:-
Set the state to true when 'start()' itself is called.

It looks like we can close couple of similar bugs (Bug#996515, Bug#959059) with this fix, unless I am missing something here.

Greg, if you are ok with this patch I will create a pull req by tomorrow.
If not, we may also use an enum of states ('init', 'started' , 'stopped') and check against these states in the code instead of boolean flag. Just my two cents.

Comment 5

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

It's OK to me if you can submit a patch. We can discuss it on GitHub.

And thanks for the investigation and fixing.

Comment 6

[psiddh]

Attachment: Fix race condition while starting & stopping tilted UI

Comment 7

[psiddh]

All green - Travis

Comment 8

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

Sidd, can you discuss with Garner for Bug 959059 and this bug?
These two bugs have almost the same title, 
but trying to fix different problem.

Also to my opinion, it seems your patch is the right one to fix the problem reported in Bug 959059.

Comment 9

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

Comment on attachment 8411245 [details] [review]
Fix race condition while starting & stopping tilted UI

r+: Passed tests and seems OK to correct the race condition.

Comment 10

[psiddh]

Yoshi, Greg : I have updated the title to reflect the fix. If you both are ok with it, I will go ahead and do 'checkin-needed'. We can continue to discuss Bug 959059 in its entry.

Comment 11

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

I am okay with it.

https://github.com/mozilla-b2g/gaia/commit/24dde6be66779874d1445c266770f0beee190943