Website Commands Queue Up When FMD is Disabled, Are Executed When Re-Enabled

VERIFIED FIXED in Firefox OS v2.0

Status

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: kglazko, Assigned: ggp)

Tracking

unspecified
2.1 S1 (1aug)
x86
Mac OS X
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.1 verified)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
STR:

 -Enable FMD, access website.
 -Disable FMD in Settings menu.
 -Play Sound. Nothing should happen. Enter lost mode, nothing should happen.
 -Now swipe to enable. Feel free to leave a delay between this and the previous step.
 -Your sound will play, your screen will lock.

The same behavior happens when we turn off the device/enter exit airplane mode, but fundamentally when disabling FMD, shouldn't we be preventing these commands from queuing?
(Reporter)

Updated

4 years ago
QA Contact: kglazko

Updated

4 years ago
Keywords: productwanted
The server has no idea what state the remote device is in (Off, disabled, traveling through a tunnel, dead battery, etc.) It merely waits for the device to reconnect and then sends the commands it has pending. 

I'm happy to hear reasonable suggestions about how to handle these sorts of things, however varying policies for commands and unknown device states can become very complicated very quickly.

Comment 2

4 years ago
jrconlin, ggp and I discussed this on IRC. the way forward is for the client to send FMD flag state information to the server which should prevent this behavior.
blocking-b2g: --- → 2.0?
Keywords: productwanted
The proposed solutions are:

FMD Toggle message
1) when the client notes that user has disabled FMD, sends a 
{enabled:false} 
message to the Server. 
2) Server notes message, changes known state of client to disabled, displays disabled controls on UI
3) When client re-enables FMD, client begins normal traffic back to Server
4) Server converts state of client to enabled, re-enables UI controls

FMD Server Pending Command collapse:
Server will collapse all pending commands to one instance of each command (e.g. one ring, one trace, one lock, etc.) The "latest" version of each command will supersede the previous version. Newer commands will be sent to the client before older ones.
blocking-b2g: 2.0? → 2.0+
Assigning to JR as he is working on collapse cmds on the server side.
Assignee: nobody → jrconlin
Assigning to JR as he is working on collapse cmds on the server side.
Target Milestone: --- → 2.0 S5 (4july)
Commands were collapsed in 
https://github.com/mozilla-services/FindMyDevice/commit/8c17fca69ea726b9f47d1073b5ece94886dc84e9

and have landed on the server. Waiting for client to provide FMD enabled status.
(Assignee)

Comment 7

4 years ago
Created attachment 8448193 [details] [review]
gaia pull request

This patch will notify the server when FMD is disabled, as that's necessary for the server to stop queuing commands. It also introduces some useful logging of our network activity.
Attachment #8448193 - Flags: review?(21)
(Assignee)

Updated

4 years ago
Attachment #8448193 - Flags: review?(21) → review?(lissyx+mozillians)
(Assignee)

Comment 8

