Skip to content

Commit 02a41a2

Browse files
author
Tarik Eshaq
authored
Polls all commands and not just one in iOS (#5236)
* Polls all commands and not just one in iOS * Fix changelog
1 parent dca2ada commit 02a41a2

3 files changed

Lines changed: 24 additions & 43 deletions

File tree

CHANGES_UNRELEASED.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,7 @@ Use the template below to make assigning a version number during the release cut
1818
- Description of the change with a link to the pull request ([#0000](https://github.com/mozilla/application-services/pull/0000))
1919
2020
-->
21+
22+
## FxA Client
23+
### What's changed
24+
- The `processRawIncomingAccountEvent` function will now process all commands, not just one. This moves the responsibilty of ensuring each push gets a UI element to the caller.\

components/fxa-client/src/internal/device.rs

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -180,29 +180,6 @@ impl FirefoxAccount {
180180
self.fetch_and_parse_commands(last_command_index + 1, None, reason)
181181
}
182182

183-
/// Retrieve and parse a specific command designated by its index.
184-
///
185-
/// **💾 This method alters the persisted account state.**
186-
///
187-
/// Note that this should not be used if possible, as it does not correctly
188-
/// handle missed messages. It's currently used only on iOS due to platform
189-
/// restrictions (but we should still try and work out how to correctly
190-
/// handle missed messages within those restrictions)
191-
/// (What's wrong: if we get a push for tab-1 and a push for tab-3, and
192-
/// between them I've never explicitly polled, I'll miss tab-2, even if I
193-
/// try polling now)
194-
pub fn ios_fetch_device_command(&mut self, index: u64) -> Result<IncomingDeviceCommand> {
195-
let mut device_commands =
196-
self.fetch_and_parse_commands(index, Some(1), CommandFetchReason::Push(index))?;
197-
let device_command = device_commands
198-
.pop()
199-
.ok_or(ErrorKind::IllegalState("Index fetch came out empty."))?;
200-
if !device_commands.is_empty() {
201-
log::warn!("Index fetch resulted in more than 1 element");
202-
}
203-
Ok(device_command)
204-
}
205-
206183
fn fetch_and_parse_commands(
207184
&mut self,
208185
index: u64,

components/fxa-client/src/internal/push.rs

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,19 @@ impl FirefoxAccount {
1313
/// Handle any incoming push message payload coming from the Firefox Accounts
1414
/// servers that has been decrypted and authenticated by the Push crate.
1515
///
16-
/// Due to iOS platform restrictions, a push notification must always show UI.
17-
/// Since FxA sends one push notification per command received,
18-
/// we must only retrieve 1 command per push message,
19-
/// otherwise we risk receiving push messages for which the UI has already been shown.
20-
/// However, note that this means iOS currently risks losing messages for
21-
/// which a push notification doesn't arrive.
16+
/// ** ⚠️ Due to iOS platform restrictions, a push notification must always show UI. **
17+
/// iOS callers of this API need to ensure that if this returns an empty list
18+
/// they display some type error notification.
19+
///
20+
/// This API could return an empty list if:
21+
/// 1. The user sends themselves multiple tabs, tab A and tab B
22+
/// 2. The device receives the notification for tab A, and queries FxA
23+
/// 3. The device gets **both** tab A and tab B from FxA
24+
/// 4. The device handles both tabs
25+
/// 5. The device receives the notification for tab B (late)
26+
/// 6. The device queries FxA again, and the tab is already gone!
27+
///
28+
/// We leave handling of the above case to the caller
2229
///
2330
/// **💾 This method alters the persisted account state.**
2431
pub fn handle_push_message(&mut self, payload: &str) -> Result<Vec<AccountEvent>> {
@@ -35,21 +42,14 @@ impl FirefoxAccount {
3542
})?;
3643
match payload {
3744
PushPayload::CommandReceived(CommandReceivedPushPayload { index, .. }) => {
38-
if cfg!(target_os = "ios") {
39-
let cmd = self.ios_fetch_device_command(index)?;
40-
Ok(vec![AccountEvent::CommandReceived {
41-
command: cmd.try_into()?,
42-
}])
43-
} else {
44-
let cmds = self.poll_device_commands(CommandFetchReason::Push(index))?;
45-
cmds.into_iter()
46-
.map(|command| {
47-
Ok(AccountEvent::CommandReceived {
48-
command: command.try_into()?,
49-
})
45+
let cmds = self.poll_device_commands(CommandFetchReason::Push(index))?;
46+
cmds.into_iter()
47+
.map(|command| {
48+
Ok(AccountEvent::CommandReceived {
49+
command: command.try_into()?,
5050
})
51-
.collect()
52-
}
51+
})
52+
.collect()
5353
}
5454
PushPayload::ProfileUpdated => {
5555
self.state.last_seen_profile = None;

0 commit comments

Comments
 (0)