Page MenuHomePhabricator
Differential D9248

[native] Fix rescinds and badge-only notifs on iOS
ClosedPublic
Actions

Authored by ashoat on Sep 20 2023, 10:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Aug 29, 1:20 AM
Unknown Object (File)
Tue, Aug 27, 2:10 AM
Unknown Object (File)
Sun, Aug 25, 8:55 PM
Unknown Object (File)
Sun, Aug 25, 2:42 AM
Unknown Object (File)
Sat, Aug 24, 6:36 AM
Unknown Object (File)
Sat, Aug 17, 9:24 PM
Unknown Object (File)
Wed, Aug 14, 7:11 PM
Unknown Object (File)
Tue, Aug 13, 5:59 PM
View All 65 Files
Subscribers
tomek

Details

Reviewers
atul
Commits
rCOMM4a1baebbbe0f: [native] Fix rescinds and badge-only notifs on iOS
Summary

D9178 broke this by requiring [content.userInfo[@"id"] to be non-nil in NotificationService.mm.

This diff makes it possible for that value to be nil without crashing the NSE.

Test Plan
  1. I managed to reproduce the issue in my local environment
  2. I then attached the Xcode debugger to the NSE, following the instructions here
  3. After my diff, I confirmed that the issue no longer occurs
  4. I also confirmed in the debugger that the value of [content.userInfo[@"id"] was in fact nil

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat created this revision.Sep 20 2023, 10:29 AM
Herald added a subscriber: tomek. · View Herald TranscriptSep 20 2023, 10:29 AM
atul published this revision for review.Sep 20 2023, 10:36 AM
atul accepted this revision.

Was there maybe some corresponding keyserver change that needed to be deployed before this release or something?

This revision is now accepted and ready to land.Sep 20 2023, 10:37 AM
ashoat added a comment.Sep 20 2023, 10:38 AM

Nope, this doesn't relate to any keyserver change

This revision was landed with ongoing or failed builds.Sep 20 2023, 10:40 AM
Closed by commit rCOMM4a1baebbbe0f: [native] Fix rescinds and badge-only notifs on iOS (authored by ashoat). · Explain Why
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM4a1baebbbe0f: [native] Fix rescinds and badge-only notifs on iOS.
atul added inline comments.Sep 20 2023, 10:45 AM
native/ios/NotificationService/NotificationService.mm
76–78 ↗(On Diff #31320)

Nit: I think generally we construct std::strings via eg std::ostringstream instead of + operator to avoid reallocating, but I think the performance overhead here is minimal so it doesn't matter and it was what was already there so it's probably not worth changing.

Harbormaster completed remote builds in B22764: Diff 31320.Sep 20 2023, 10:46 AM

Revision Contents
Changeset List

PathSize
native/
ios/
NotificationService/
17 lines

Diff 31322

View Options

native/ios/NotificationService/NotificationService.mm

Loading...