Minor. When Nfc.js in Gonk gets a techLost, it should to send this information to the last app as well as clear the last app. This was partly fixed in v2.1/master. This patch fixes it in 2.0.

V2.0? This might affect shrinking UI if switching apps: use NFC app, switch NFC app with NFC session still active on the first one.

Comment on attachment 8469685 [details] [diff] [review] 0001-Fix-bad-app-id.patch Review of attachment 8469685 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/Nfc.js @@ +523,5 @@ > delete message.sessionId; > > gSystemMessenger.broadcastMessage("nfc-manager-tech-lost", message); > // Notify 'PeerLost' to appropriate registered target, if any > + gMessageManager.notifyPeerEvent(gMessageManager.currentPeerAppId, NFC.NFC_PEER_EVENT_LOST); This part is already handled in Bug 1034474. @@ +528,3 @@ > delete this.sessionTokenMap[this._currentSessionId]; > this._currentSessionId = null; > + gMessageManager.currentPeerAppId = null; This part not, please upload a patch with this modification, and we will need to uplift Bug 1034474 and this bug.

Hm...the file Nfc.js also moved in between on master due to the directory change (Bug 933141). I'm not quite sure we want all that to change in v2.0 branch to keep the same commits. Suggestions?

^ Nevermind. Mergable, though there's a need to partial merge some other fixes.

"regular" patch, master: dom/system/gonk/Nfc.js

Master patch.

Update wrong patch for v2.0 branch. This patch depends on Bug 1034474 being pull in as well.

Any ideas on how to get testing for the internal state of the gecko parent, between 2 gaia apps?

Comment on attachment 8471182 [details] [diff] [review] Master-0001-Bug-1050577-NFC-Techlost-not-clearing-peer-appId-cor.patch Review of attachment 8471182 [details] [diff] [review]: ----------------------------------------------------------------- Thanks. Add r=me, and re-format the subject. (remove the [PATCH] prefix)

Comment on attachment 8471184 [details] [diff] [review] v2.0 - 0001-Bug-1050577-NFC-Techlost-not-clearing-peer-appId-cor.patch Review of attachment 8471184 [details] [diff] [review]: ----------------------------------------------------------------- r+ on master is enough. But please update the subject as well.

Thanks Yoshi. Updated master version, and removed 2.0 version. Trying: https://tbpl.mozilla.org/?tree=Try&rev=04fe2675f27b

(In reply to Garner Lee from comment #11) > removed 2.0 version. We need to uplift it to the 2.0 branch so I move it back. What I meant before is we need two patches for master, and v2.0 branch, and r+ on master version is enough, don't need to ask r? for the v2.0 version.

Comment on attachment 8472062 [details] [diff] [review] [V2.0] Patch Please note that we need to uplift the patch from Bug 1034474 first. [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 933136. User impact if declined: onpeerlost could be notified to wrong app. Testing completed: Testing on device. Risk to taking this patch (and alternatives if risky): No. String or UUID changes made by this patch: No.

(In reply to Yoshi Huang[:allstars.chh] from comment #15) > Comment on attachment 8472062 [details] [diff] [review] > [V2.0] Patch > > Please note that we need to uplift the patch from Bug 1034474 first. > > [Approval Request Comment] > Bug caused by (feature/regressing bug #): > Bug 933136. > > User impact if declined: > onpeerlost could be notified to wrong app. > > Testing completed: > Testing on device. > > Risk to taking this patch (and alternatives if risky): > No. > > String or UUID changes made by this patch: > No. Only 2.0+ release blocking bugs can land on 2.0 at this point. After reading through the bug I do not think we would hold up a release for this issue. If you don't agree, please feel free to nominate this with 2.0? along with details on why this should be a blocker

Comment on attachment 8472062 [details] [diff] [review] [V2.0] Patch It shouldn't affect the behavior of onpeerlost, so cancelling the uplift.