Krzysztof Mioduszewski[:tauzen] Assignee
	Description • 10 years ago

NfcManager in handleTechnologyDiscovered [1] performs 3 checks on first NDEF record to determine if it's a handover record. Each check, if successful, calls different NfcHandoverManager method which passes the control to NfcHandoverManager and finishes the processing of NDEF message in NfcManager. We will need to add one more check in Bug 1011387, and maybe more in the future.

I would propose to introduce one public method in NfcHandoverManager which would perform all the necessary checks and handle handover records appropriately if needed. This way we would have the whole logic responsible for doing handover in one place, easier to extend and maintain in the future. This is also similar to how Android does it.

[1] - https://github.com/mozilla-b2g/gaia/blob/6b64df6fc533e71b5be576c77e856dac6e92b1fc/apps/system/js/nfc_manager.js#L350

	Krzysztof Mioduszewski[:tauzen] Assignee
	Comment 2 • 10 years ago

Travis failed on unrelated marionette test: 'search navigate to coolpage.html context menu opens after a context menu'
Try results seem ok: https://tbpl.mozilla.org/?rev=9aa4fd11ebc70746a7b45ea89e131cdaf76adb5b&tree=Gaia-Try

	Krzysztof Mioduszewski[:tauzen] Assignee
	Comment 3 • 10 years ago

Comment on attachment 8451372 [details]
pull-request-1027568.txt

Since Bug 948823 landed yesterday I had to rebase and resolve conflicts. I'm pushing the updated version for testing. Once the tests will be finished I'll ask for the review once more.

	Krzysztof Mioduszewski[:tauzen] Assignee
	Comment 4 • 10 years ago

Comment on attachment 8451372 [details]
pull-request-1027568.txt

Gaia-try: https://tbpl.mozilla.org/?rev=b15bb73ef5938f0bd88bfa25fd565d27f89c53df&tree=Gaia-Try

Travis failed on marionette test for vertical home which are unrelated.

	Greg Weng [:snowmantw][:gweng][:λ]
	Comment 5 • 10 years ago

Comment on attachment 8451372 [details]
pull-request-1027568.txt

Code looks good and passed all tests, thanks.

	Krzysztof Mioduszewski[:tauzen] Assignee
	Comment 6 • 10 years ago

Bug 998175 has landed and rebase is required. Removing checkin-needed. As previously, once the tests will be ok I will add it back.

	Krzysztof Mioduszewski[:tauzen] Assignee
	Comment 7 • 10 years ago

latest try results: https://tbpl.mozilla.org/?rev=8c66891e8f282174ab639ada0a4dc5f7435e630f&tree=Gaia-Try

Travis green except for test_privileged_app_contacts_prompt.py ui test which is not related.

	Ryan VanderMeulen [:RyanVM]
	Comment 8 • 10 years ago

Master: https://github.com/mozilla-b2g/gaia/commit/c707a146a71388db08032807660f8963ba728004