Closed Bug 1583413 Opened 11 months ago Closed 10 months ago

SendTab device list will be empty is sync isn't configured

Categories

(Firefox :: Firefox Accounts, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 71
Tracking Status
firefox71 --- fixed

People

(Reporter: markh, Assigned: lina)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Currently we populate the fxa device list via sync, but that's not going to work if sync isn't enabled - we will need to get it directly from fxa.

I can take this one.

Assignee: nobody → lina
Status: NEW → ASSIGNED

Mark, while I'm here, should we drop support for old Send Tab clients (I guess just Fennec at this point?), or support both and combine the list we get from FxA with Sync?

Flags: needinfo?(markh)

(In reply to Lina Cambridge (she/her) [:lina] from comment #2)

Mark, while I'm here, should we drop support for old Send Tab clients (I guess just Fennec at this point?), or support both and combine the list we get from FxA with Sync?

Good question - in bug 1579445 I suggested to Ed that instead of doing a sync we just hit fxa, and he indicated that would cause issues. Ed, what was the reason we still went via sync in that bug?

Flags: needinfo?(markh) → needinfo?(eoger)

Calling getDeviceList() after every UIState.ON_UPDATE might hammer the server—but maybe we could do caching, like we do for profiles? IIUC, it would also mean we couldn't send tabs to Fennec, but maybe we're OK with dropping support at this point?

I'd like to see a FxAccountsDevice.js module that's sync aware but where the canonical source of truth is the FxA devices (and listens to sync events!). Bonus point would be to move the device registration logic in there also.
For caching I've proposed adding an eTag server-side but it never got traction: https://github.com/mozilla/fxa/issues/524, but I don't think it's entirely necessary anyway.

Flags: needinfo?(eoger)

This commit contains no functional changes, just moving code around to
make the next patch in this series easier to write.

Instead of using the list of FxA devices from the Sync clients engine,
we now fetch the list of Send Tab devices from FxA. This works like
this:

  • FxAccountsDevice caches the device list in memory, and limits
    requests to once every 10 minutes. (We don't serialize the requests,
    however, so FxA and Sync fetching the device list concurrently
    might still send multiple requests).
  • FxAccountsDevice fires a new observer notification when the cached
    list is invalidated, after a device is connected or disconnected.
    This notifies Send Tab and FxA when to refresh their lists. Using the
    device connected and disconnected notifications might mean that the
    consumers will fetch a stale cached list, depending on if the
    FxAccountsDevice or the consumer observer fires first.
  • The Send Tab UI refreshes FxA devices in the background. Matching
    FxA devices to Sync client records is best effort, and only impacts
    the fallback case if the target doesn't support FxA commands.

Depends on D47520

I haven't fixed the tests yet, but does this look sensible?

Attachment #9097072 - Attachment description: Bug 1583413 - Move device-related methods into `FxAccountsDevice.jsm`. r?eoger! → Bug 1583413 - Move device-related methods into `FxAccountsDevice.jsm`. r?markh!,eoger
Attachment #9097073 - Attachment description: Bug 1583413 - Fetch the Send Tab target list from FxA, not Sync. r?markh!,eoger! → Bug 1583413 - Fetch the Send Tab target list from FxA, not Sync. r?markh!,eoger
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8d090eb60c78
Move device-related methods into `FxAccountsDevice.jsm`. r=eoger
https://hg.mozilla.org/integration/autoland/rev/0e871ed50b6c
Fetch the Send Tab target list from FxA, not Sync. r=markh,eoger
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
Regressions: 1587227
You need to log in before you can comment on or make changes to this bug.