4 years ago
For reference, the FMD website is https://fmd.stage.mozaws.net/
(In reply to Guilherme Gonçalves [:ggp] from comment #8)
> For reference, the FMD website is https://fmd.stage.mozaws.net/

So this is completely broken: I created an account on the stage FMD site/OAuth server (https://accounts.stage.mozaws.net/oauth/), I can login on this account from my laptop, but when I try to connect to the very same FMD from my device, it completely fails and asks me to create a new account.
(In reply to Alexandre LISSY :gerard-majax from comment #9)
> (In reply to Guilherme Gonçalves [:ggp] from comment #8)
> > For reference, the FMD website is https://fmd.stage.mozaws.net/
> 
> So this is completely broken: I created an account on the stage FMD
> site/OAuth server (https://accounts.stage.mozaws.net/oauth/), I can login on
> this account from my laptop, but when I try to connect to the very same FMD
> from my device, it completely fails and asks me to create a new account.

But creating the account from the device works.
(Assignee)

Comment 11

4 years ago
That sounds like bug 1029242, which seems intermittent and we thought was gone :(

Sorry about this, we'll get it working today.
See Also: → bug 1029242
Assignee: jrconlin → lissyx+mozillians
Since stage servers does not seems to be working reliably since I've been assigned to review this, I don't see when we can make sure the feature works as expected.
(Reporter)

Comment 13

4 years ago
It seems to be working well enough to test this patch today. I am running manual MozTrap tests, please see Find My Device test run for results: https://moztrap.mozilla.org/runtests/run/4586/env/27835/

Updated

4 years ago
Target Milestone: 2.0 S5 (4july) → 2.0 S6 (18july)
Assignee: lissyx+mozillians → ggoncalves
(In reply to kglazko from comment #13)
> It seems to be working well enough to test this patch today. I am running
> manual MozTrap tests, please see Find My Device test run for results:
> https://moztrap.mozilla.org/runtests/run/4586/env/27835/

Yeah, but the state was really bad last week. Now it's working more reliably, I've already been able to review the other parts :)
Actually, just looking it from the log (and to my untrained eye), it looks like the device is receiving push notification when FMD is disabled.  When I disabled FMD, the logcat showed following when I sent Ring or Lock command. 
/GeckoDump( 1508): [findmydevice] findmydevice got push notification!
I/GeckoDump( 1508): [findmydevice] findmydevice got push notification!
I/GeckoDump( 1508): [findmydevice] got wake up request  <- I enabled here
I/GeckoDump( 1508): [findmydevice] enabled, trying to reach the server
I/GeckoDump( 1508): [findmydevice] command r, args [30]
I/GeckoDump( 1508): [findmydevice] command t, args [60]
I/GeckoDump( 1508): [findmydevice] updating location to (43.6473884, -79.3941872)

Unless there is an ack that device does not send out when a push notification is sent, and server resends them when FMD is enabled back.
(Assignee)

Comment 16

4 years ago
(In reply to No-Jun Park [:njpark] from comment #15)
> Actually, just looking it from the log (and to my untrained eye), it looks
> like the device is receiving push notification when FMD is disabled.

That's expected I think. Right now (without this patch), FMD won't contact the server if it's disabled. It may still receive a push notification, but it won't do anything with it if disabled.

With the patch in this bug, it should be possible for the server to not even send the push notification, since then it will know that the device will ignore it.
And I'm again facing issues with the server side: once FMD enabled after performing a factory reset, the website does not find my device.

logcat shows a device id: d7378a7d40d73eb780b90abdf55fc4f7
when I try to reach https://find.firefox.com/#devices/d7378a7d40d73eb780b90abdf55fc4f7 it complains about unknown device.
I believe that what's happening here is that the server is currently retaining the user's deviceid in the local session cookie. This is causing it to try and reconnect to the previously deleted device. (After a factory reset, no previous data exists on the phone, so a new deviceID is generated.) 

This problem should resolve itself once the WebUI browser session expires. We're working on a few additional potential fixes (including possibly booting the user on device erase.) These will require UX, but are not blocking FC for client work for 2.0.
It works today.
Your PR needs to be rebased, I cannot apply it.
Flags: needinfo?(ggoncalves)
(Assignee)

Comment 21

4 years ago
PR updated.
Flags: needinfo?(ggoncalves)

Updated

4 years ago
Target Milestone: 2.0 S6 (18july) → ---
(In reply to JR Conlin [:jrconlin,:jconlin] from comment #18)
> I believe that what's happening here is that the server is currently
> retaining the user's deviceid in the local session cookie. This is causing
> it to try and reconnect to the previously deleted device. (After a factory
> reset, no previous data exists on the phone, so a new deviceID is
> generated.) 
> 
> This problem should resolve itself once the WebUI browser session expires.
> We're working on a few additional potential fixes (including possibly
> booting the user on device erase.) These will require UX, but are not
> blocking FC for client work for 2.0.

I'm again hitting this issue ...
I cannot test the feature since I'm getting once again blocked because of this deviceid mess. Even destroying my session's cookie does not help.
Flags: needinfo?(jrconlin)
I'm a bit confused. The server has not been changed in 12 days <https://find.firefox.com/metrics/>, and you reported that things were working for you in #19. We are explicitly not updating the server code to remove any potential issues that might be introduced while we lock down the client code. 

It may be that the session cookie is not being properly deleted or that you're returning to the device URL rather than the root <https://find.firefox.com/>.
Flags: needinfo?(jrconlin)
(In reply to JR Conlin [:jrconlin,:jconlin] from comment #24)
> I'm a bit confused. The server has not been changed in 12 days
> <https://find.firefox.com/metrics/>, and you reported that things were
> working for you in #19. We are explicitly not updating the server code to
> remove any potential issues that might be introduced while we lock down the
> client code. 
> 
> It may be that the session cookie is not being properly deleted or that
> you're returning to the device URL rather than the root
> <https://find.firefox.com/>.

No, I did returned to the root URL. And yes, it was working in comment 19, but in between I did a  bunch of factory reset/re-enabling the feature, for other needs.
Okay, I see the enabled flag sent to false. Looks like we are still missing some server side part, because the server still sends us requests. Re-enabling FMD after this does not trigger the requests that we received while it was off, so the feature looks to be working as expected :)
Comment on attachment 8448193 [details] [review]
gaia pull request

I commented on Github, there is mostly a couple of changes on the way asserts are written in unit test.

Once the comments are addressed, it should be okay to land!

Thanks, and sorry for the delay in being able to properly test this :(
Attachment #8448193 - Flags: review?(lissyx+mozillians) → review+
(Assignee)

Comment 28

4 years ago
PR updated, I'll be watching the tests to mark this for checkin when they're green.
master: https://github.com/mozilla-b2g/gaia/commit/860030fbfbb6889745711cb73790c1c2711f0bca
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S1 (1aug)
v2.0: https://github.com/mozilla-b2g/gaia/commit/9b6d7357031f2412b18a2fb140d5c974842d4393
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
(Reporter)

Updated

4 years ago
Flags: in-moztrap+
Duplicate of this bug: 1044368
Created attachment 8464013 [details] [review]
Gaia pull request - with unit test failure addressed
Attachment #8448193 - Attachment is obsolete: true
Attachment #8464013 - Flags: review?(lissyx+mozillians)
Attachment #8464013 - Attachment description: Gaia pull request - with unit test faulire addressed → Gaia pull request - with unit test failure addressed
Comment on attachment 8464013 [details] [review]
Gaia pull request - with unit test failure addressed

Unit tests are okay locally on v2.0, thanks :)
Attachment #8464013 - Flags: review?(lissyx+mozillians) → review+
Keywords: checkin-needed
v2.0: https://github.com/mozilla-b2g/gaia/commit/a7f5bbf42d42debfd14f65a15a7727880a32f8b2
status-b2g-v2.0: affected → fixed
Keywords: checkin-needed
This issue has been successfully verified on Flame v2.1&v2.0.
See attachment: verified_v2.0.mp4
Reproducing rate: 0/5

STR:

1.Enable FMD on deivce.
2.Access the website "find.firefox.com" on PC,sign in Fx account.
**User can play sound via tap "Ring Device",then stop ring.
3.Disable FMD in Settings.
4.Tap "Ring Device" to play sound on PC.
**No sound on deivce.
5.Enable FMD.
**No sound on deivce.
6.Tap "Ring Device" to play sound.
**Sound playing.

Flame 2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141127000203
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141127.034818
FW-Date         Thu Nov 27 03:48:29 EST 2014
Bootloader      L1TC00011880

Flame 2.1 build:
Gaia-Rev        5372b675e018b6aac97d95ff5db8d4bd16addb9b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f34377ae402b
Build-ID        20141127001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141127.035005
FW-Date         Thu Nov 27 03:50:16 EST 2014
Bootloader      L1TC00011880
status-b2g-v2.0: fixed → verified
status-b2g-v2.1: fixed → verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.