From 12295bd8c512f002ab1b6ae4380df528aa26182b Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 18 Jun 2018 16:52:46 -0400 Subject: [PATCH 01/11] Don't cache CNContact. --- ...ShareToExistingContactViewController.swift | 29 ++++- .../ViewModels/ContactShareViewModel.swift | 24 ++-- .../SharingThreadPickerViewController.m | 71 +++++----- SignalMessaging/contacts/OWSContactsManager.h | 7 + SignalMessaging/contacts/OWSContactsManager.m | 17 ++- .../contacts/SystemContactsFetcher.swift | 122 +++++++++++++++--- SignalServiceKit/src/Contacts/Contact.h | 10 +- SignalServiceKit/src/Contacts/Contact.m | 39 +++--- 8 files changed, 225 insertions(+), 94 deletions(-) diff --git a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift index 0f5c1a3eb..94eee89c9 100644 --- a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift +++ b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift @@ -55,17 +55,34 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi navigationController.popViewController(animated: true) } - func contactsPicker(_: ContactsPicker, didSelectContact contact: Contact) { + func contactsPicker(_: ContactsPicker, didSelectContact newContact: Contact) { Logger.debug("\(self.logTag) in \(#function)") - guard let mergedContact: CNContact = self.contactShare.cnContact(mergedWithExistingContact: contact) else { - owsFail("\(logTag) in \(#function) mergedContact was unexpectedly nil") - return - } + let contactsManager = Environment.current().contactsManager + contactsManager?.cnContact(withId: self.contactShare.cnContactId, + success: { [weak self] oldCNContact in + contactsManager?.cnContact(withId: newContact.cnContactId, + success: { newCNContact in + guard let strongSelf = weakSelf else { + return + } + strongSelf.merge(oldCNContact: oldCNContact, newCNContact: newCNContact) + }, failure: { + // TODO: Alert? + }) + }, failure: { + // TODO: Alert? + }) + } + + func merge(oldCNContact: CNContact, newCNContact: CNContact) { + Logger.debug("\(self.logTag) in \(#function)") + + let mergedCNContact: CNContact = Contact.merge(cnContact: oldCNContact, newCNContact: newCNContact) // Not actually a "new" contact, but this brings up the edit form rather than the "Read" form // saving our users a tap in some cases when we already know they want to edit. - let contactViewController: CNContactViewController = CNContactViewController(forNewContact: mergedContact) + let contactViewController: CNContactViewController = CNContactViewController(forNewContact: mergedCNContact) // Default title is "New Contact". We could give a more descriptive title, but anything // seems redundant - the context is sufficiently clear. diff --git a/SignalMessaging/ViewModels/ContactShareViewModel.swift b/SignalMessaging/ViewModels/ContactShareViewModel.swift index 1be27c2d8..2efb305c3 100644 --- a/SignalMessaging/ViewModels/ContactShareViewModel.swift +++ b/SignalMessaging/ViewModels/ContactShareViewModel.swift @@ -148,16 +148,20 @@ public class ContactShareViewModel: NSObject { return dbRecord.isProfileAvatar } - @objc - public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? { - - guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord, imageData: self.avatarImageData) else { - owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil") - return nil - } - - return existingContact.buildCNContact(mergedWithNewContact: newCNContact) - } +// @objc +// public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? { +// public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? { +//// success successParam: @escaping (CNContact) -> Void, +//// failure failureParam: @escaping () -> Void) { +// +// guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord, imageData: self.avatarImageData) else { +// owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil") +// return nil +// } +// +// let mergedCNContact = Contact.merge(cnContact: <#T##CNContact#>, newCNContact: <#T##CNContact#>) +// return existingContact.buildCNContact(mergedWithNewContact: newCNContact) +// } @objc public func copy(withName name: OWSContactName) -> ContactShareViewModel { diff --git a/SignalMessaging/attachments/SharingThreadPickerViewController.m b/SignalMessaging/attachments/SharingThreadPickerViewController.m index 5b9c20b6c..19acabc23 100644 --- a/SignalMessaging/attachments/SharingThreadPickerViewController.m +++ b/SignalMessaging/attachments/SharingThreadPickerViewController.m @@ -149,37 +149,7 @@ typedef void (^SendMessageBlock)(SendCompletionBlock completion); self.thread = thread; if (self.attachment.isConvertibleToContactShare) { - NSData *data = self.attachment.data; - - Contact *_Nullable contact = [Contact contactWithVCardData:data]; - OWSContact *_Nullable contactShareRecord = [OWSContacts contactForSystemContact:contact.cnContact]; - if (!contactShareRecord) { - DDLogError(@"%@ Could not convert system contact.", self.logTag); - return; - } - - BOOL isProfileAvatar = NO; - NSData *_Nullable avatarImageData = contact.imageData; - for (NSString *recipientId in contact.textSecureIdentifiers) { - if (avatarImageData) { - break; - } - avatarImageData = [self.contactsManager profileImageDataForPhoneIdentifier:recipientId]; - if (avatarImageData) { - isProfileAvatar = YES; - } - } - contactShareRecord.isProfileAvatar = isProfileAvatar; - - ContactShareViewModel *contactShare = - [[ContactShareViewModel alloc] initWithContactShareRecord:contactShareRecord - avatarImageData:avatarImageData]; - - ContactShareApprovalViewController *approvalVC = - [[ContactShareApprovalViewController alloc] initWithContactShare:contactShare - contactsManager:self.contactsManager - delegate:self]; - [self.navigationController pushViewController:approvalVC animated:YES]; + [self showContactShareApproval]; return; } @@ -200,6 +170,45 @@ typedef void (^SendMessageBlock)(SendCompletionBlock completion); } } +- (void)showContactShareApproval +{ + OWSAssert(self.attachment); + OWSAssert(self.thread); + OWSAssert(self.attachment.isConvertibleToContactShare); + + NSData *data = self.attachment.data; + + CNContact *_Nullable cnContact = [Contact cnContactWithVCardData:data]; + Contact *_Nullable contact = [[Contact alloc] initWithSystemContact:cnContact]; + OWSContact *_Nullable contactShareRecord = [OWSContacts contactForSystemContact:cnContact]; + if (!contactShareRecord) { + DDLogError(@"%@ Could not convert system contact.", self.logTag); + return; + } + + BOOL isProfileAvatar = NO; + NSData *_Nullable avatarImageData = contact.imageData; + for (NSString *recipientId in contact.textSecureIdentifiers) { + if (avatarImageData) { + break; + } + avatarImageData = [self.contactsManager profileImageDataForPhoneIdentifier:recipientId]; + if (avatarImageData) { + isProfileAvatar = YES; + } + } + contactShareRecord.isProfileAvatar = isProfileAvatar; + + ContactShareViewModel *contactShare = + [[ContactShareViewModel alloc] initWithContactShareRecord:contactShareRecord avatarImageData:avatarImageData]; + + ContactShareApprovalViewController *approvalVC = + [[ContactShareApprovalViewController alloc] initWithContactShare:contactShare + contactsManager:self.contactsManager + delegate:self]; + [self.navigationController pushViewController:approvalVC animated:YES]; +} + // override - (void)dismissPressed:(id)sender { diff --git a/SignalMessaging/contacts/OWSContactsManager.h b/SignalMessaging/contacts/OWSContactsManager.h index 3a34898ba..70fea7791 100644 --- a/SignalMessaging/contacts/OWSContactsManager.h +++ b/SignalMessaging/contacts/OWSContactsManager.h @@ -9,6 +9,9 @@ NS_ASSUME_NONNULL_BEGIN extern NSString *const OWSContactsManagerSignalAccountsDidChangeNotification; +typedef void (^CNContactFetchSuccess)(CNContact *cnContact); +typedef void (^CNContactFetchFailure)(void); + @class ImageCache; @class SignalAccount; @class UIFont; @@ -59,6 +62,10 @@ extern NSString *const OWSContactsManagerSignalAccountsDidChangeNotification; // contacts haven't changed, and will clear out any stale cached SignalAccounts - (void)userRequestedSystemContactsRefreshWithCompletion:(void (^)(NSError *_Nullable error))completionHandler; +- (void)cnContactWithId:(NSString *)contactId + success:(CNContactFetchSuccess)success + failure:(CNContactFetchFailure)failure; + #pragma mark - Util - (BOOL)isSystemContact:(NSString *)recipientId; diff --git a/SignalMessaging/contacts/OWSContactsManager.m b/SignalMessaging/contacts/OWSContactsManager.m index 9f85800ed..86d5a81bf 100644 --- a/SignalMessaging/contacts/OWSContactsManager.m +++ b/SignalMessaging/contacts/OWSContactsManager.m @@ -133,6 +133,17 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification return self.systemContactsFetcher.supportsContactEditing; } +- (void)cnContactWithId:(NSString *)contactId + success:(CNContactFetchSuccess)success + failure:(CNContactFetchFailure)failure +{ + OWSAssert(contactId.length > 0); + OWSAssert(success); + OWSAssert(failure); + + return [self.systemContactsFetcher fetchCNContactWithContactId:contactId success:success failure:failure]; +} + #pragma mark - SystemContactsFetcherDelegate - (void)systemContactsFetcher:(SystemContactsFetcher *)systemsContactsFetcher @@ -255,13 +266,13 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification [self.dbReadConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { for (Contact *contact in contacts) { NSArray *signalRecipients = [contact signalRecipientsWithTransaction:transaction]; - contactIdToSignalRecipientsMap[contact.uniqueId] = signalRecipients; + contactIdToSignalRecipientsMap[contact.cnContactId] = signalRecipients; } }]; NSMutableSet *seenRecipientIds = [NSMutableSet new]; for (Contact *contact in contacts) { - NSArray *signalRecipients = contactIdToSignalRecipientsMap[contact.uniqueId]; + NSArray *signalRecipients = contactIdToSignalRecipientsMap[contact.cnContactId]; for (SignalRecipient *signalRecipient in [signalRecipients sortedArrayUsingSelector:@selector(compare:)]) { if ([seenRecipientIds containsObject:signalRecipient.recipientId]) { DDLogDebug(@"Ignoring duplicate contact: %@, %@", signalRecipient.recipientId, contact.fullName); @@ -584,7 +595,7 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification NSAttributedString *lastName = [[NSAttributedString alloc] initWithString:cachedLastName attributes:lastNameAttributes]; - CNContact *_Nullable cnContact = self.allContactsMap[recipientId].cnContact; + CNContact *_Nullable cnContact = self.allContactsMap[recipientId].cnContactForFormatting; if (!cnContact) { // If we don't have a CNContact for this recipient id, make one. // Presumably [CNContactFormatter nameOrderForContact:] tries diff --git a/SignalMessaging/contacts/SystemContactsFetcher.swift b/SignalMessaging/contacts/SystemContactsFetcher.swift index 12178f179..e82a5b55a 100644 --- a/SignalMessaging/contacts/SystemContactsFetcher.swift +++ b/SignalMessaging/contacts/SystemContactsFetcher.swift @@ -17,12 +17,12 @@ protocol ContactStoreAdaptee { var supportsContactEditing: Bool { get } func requestAccess(completionHandler: @escaping (Bool, Error?) -> Void) func fetchContacts() -> Result<[Contact], Error> + func fetchCNContact(contactId: String) -> CNContact? func startObservingChanges(changeHandler: @escaping () -> Void) } public -class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee { - let TAG = "[ContactsFrameworkContactStoreAdaptee]" +class ContactsFrameworkContactStoreAdaptee: NSObject, ContactStoreAdaptee { private let contactStore = CNContactStore() private var changeHandler: (() -> Void)? private var initializedObserver = false @@ -72,7 +72,7 @@ class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee { return } - Logger.info("\(self.TAG) sort order changed: \(String(describing: self.lastSortOrder)) -> \(String(describing: currentSortOrder))") + Logger.info("\(self.logTag) sort order changed: \(String(describing: self.lastSortOrder)) -> \(String(describing: currentSortOrder))") self.lastSortOrder = currentSortOrder self.runChangeHandler() } @@ -81,7 +81,7 @@ class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee { @objc func runChangeHandler() { guard let changeHandler = self.changeHandler else { - owsFail("\(TAG) trying to run change handler before it was registered") + owsFail("\(self.logTag) trying to run change handler before it was registered") return } changeHandler() @@ -100,13 +100,35 @@ class ContactsFrameworkContactStoreAdaptee: ContactStoreAdaptee { systemContacts.append(contact) } } catch let error as NSError { - owsFail("\(self.TAG) Failed to fetch contacts with error:\(error)") + owsFail("\(self.logTag) Failed to fetch contacts with error:\(error)") return .error(error) } let contacts = systemContacts.map { Contact(systemContact: $0) } return .success(contacts) } + + func fetchCNContact(contactId: String) -> CNContact? { + var result: CNContact? + do { + let contactFetchRequest = CNContactFetchRequest(keysToFetch: ContactsFrameworkContactStoreAdaptee.allowedContactKeys) + contactFetchRequest.sortOrder = .userDefault + contactFetchRequest.predicate = CNContact.predicateForContacts(withIdentifiers: [contactId]) + + try self.contactStore.enumerateContacts(with: contactFetchRequest) { (contact, _) -> Void in + guard result == nil else { + owsFail("\(self.logTag) More than one contact with contact id.") + return + } + result = contact + } + } catch let error as NSError { + owsFail("\(self.logTag) Failed to fetch contact with error:\(error)") + return nil + } + + return result + } } @objc @@ -125,8 +147,6 @@ public enum ContactStoreAuthorizationStatus: UInt { @objc public class SystemContactsFetcher: NSObject { - private let TAG = "[SystemContactsFetcher]" - private let serialQueue = DispatchQueue(label: "SystemContactsFetcherQueue") var lastContactUpdateHash: Int? @@ -207,20 +227,20 @@ public class SystemContactsFetcher: NSObject { switch authorizationStatus { case .notDetermined: if CurrentAppContext().isInBackground() { - Logger.error("\(self.TAG) do not request contacts permission when app is in background") + Logger.error("\(self.logTag) do not request contacts permission when app is in background") completion(nil) return } self.contactStoreAdapter.requestAccess { (granted, error) in if let error = error { - Logger.error("\(self.TAG) error fetching contacts: \(error)") + Logger.error("\(self.logTag) error fetching contacts: \(error)") completion(error) return } guard granted else { // This case should have been caught by the error guard a few lines up. - owsFail("\(self.TAG) declined contact access.") + owsFail("\(self.logTag) declined contact access.") completion(nil) return } @@ -232,7 +252,7 @@ public class SystemContactsFetcher: NSObject { case .authorized: self.updateContacts(completion: completion) case .denied, .restricted: - Logger.debug("\(TAG) contacts were \(self.authorizationStatus)") + Logger.debug("\(logTag) contacts were \(self.authorizationStatus)") self.delegate?.systemContactsFetcher(self, hasAuthorizationStatus: authorizationStatus) completion(nil) } @@ -292,7 +312,7 @@ public class SystemContactsFetcher: NSObject { guard let _ = self else { return } - Logger.error("background task time ran out contacts fetch completed.") + Logger.error("background task time ran out before contacts fetch completed.") }) // Ensure completion is invoked on main thread. @@ -310,7 +330,7 @@ public class SystemContactsFetcher: NSObject { serialQueue.async { - Logger.info("\(self.TAG) fetching contacts") + Logger.info("\(self.logTag) fetching contacts") var fetchedContacts: [Contact]? switch self.contactStoreAdapter.fetchContacts() { @@ -322,21 +342,21 @@ public class SystemContactsFetcher: NSObject { } guard let contacts = fetchedContacts else { - owsFail("\(self.TAG) contacts was unexpectedly not set.") + owsFail("\(self.logTag) contacts was unexpectedly not set.") completion(nil) } - Logger.info("\(self.TAG) fetched \(contacts.count) contacts.") + Logger.info("\(self.logTag) fetched \(contacts.count) contacts.") let contactsHash = HashableArray(contacts).hashValue DispatchQueue.main.async { var shouldNotifyDelegate = false if self.lastContactUpdateHash != contactsHash { - Logger.info("\(self.TAG) contact hash changed. new contactsHash: \(contactsHash)") + Logger.info("\(self.logTag) contact hash changed. new contactsHash: \(contactsHash)") shouldNotifyDelegate = true } else if isUserRequested { - Logger.info("\(self.TAG) ignoring debounce due to user request") + Logger.info("\(self.logTag) ignoring debounce due to user request") shouldNotifyDelegate = true } else { @@ -346,19 +366,19 @@ public class SystemContactsFetcher: NSObject { let expiresAtDate = Date(timeInterval: kDebounceInterval, since: lastDelegateNotificationDate) if Date() > expiresAtDate { - Logger.info("\(self.TAG) debounce interval expired at: \(expiresAtDate)") + Logger.info("\(self.logTag) debounce interval expired at: \(expiresAtDate)") shouldNotifyDelegate = true } else { - Logger.info("\(self.TAG) ignoring since debounce interval hasn't expired") + Logger.info("\(self.logTag) ignoring since debounce interval hasn't expired") } } else { - Logger.info("\(self.TAG) first contact fetch. contactsHash: \(contactsHash)") + Logger.info("\(self.logTag) first contact fetch. contactsHash: \(contactsHash)") shouldNotifyDelegate = true } } guard shouldNotifyDelegate else { - Logger.info("\(self.TAG) no reason to notify delegate.") + Logger.info("\(self.logTag) no reason to notify delegate.") completion(nil) @@ -373,6 +393,66 @@ public class SystemContactsFetcher: NSObject { } } } + + @objc + public func fetchCNContact(contactId: String, + success successParam: @escaping (CNContact) -> Void, + failure failureParam: @escaping () -> Void) { + SwiftAssertIsOnMainThread(#function) + + guard authorizationStatus == .authorized else { + Logger.error("\(logTag) contact fetch failed; no access.") + failureParam() + return + } + + var backgroundTask: OWSBackgroundTask? = OWSBackgroundTask(label: "\(#function)", completionBlock: { [weak self] status in + SwiftAssertIsOnMainThread(#function) + + guard status == .expired else { + return + } + + guard let _ = self else { + return + } + Logger.error("background task time ran out before contact fetch completed.") + }) + + // Ensure success is invoked on main thread. + let success: (CNContact) -> Void = { contact in + DispatchMainThreadSafe({ + successParam(contact) + + assert(backgroundTask != nil) + backgroundTask = nil + }) + } + + // Ensure success is invoked on main thread. + let failure: () -> Void = { + DispatchMainThreadSafe({ + failureParam() + + assert(backgroundTask != nil) + backgroundTask = nil + }) + } + + // Don't use the serial queue. + DispatchQueue.global().async { + + Logger.info("\(self.logTag) fetching contact") + + if let cnContact = self.contactStoreAdapter.fetchCNContact(contactId: contactId) { + Logger.info("\(self.logTag) contact found") + success(cnContact) + } else { + Logger.info("\(self.logTag) contact not found") + failure() + } + } + } } struct HashableArray: Hashable { diff --git a/SignalServiceKit/src/Contacts/Contact.h b/SignalServiceKit/src/Contacts/Contact.h index 6e064dd7f..250611f12 100644 --- a/SignalServiceKit/src/Contacts/Contact.h +++ b/SignalServiceKit/src/Contacts/Contact.h @@ -26,12 +26,11 @@ NS_ASSUME_NONNULL_BEGIN @property (readonly, nonatomic) NSArray *parsedPhoneNumbers; @property (readonly, nonatomic) NSArray *userTextPhoneNumbers; @property (readonly, nonatomic) NSArray *emails; -@property (readonly, nonatomic) NSString *uniqueId; @property (nonatomic, readonly) BOOL isSignalContact; +@property (nonatomic, readonly) NSString *cnContactId; #if TARGET_OS_IOS @property (nullable, readonly, nonatomic) UIImage *image; @property (nullable, readonly, nonatomic) NSData *imageData; -@property (nullable, nonatomic, readonly) CNContact *cnContact; #endif // TARGET_OS_IOS - (NSArray *)signalRecipientsWithTransaction:(YapDatabaseReadTransaction *)transaction; @@ -41,7 +40,7 @@ NS_ASSUME_NONNULL_BEGIN #if TARGET_OS_IOS - (instancetype)initWithSystemContact:(CNContact *)contact NS_AVAILABLE_IOS(9_0); -+ (nullable Contact *)contactWithVCardData:(NSData *)data; ++ (nullable CNContact *)cnContactWithVCardData:(NSData *)data; - (NSString *)nameForPhoneNumber:(NSString *)recipientId; @@ -51,7 +50,10 @@ NS_ASSUME_NONNULL_BEGIN + (NSString *)formattedFullNameWithCNContact:(CNContact *)cnContact NS_SWIFT_NAME(formattedFullName(cnContact:)); + (nullable NSString *)localizedStringForCNLabel:(nullable NSString *)cnLabel; -- (CNContact *)buildCNContactMergedWithNewContact:(CNContact *)newCNContact NS_SWIFT_NAME(buildCNContact(mergedWithNewContact:)); ++ (CNContact *)mergeCNContact:(CNContact *)oldCNContact + newCNContact:(CNContact *)newCNContact NS_SWIFT_NAME(merge(cnContact:newCNContact:)); + +- (CNContact *)cnContactForFormatting; @end diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index 0f6e45621..89d1cf8de 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -16,7 +16,7 @@ NS_ASSUME_NONNULL_BEGIN @interface Contact () -@property (readonly, nonatomic) NSMutableDictionary *phoneNumberNameMap; +@property (nonatomic, readonly) NSMutableDictionary *phoneNumberNameMap; @end @@ -37,11 +37,10 @@ NS_ASSUME_NONNULL_BEGIN return self; } - _cnContact = contact; + _cnContactId = contact.identifier; _firstName = contact.givenName.ows_stripped; _lastName = contact.familyName.ows_stripped; _fullName = [Contact formattedFullNameWithCNContact:contact]; - _uniqueId = contact.identifier; NSMutableArray *phoneNumbers = [NSMutableArray new]; NSMutableDictionary *phoneNumberNameMap = [NSMutableDictionary new]; @@ -114,17 +113,6 @@ NS_ASSUME_NONNULL_BEGIN return self; } -+ (nullable Contact *)contactWithVCardData:(NSData *)data -{ - CNContact *_Nullable cnContact = [self cnContactWithVCardData:data]; - - if (!cnContact) { - return nil; - } - - return [[self alloc] initWithSystemContact:cnContact]; -} - - (nullable UIImage *)image { if (_image) { @@ -326,9 +314,14 @@ NS_ASSUME_NONNULL_BEGIN return contacts.firstObject; } -- (CNContact *)buildCNContactMergedWithNewContact:(CNContact *)newCNContact ++ (CNContact *)mergeCNContact:(CNContact *)oldCNContact newCNContact:(CNContact *)newCNContact { - CNMutableContact *_Nullable mergedCNContact = [self.cnContact mutableCopy]; + OWSAssert(oldCNContact); + OWSAssert(newCNContact); + + Contact *oldContact = [[Contact alloc] initWithSystemContact:oldCNContact]; + + CNMutableContact *_Nullable mergedCNContact = [oldCNContact mutableCopy]; if (!mergedCNContact) { OWSFail(@"%@ in %s mergedCNContact was unexpectedly nil", self.logTag, __PRETTY_FUNCTION__); return [CNContact new]; @@ -351,8 +344,8 @@ NS_ASSUME_NONNULL_BEGIN } // Phone Numbers - NSSet *existingParsedPhoneNumberSet = [NSSet setWithArray:self.parsedPhoneNumbers]; - NSSet *existingUnparsedPhoneNumberSet = [NSSet setWithArray:self.userTextPhoneNumbers]; + NSSet *existingParsedPhoneNumberSet = [NSSet setWithArray:oldContact.parsedPhoneNumbers]; + NSSet *existingUnparsedPhoneNumberSet = [NSSet setWithArray:oldContact.userTextPhoneNumbers]; NSMutableArray *> *mergedPhoneNumbers = [mergedCNContact.phoneNumbers mutableCopy]; for (CNLabeledValue *labeledPhoneNumber in newCNContact.phoneNumbers) { @@ -371,7 +364,7 @@ NS_ASSUME_NONNULL_BEGIN mergedCNContact.phoneNumbers = mergedPhoneNumbers; // Emails - NSSet *existingEmailSet = [NSSet setWithArray:self.emails]; + NSSet *existingEmailSet = [NSSet setWithArray:oldContact.emails]; NSMutableArray *> *mergedEmailAddresses = [mergedCNContact.emailAddresses mutableCopy]; for (CNLabeledValue *labeledEmail in newCNContact.emailAddresses) { NSString *normalizedValue = labeledEmail.value.ows_stripped; @@ -416,6 +409,14 @@ NS_ASSUME_NONNULL_BEGIN return localizedLabel; } +- (CNContact *)cnContactForFormatting +{ + CNMutableContact *cnContact = [CNMutableContact new]; + cnContact.givenName = self.firstName; + cnContact.familyName = self.lastName; + return cnContact; +} + @end NS_ASSUME_NONNULL_END From 83f11ad79b1473be12d90d68b956349e2205116a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 18 Jun 2018 17:20:27 -0400 Subject: [PATCH 02/11] Don't cache CNContact. --- ...ShareToExistingContactViewController.swift | 25 ++++----- .../src/ViewControllers/ContactsPicker.swift | 4 +- .../ConversationViewController.m | 11 ++-- .../OWSAddToContactViewController.m | 8 +-- Signal/src/views/ContactCell.swift | 3 +- .../ViewModels/ContactShareViewModel.swift | 16 ------ SignalMessaging/Views/ContactsViewHelper.m | 2 +- SignalMessaging/contacts/OWSContactsManager.h | 7 +-- SignalMessaging/contacts/OWSContactsManager.m | 27 +++++++--- .../contacts/SystemContactsFetcher.swift | 54 ++----------------- SignalServiceKit/src/Contacts/Contact.h | 3 +- SignalServiceKit/src/Contacts/Contact.m | 19 ++++--- 12 files changed, 65 insertions(+), 114 deletions(-) diff --git a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift index 94eee89c9..3ba404a14 100644 --- a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift +++ b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift @@ -55,24 +55,21 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi navigationController.popViewController(animated: true) } - func contactsPicker(_: ContactsPicker, didSelectContact newContact: Contact) { + func contactsPicker(_: ContactsPicker, didSelectContact oldContact: Contact) { Logger.debug("\(self.logTag) in \(#function)") let contactsManager = Environment.current().contactsManager - contactsManager?.cnContact(withId: self.contactShare.cnContactId, - success: { [weak self] oldCNContact in - contactsManager?.cnContact(withId: newContact.cnContactId, - success: { newCNContact in - guard let strongSelf = weakSelf else { - return - } - strongSelf.merge(oldCNContact: oldCNContact, newCNContact: newCNContact) - }, failure: { - // TODO: Alert? - }) - }, failure: { + guard let oldCNContact = contactsManager?.cnContact(withId: oldContact.cnContactId) else { // TODO: Alert? - }) + Logger.warn("\(logTag) could not load old CNContact.") + return + } + guard let newCNContact = OWSContacts.systemContact(for: self.contactShare.dbRecord, imageData: self.contactShare.avatarImageData) else { + // TODO: Alert? + Logger.warn("\(logTag) could not load new CNContact.") + return + } + merge(oldCNContact: oldCNContact, newCNContact: newCNContact) } func merge(oldCNContact: CNContact, newCNContact: CNContact) { diff --git a/Signal/src/ViewControllers/ContactsPicker.swift b/Signal/src/ViewControllers/ContactsPicker.swift index fbd920cbb..c42f071a2 100644 --- a/Signal/src/ViewControllers/ContactsPicker.swift +++ b/Signal/src/ViewControllers/ContactsPicker.swift @@ -237,7 +237,7 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView let contact = Contact(systemContact: cnContact) cell.configure(contact: contact, subtitleType: subtitleCellType, showsWhenSelected: self.allowsMultipleSelection, contactsManager: self.contactsManager) - let isSelected = selectedContacts.contains(where: { $0.uniqueId == contact.uniqueId }) + let isSelected = selectedContacts.contains(where: { $0.cnContactId == contact.cnContactId }) cell.isSelected = isSelected // Make sure we preserve selection across tableView.reloadData which happens when toggling between @@ -256,7 +256,7 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView let deselectedContact = cell.contact! selectedContacts = selectedContacts.filter { - return $0.uniqueId != deselectedContact.uniqueId + return $0.cnContactId != deselectedContact.cnContactId } } diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index bab602eb6..7ff1f1805 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -5118,13 +5118,18 @@ interactionControllerForAnimationController:(id CNContact? { -// public func cnContact(mergedWithExistingContact existingContact: Contact) -> CNContact? { -//// success successParam: @escaping (CNContact) -> Void, -//// failure failureParam: @escaping () -> Void) { -// -// guard let newCNContact = OWSContacts.systemContact(for: self.dbRecord, imageData: self.avatarImageData) else { -// owsFail("\(logTag) in \(#function) newCNContact was unexpectedly nil") -// return nil -// } -// -// let mergedCNContact = Contact.merge(cnContact: <#T##CNContact#>, newCNContact: <#T##CNContact#>) -// return existingContact.buildCNContact(mergedWithNewContact: newCNContact) -// } - @objc public func copy(withName name: OWSContactName) -> ContactShareViewModel { @@ -181,5 +166,4 @@ public class ContactShareViewModel: NSObject { // If we want to keep the avatar image, the caller will need to re-apply it. return ContactShareViewModel(contactShareRecord: newDbRecord, avatarImageData: nil) } - } diff --git a/SignalMessaging/Views/ContactsViewHelper.m b/SignalMessaging/Views/ContactsViewHelper.m index 60820ae6a..b6872af6b 100644 --- a/SignalMessaging/Views/ContactsViewHelper.m +++ b/SignalMessaging/Views/ContactsViewHelper.m @@ -355,7 +355,7 @@ NS_ASSUME_NONNULL_BEGIN cnContact = updatedContact; } if (signalAccount && !cnContact) { - cnContact = signalAccount.contact.cnContact; + cnContact = [self.contactsManager cnContactWithId:signalAccount.contact.cnContactId]; } if (cnContact) { if (shouldEditImmediately) { diff --git a/SignalMessaging/contacts/OWSContactsManager.h b/SignalMessaging/contacts/OWSContactsManager.h index 70fea7791..8ab416be8 100644 --- a/SignalMessaging/contacts/OWSContactsManager.h +++ b/SignalMessaging/contacts/OWSContactsManager.h @@ -9,9 +9,6 @@ NS_ASSUME_NONNULL_BEGIN extern NSString *const OWSContactsManagerSignalAccountsDidChangeNotification; -typedef void (^CNContactFetchSuccess)(CNContact *cnContact); -typedef void (^CNContactFetchFailure)(void); - @class ImageCache; @class SignalAccount; @class UIFont; @@ -62,9 +59,7 @@ typedef void (^CNContactFetchFailure)(void); // contacts haven't changed, and will clear out any stale cached SignalAccounts - (void)userRequestedSystemContactsRefreshWithCompletion:(void (^)(NSError *_Nullable error))completionHandler; -- (void)cnContactWithId:(NSString *)contactId - success:(CNContactFetchSuccess)success - failure:(CNContactFetchFailure)failure; +- (nullable CNContact *)cnContactWithId:(nullable NSString *)contactId; #pragma mark - Util diff --git a/SignalMessaging/contacts/OWSContactsManager.m b/SignalMessaging/contacts/OWSContactsManager.m index 86d5a81bf..8ab68c022 100644 --- a/SignalMessaging/contacts/OWSContactsManager.m +++ b/SignalMessaging/contacts/OWSContactsManager.m @@ -36,6 +36,7 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification @property (nonatomic, readonly) SystemContactsFetcher *systemContactsFetcher; @property (nonatomic, readonly) YapDatabaseConnection *dbReadConnection; @property (nonatomic, readonly) YapDatabaseConnection *dbWriteConnection; +@property (nonatomic, readonly) NSCache *cnContactCache; @end @@ -60,6 +61,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification _signalAccounts = @[]; _systemContactsFetcher = [SystemContactsFetcher new]; _systemContactsFetcher.delegate = self; + _cnContactCache = [NSCache new]; + _cnContactCache.countLimit = 50; OWSSingletonAssert(); @@ -133,15 +136,24 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification return self.systemContactsFetcher.supportsContactEditing; } -- (void)cnContactWithId:(NSString *)contactId - success:(CNContactFetchSuccess)success - failure:(CNContactFetchFailure)failure +- (nullable CNContact *)cnContactWithId:(nullable NSString *)contactId { OWSAssert(contactId.length > 0); - OWSAssert(success); - OWSAssert(failure); + OWSAssert(self.cnContactCache); - return [self.systemContactsFetcher fetchCNContactWithContactId:contactId success:success failure:failure]; + if (!contactId) { + return nil; + } + + CNContact *_Nullable cnContact = [self.cnContactCache objectForKey:contactId]; + if (!cnContact) { + cnContact = [self.systemContactsFetcher fetchCNContactWithContactId:contactId]; + if (cnContact) { + [self.cnContactCache setObject:cnContact forKey:contactId]; + } + } + + return cnContact; } #pragma mark - SystemContactsFetcherDelegate @@ -595,7 +607,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification NSAttributedString *lastName = [[NSAttributedString alloc] initWithString:cachedLastName attributes:lastNameAttributes]; - CNContact *_Nullable cnContact = self.allContactsMap[recipientId].cnContactForFormatting; + NSString *_Nullable cnContactId = self.allContactsMap[recipientId].cnContactId; + CNContact *_Nullable cnContact = [self cnContactWithId:cnContactId]; if (!cnContact) { // If we don't have a CNContact for this recipient id, make one. // Presumably [CNContactFormatter nameOrderForContact:] tries diff --git a/SignalMessaging/contacts/SystemContactsFetcher.swift b/SignalMessaging/contacts/SystemContactsFetcher.swift index e82a5b55a..d963772fe 100644 --- a/SignalMessaging/contacts/SystemContactsFetcher.swift +++ b/SignalMessaging/contacts/SystemContactsFetcher.swift @@ -395,63 +395,15 @@ public class SystemContactsFetcher: NSObject { } @objc - public func fetchCNContact(contactId: String, - success successParam: @escaping (CNContact) -> Void, - failure failureParam: @escaping () -> Void) { + public func fetchCNContact(contactId: String) -> CNContact? { SwiftAssertIsOnMainThread(#function) guard authorizationStatus == .authorized else { Logger.error("\(logTag) contact fetch failed; no access.") - failureParam() - return - } - - var backgroundTask: OWSBackgroundTask? = OWSBackgroundTask(label: "\(#function)", completionBlock: { [weak self] status in - SwiftAssertIsOnMainThread(#function) - - guard status == .expired else { - return - } - - guard let _ = self else { - return - } - Logger.error("background task time ran out before contact fetch completed.") - }) - - // Ensure success is invoked on main thread. - let success: (CNContact) -> Void = { contact in - DispatchMainThreadSafe({ - successParam(contact) - - assert(backgroundTask != nil) - backgroundTask = nil - }) - } - - // Ensure success is invoked on main thread. - let failure: () -> Void = { - DispatchMainThreadSafe({ - failureParam() - - assert(backgroundTask != nil) - backgroundTask = nil - }) + return nil } - // Don't use the serial queue. - DispatchQueue.global().async { - - Logger.info("\(self.logTag) fetching contact") - - if let cnContact = self.contactStoreAdapter.fetchCNContact(contactId: contactId) { - Logger.info("\(self.logTag) contact found") - success(cnContact) - } else { - Logger.info("\(self.logTag) contact not found") - failure() - } - } + return contactStoreAdapter.fetchCNContact(contactId: contactId) } } diff --git a/SignalServiceKit/src/Contacts/Contact.h b/SignalServiceKit/src/Contacts/Contact.h index 250611f12..d46624548 100644 --- a/SignalServiceKit/src/Contacts/Contact.h +++ b/SignalServiceKit/src/Contacts/Contact.h @@ -40,6 +40,7 @@ NS_ASSUME_NONNULL_BEGIN #if TARGET_OS_IOS - (instancetype)initWithSystemContact:(CNContact *)contact NS_AVAILABLE_IOS(9_0); ++ (nullable Contact *)contactWithVCardData:(NSData *)data; + (nullable CNContact *)cnContactWithVCardData:(NSData *)data; - (NSString *)nameForPhoneNumber:(NSString *)recipientId; @@ -53,8 +54,6 @@ NS_ASSUME_NONNULL_BEGIN + (CNContact *)mergeCNContact:(CNContact *)oldCNContact newCNContact:(CNContact *)newCNContact NS_SWIFT_NAME(merge(cnContact:newCNContact:)); -- (CNContact *)cnContactForFormatting; - @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index 89d1cf8de..be582d401 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -113,6 +113,17 @@ NS_ASSUME_NONNULL_BEGIN return self; } ++ (nullable Contact *)contactWithVCardData:(NSData *)data +{ + CNContact *_Nullable cnContact = [self cnContactWithVCardData:data]; + + if (!cnContact) { + return nil; + } + + return [[self alloc] initWithSystemContact:cnContact]; +} + - (nullable UIImage *)image { if (_image) { @@ -409,14 +420,6 @@ NS_ASSUME_NONNULL_BEGIN return localizedLabel; } -- (CNContact *)cnContactForFormatting -{ - CNMutableContact *cnContact = [CNMutableContact new]; - cnContact.givenName = self.firstName; - cnContact.familyName = self.lastName; - return cnContact; -} - @end NS_ASSUME_NONNULL_END From d3d9d2e64c492e0148172da78a3c06c6fa00ec87 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 18 Jun 2018 17:20:49 -0400 Subject: [PATCH 03/11] Don't cache CNContact. --- .../ThreadSettings/OWSAddToContactViewController.m | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Signal/src/ViewControllers/ThreadSettings/OWSAddToContactViewController.m b/Signal/src/ViewControllers/ThreadSettings/OWSAddToContactViewController.m index 44703f7de..d60423603 100644 --- a/Signal/src/ViewControllers/ThreadSettings/OWSAddToContactViewController.m +++ b/Signal/src/ViewControllers/ThreadSettings/OWSAddToContactViewController.m @@ -198,7 +198,8 @@ NS_ASSUME_NONNULL_BEGIN } CNContact *_Nullable cnContact = [self.contactsManager cnContactWithId:contact.cnContactId]; if (!cnContact) { - // TODO: + OWSFail(@"%@ Could not load system contact.", self.logTag); + return; } [self.contactsViewHelper presentContactViewControllerForRecipientId:self.recipientId fromViewController:self From b9e2963f47ac5362cda57f07eadd4f36357ce47c Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 19 Jun 2018 09:08:38 -0400 Subject: [PATCH 04/11] Don't cache CNContact. --- SignalMessaging/contacts/OWSContactsManager.m | 18 +++++----- SignalMessaging/utils/LRUCache.swift | 34 ++++++++++++++----- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/SignalMessaging/contacts/OWSContactsManager.m b/SignalMessaging/contacts/OWSContactsManager.m index 8ab68c022..13057afc4 100644 --- a/SignalMessaging/contacts/OWSContactsManager.m +++ b/SignalMessaging/contacts/OWSContactsManager.m @@ -36,7 +36,7 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification @property (nonatomic, readonly) SystemContactsFetcher *systemContactsFetcher; @property (nonatomic, readonly) YapDatabaseConnection *dbReadConnection; @property (nonatomic, readonly) YapDatabaseConnection *dbWriteConnection; -@property (nonatomic, readonly) NSCache *cnContactCache; +@property (nonatomic, readonly) AnyLRUCache *cnContactCache; @end @@ -61,8 +61,7 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification _signalAccounts = @[]; _systemContactsFetcher = [SystemContactsFetcher new]; _systemContactsFetcher.delegate = self; - _cnContactCache = [NSCache new]; - _cnContactCache.countLimit = 50; + _cnContactCache = [[AnyLRUCache alloc] initWithMaxSize:50]; OWSSingletonAssert(); @@ -145,11 +144,14 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification return nil; } - CNContact *_Nullable cnContact = [self.cnContactCache objectForKey:contactId]; - if (!cnContact) { - cnContact = [self.systemContactsFetcher fetchCNContactWithContactId:contactId]; - if (cnContact) { - [self.cnContactCache setObject:cnContact forKey:contactId]; + CNContact *_Nullable cnContact; + @synchronized(self.cnContactCache) { + cnContact = (CNContact * _Nullable)[self.cnContactCache getWithKey:contactId]; + if (!cnContact) { + cnContact = [self.systemContactsFetcher fetchCNContactWithContactId:contactId]; + if (cnContact) { + [self.cnContactCache setWithKey:contactId value:cnContact]; + } } } diff --git a/SignalMessaging/utils/LRUCache.swift b/SignalMessaging/utils/LRUCache.swift index 26d616c44..4321d6845 100644 --- a/SignalMessaging/utils/LRUCache.swift +++ b/SignalMessaging/utils/LRUCache.swift @@ -24,26 +24,46 @@ public class AnyLRUCache: NSObject { } // A simple LRU cache bounded by the number of entries. -// -// TODO: We might want to observe memory pressure notifications. public class LRUCache { private var cacheMap: [KeyType: ValueType] = [:] private var cacheOrder: [KeyType] = [] private let maxSize: Int + @objc public init(maxSize: Int) { self.maxSize = maxSize + + NotificationCenter.default.addObserver(self, + selector: #selector(didReceiveMemoryWarning), + name: NSNotification.Name.UIApplicationDidReceiveMemoryWarning, + object: nil) + } + + deinit { + NotificationCenter.default.removeObserver(self) + } + + @objc func didReceiveMemoryWarning() { + SwiftAssertIsOnMainThread(#function) + + cacheMap.removeAll() + cacheOrder.removeAll() + } + + private func updateCacheOrder(key: KeyType) { + cacheOrder = cacheOrder.filter { $0 != key } + cacheOrder.append(key) } public func get(key: KeyType) -> ValueType? { guard let value = cacheMap[key] else { + // Miss return nil } - // Update cache order. - cacheOrder = cacheOrder.filter { $0 != key } - cacheOrder.append(key) + // Hit + updateCacheOrder(key: key) return value } @@ -51,9 +71,7 @@ public class LRUCache { public func set(key: KeyType, value: ValueType) { cacheMap[key] = value - // Update cache order. - cacheOrder = cacheOrder.filter { $0 != key } - cacheOrder.append(key) + updateCacheOrder(key: key) while cacheOrder.count > maxSize { guard let staleKey = cacheOrder.first else { From 41a2ea03b019578ed539d2427d74c1fffb22380f Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 19 Jun 2018 09:14:32 -0400 Subject: [PATCH 05/11] Don't cache CNContact. --- Signal/src/ViewControllers/DebugUI/DebugUIMisc.m | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMisc.m b/Signal/src/ViewControllers/DebugUI/DebugUIMisc.m index 6b3232ed8..1b3229e41 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMisc.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMisc.m @@ -133,6 +133,11 @@ NS_ASSUME_NONNULL_BEGIN } }]]; + [items addObject:[OWSTableItem itemWithTitle:@"Fetch system contacts" + actionBlock:^() { + [Environment.current.contactsManager requestSystemContactsOnce]; + }]]; + return [OWSTableSection sectionWithTitle:self.name items:items]; } From af977ca409e251e94caefd6b405e0c6a65513ea4 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 19 Jun 2018 09:17:50 -0400 Subject: [PATCH 06/11] Don't cache CNContact. --- .../AddContactShareToExistingContactViewController.swift | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift index 3ba404a14..c272d9f0e 100644 --- a/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift +++ b/Signal/src/ViewControllers/AddContactShareToExistingContactViewController.swift @@ -60,13 +60,11 @@ class AddContactShareToExistingContactViewController: ContactsPicker, ContactsPi let contactsManager = Environment.current().contactsManager guard let oldCNContact = contactsManager?.cnContact(withId: oldContact.cnContactId) else { - // TODO: Alert? - Logger.warn("\(logTag) could not load old CNContact.") + owsFail("\(logTag) could not load old CNContact.") return } guard let newCNContact = OWSContacts.systemContact(for: self.contactShare.dbRecord, imageData: self.contactShare.avatarImageData) else { - // TODO: Alert? - Logger.warn("\(logTag) could not load new CNContact.") + owsFail("\(logTag) could not load new CNContact.") return } merge(oldCNContact: oldCNContact, newCNContact: newCNContact) From 08ca4fdb509f312dd208fe70de2d5be07d128350 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jun 2018 11:41:14 -0400 Subject: [PATCH 07/11] Lazy-load contact avatar data and images. Use NSCache for avatar images. --- .../ConversationViewController.m | 2 +- Signal/src/views/ContactCell.swift | 2 +- .../SharingThreadPickerViewController.m | 2 +- SignalMessaging/contacts/OWSContactsManager.h | 2 - SignalMessaging/contacts/OWSContactsManager.m | 40 +++++++++- SignalMessaging/utils/LRUCache.swift | 16 +++- SignalServiceKit/src/Contacts/Contact.h | 8 +- SignalServiceKit/src/Contacts/Contact.m | 75 ++++++++----------- .../src/Devices/OWSContactsOutputStream.h | 9 ++- .../src/Devices/OWSContactsOutputStream.m | 26 ++++--- .../DeviceSyncing/OWSSyncContactsMessage.m | 8 +- .../src/Protocols/ContactsManagerProtocol.h | 7 ++ 12 files changed, 125 insertions(+), 72 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 7ff1f1805..2926a492d 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -5134,7 +5134,7 @@ interactionControllerForAnimationController:(id 0); @@ -158,6 +162,38 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification return cnContact; } +- (nullable NSData *)avatarDataForCNContactId:(nullable NSString *)contactId +{ + // Don't bother to cache avatar data. + CNContact *_Nullable cnContact = [self cnContactWithId:contactId]; + return [Contact avatarDataForCNContact:cnContact]; +} + +- (nullable UIImage *)avatarImageForCNContactId:(nullable NSString *)contactId +{ + OWSAssert(self.cnContactAvatarCache); + + if (!contactId) { + return nil; + } + + UIImage *_Nullable avatarImage; + @synchronized(self.cnContactAvatarCache) { + avatarImage = (UIImage * _Nullable)[self.cnContactAvatarCache getWithKey:contactId]; + if (!avatarImage) { + NSData *_Nullable avatarData = [self avatarDataForCNContactId:contactId]; + if (avatarData) { + avatarImage = [UIImage imageWithData:avatarData]; + } + if (avatarImage) { + [self.cnContactAvatarCache setWithKey:contactId value:avatarImage]; + } + } + } + + return avatarImage; +} + #pragma mark - SystemContactsFetcherDelegate - (void)systemContactsFetcher:(SystemContactsFetcher *)systemsContactsFetcher @@ -257,6 +293,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification dispatch_async(dispatch_get_main_queue(), ^{ self.allContacts = contacts; self.allContactsMap = [allContactsMap copy]; + [self.cnContactCache clear]; + [self.cnContactAvatarCache clear]; [self.avatarCache removeAllImages]; @@ -791,7 +829,7 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification contact = [self signalAccountForRecipientId:identifier].contact; } - return contact.image; + return [self avatarImageForCNContactId:contact.cnContactId]; } - (nullable UIImage *)profileImageForPhoneIdentifier:(nullable NSString *)identifier diff --git a/SignalMessaging/utils/LRUCache.swift b/SignalMessaging/utils/LRUCache.swift index 4321d6845..9a252dcee 100644 --- a/SignalMessaging/utils/LRUCache.swift +++ b/SignalMessaging/utils/LRUCache.swift @@ -5,7 +5,7 @@ @objc public class AnyLRUCache: NSObject { - let backingCache: LRUCache + private let backingCache: LRUCache @objc public init(maxSize: Int) { @@ -21,6 +21,11 @@ public class AnyLRUCache: NSObject { public func set(key: NSObject, value: NSObject) { self.backingCache.set(key: key, value: value) } + + @objc + public func clear() { + self.backingCache.clear() + } } // A simple LRU cache bounded by the number of entries. @@ -47,8 +52,7 @@ public class LRUCache { @objc func didReceiveMemoryWarning() { SwiftAssertIsOnMainThread(#function) - cacheMap.removeAll() - cacheOrder.removeAll() + clear() } private func updateCacheOrder(key: KeyType) { @@ -82,4 +86,10 @@ public class LRUCache { cacheMap.removeValue(forKey: staleKey) } } + + @objc + public func clear() { + cacheMap.removeAll() + cacheOrder.removeAll() + } } diff --git a/SignalServiceKit/src/Contacts/Contact.h b/SignalServiceKit/src/Contacts/Contact.h index d46624548..1663fd8ee 100644 --- a/SignalServiceKit/src/Contacts/Contact.h +++ b/SignalServiceKit/src/Contacts/Contact.h @@ -28,10 +28,6 @@ NS_ASSUME_NONNULL_BEGIN @property (readonly, nonatomic) NSArray *emails; @property (nonatomic, readonly) BOOL isSignalContact; @property (nonatomic, readonly) NSString *cnContactId; -#if TARGET_OS_IOS -@property (nullable, readonly, nonatomic) UIImage *image; -@property (nullable, readonly, nonatomic) NSData *imageData; -#endif // TARGET_OS_IOS - (NSArray *)signalRecipientsWithTransaction:(YapDatabaseReadTransaction *)transaction; // TODO: Remove this method. @@ -39,7 +35,7 @@ NS_ASSUME_NONNULL_BEGIN #if TARGET_OS_IOS -- (instancetype)initWithSystemContact:(CNContact *)contact NS_AVAILABLE_IOS(9_0); +- (instancetype)initWithSystemContact:(CNContact *)cnContact NS_AVAILABLE_IOS(9_0); + (nullable Contact *)contactWithVCardData:(NSData *)data; + (nullable CNContact *)cnContactWithVCardData:(NSData *)data; @@ -54,6 +50,8 @@ NS_ASSUME_NONNULL_BEGIN + (CNContact *)mergeCNContact:(CNContact *)oldCNContact newCNContact:(CNContact *)newCNContact NS_SWIFT_NAME(merge(cnContact:newCNContact:)); ++ (nullable NSData *)avatarDataForCNContact:(nullable CNContact *)cnContact; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index be582d401..e079d2afa 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -9,6 +9,7 @@ #import "PhoneNumber.h" #import "SignalRecipient.h" #import "TSAccountManager.h" +#import "TextSecureKitEnv.h" @import Contacts; @@ -17,6 +18,7 @@ NS_ASSUME_NONNULL_BEGIN @interface Contact () @property (nonatomic, readonly) NSMutableDictionary *phoneNumberNameMap; +@property (nonatomic, readonly) NSUInteger imageHash; @end @@ -26,25 +28,24 @@ NS_ASSUME_NONNULL_BEGIN @synthesize comparableNameFirstLast = _comparableNameFirstLast; @synthesize comparableNameLastFirst = _comparableNameLastFirst; -@synthesize image = _image; #if TARGET_OS_IOS -- (instancetype)initWithSystemContact:(CNContact *)contact +- (instancetype)initWithSystemContact:(CNContact *)cnContact { self = [super init]; if (!self) { return self; } - _cnContactId = contact.identifier; - _firstName = contact.givenName.ows_stripped; - _lastName = contact.familyName.ows_stripped; - _fullName = [Contact formattedFullNameWithCNContact:contact]; + _cnContactId = cnContact.identifier; + _firstName = cnContact.givenName.ows_stripped; + _lastName = cnContact.familyName.ows_stripped; + _fullName = [Contact formattedFullNameWithCNContact:cnContact]; NSMutableArray *phoneNumbers = [NSMutableArray new]; NSMutableDictionary *phoneNumberNameMap = [NSMutableDictionary new]; - for (CNLabeledValue *phoneNumberField in contact.phoneNumbers) { + for (CNLabeledValue *phoneNumberField in cnContact.phoneNumbers) { if ([phoneNumberField.value isKindOfClass:[CNPhoneNumber class]]) { CNPhoneNumber *phoneNumber = (CNPhoneNumber *)phoneNumberField.value; [phoneNumbers addObject:phoneNumber.stringValue]; @@ -96,18 +97,21 @@ NS_ASSUME_NONNULL_BEGIN [self parsedPhoneNumbersFromUserTextPhoneNumbers:phoneNumbers phoneNumberNameMap:phoneNumberNameMap]; NSMutableArray *emailAddresses = [NSMutableArray new]; - for (CNLabeledValue *emailField in contact.emailAddresses) { + for (CNLabeledValue *emailField in cnContact.emailAddresses) { if ([emailField.value isKindOfClass:[NSString class]]) { [emailAddresses addObject:(NSString *)emailField.value]; } } _emails = [emailAddresses copy]; - if (contact.thumbnailImageData) { - _imageData = [contact.thumbnailImageData copy]; - } else if (contact.imageData) { - // This only occurs when sharing a contact via the share extension - _imageData = [contact.imageData copy]; + NSData *_Nullable avatarData = [Contact avatarDataForCNContact:cnContact]; + if (avatarData) { + NSUInteger hashValue = 0; + NSData *hashData = [Cryptography computeSHA256Digest:avatarData truncatedToBytes:sizeof(hashValue)]; + [hashData getBytes:&hashValue length:sizeof(hashValue)]; + _imageHash = hashValue; + } else { + _imageHash = 0; } return self; @@ -124,29 +128,6 @@ NS_ASSUME_NONNULL_BEGIN return [[self alloc] initWithSystemContact:cnContact]; } -- (nullable UIImage *)image -{ - if (_image) { - return _image; - } - - if (!self.imageData) { - return nil; - } - - _image = [UIImage imageWithData:self.imageData]; - return _image; -} - -+ (MTLPropertyStorage)storageBehaviorForPropertyWithKey:(NSString *)propertyKey -{ - if ([propertyKey isEqualToString:@"cnContact"] || [propertyKey isEqualToString:@"image"]) { - return MTLPropertyStorageTransitory; - } else { - return [super storageBehaviorForPropertyWithKey:propertyKey]; - } -} - #endif // TARGET_OS_IOS - (NSArray *)parsedPhoneNumbersFromUserTextPhoneNumbers:(NSArray *)userTextPhoneNumbers @@ -277,6 +258,20 @@ NS_ASSUME_NONNULL_BEGIN return value; } ++ (nullable NSData *)avatarDataForCNContact:(nullable CNContact *)cnContact +{ + if (cnContact.thumbnailImageData) { + return cnContact.thumbnailImageData.copy; + } else if (cnContact.imageData) { + // This only occurs when sharing a contact via the share extension + return cnContact.imageData.copy; + } else { + return nil; + } +} + +// This method is used to de-bounce system contact fetch notifications +// by checking for changes in the contact data. - (NSUInteger)hash { // base hash is some arbitrary number @@ -284,13 +279,7 @@ NS_ASSUME_NONNULL_BEGIN hash = hash ^ self.fullName.hash; - if (self.imageData) { - NSUInteger thumbnailHash = 0; - NSData *thumbnailHashData = - [Cryptography computeSHA256Digest:self.imageData truncatedToBytes:sizeof(thumbnailHash)]; - [thumbnailHashData getBytes:&thumbnailHash length:sizeof(thumbnailHash)]; - hash = hash ^ thumbnailHash; - } + hash = hash ^ self.imageHash; for (PhoneNumber *phoneNumber in self.parsedPhoneNumbers) { hash = hash ^ phoneNumber.toE164.hash; diff --git a/SignalServiceKit/src/Devices/OWSContactsOutputStream.h b/SignalServiceKit/src/Devices/OWSContactsOutputStream.h index 4f59766c7..1f9734102 100644 --- a/SignalServiceKit/src/Devices/OWSContactsOutputStream.h +++ b/SignalServiceKit/src/Devices/OWSContactsOutputStream.h @@ -1,19 +1,22 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "OWSChunkedOutputStream.h" NS_ASSUME_NONNULL_BEGIN -@class SignalAccount; @class OWSRecipientIdentity; +@class SignalAccount; + +@protocol ContactsManagerProtocol; @interface OWSContactsOutputStream : OWSChunkedOutputStream - (void)writeSignalAccount:(SignalAccount *)signalAccount recipientIdentity:(nullable OWSRecipientIdentity *)recipientIdentity - profileKeyData:(nullable NSData *)profileKeyData; + profileKeyData:(nullable NSData *)profileKeyData + contactsManager:(id)contactsManager; @end diff --git a/SignalServiceKit/src/Devices/OWSContactsOutputStream.m b/SignalServiceKit/src/Devices/OWSContactsOutputStream.m index 50470f7d2..6b66f838a 100644 --- a/SignalServiceKit/src/Devices/OWSContactsOutputStream.m +++ b/SignalServiceKit/src/Devices/OWSContactsOutputStream.m @@ -4,6 +4,7 @@ #import "OWSContactsOutputStream.h" #import "Contact.h" +#import "ContactsManagerProtocol.h" #import "Cryptography.h" #import "MIMETypeUtil.h" #import "NSData+keyVersionByte.h" @@ -22,9 +23,11 @@ NS_ASSUME_NONNULL_BEGIN - (void)writeSignalAccount:(SignalAccount *)signalAccount recipientIdentity:(nullable OWSRecipientIdentity *)recipientIdentity profileKeyData:(nullable NSData *)profileKeyData + contactsManager:(id)contactsManager { OWSAssert(signalAccount); OWSAssert(signalAccount.contact); + OWSAssert(contactsManager); OWSSignalServiceProtosContactDetailsBuilder *contactBuilder = [OWSSignalServiceProtosContactDetailsBuilder new]; [contactBuilder setName:signalAccount.contact.fullName]; @@ -38,15 +41,18 @@ NS_ASSUME_NONNULL_BEGIN contactBuilder.verifiedBuilder = verifiedBuilder; } - NSData *avatarPng; - if (signalAccount.contact.image) { - OWSSignalServiceProtosContactDetailsAvatarBuilder *avatarBuilder = - [OWSSignalServiceProtosContactDetailsAvatarBuilder new]; - - [avatarBuilder setContentType:OWSMimeTypeImagePng]; - avatarPng = UIImagePNGRepresentation(signalAccount.contact.image); - [avatarBuilder setLength:(uint32_t)avatarPng.length]; - [contactBuilder setAvatarBuilder:avatarBuilder]; + UIImage *_Nullable rawAvatar = [contactsManager avatarImageForCNContactId:signalAccount.contact.cnContactId]; + NSData *_Nullable avatarPng; + if (rawAvatar) { + avatarPng = UIImagePNGRepresentation(rawAvatar); + if (avatarPng) { + OWSSignalServiceProtosContactDetailsAvatarBuilder *avatarBuilder = + [OWSSignalServiceProtosContactDetailsAvatarBuilder new]; + + [avatarBuilder setContentType:OWSMimeTypeImagePng]; + [avatarBuilder setLength:(uint32_t)avatarPng.length]; + [contactBuilder setAvatarBuilder:avatarBuilder]; + } } if (profileKeyData) { @@ -79,7 +85,7 @@ NS_ASSUME_NONNULL_BEGIN [self.delegateStream writeRawVarint32:contactDataLength]; [self.delegateStream writeRawData:contactData]; - if (signalAccount.contact.image) { + if (avatarPng) { [self.delegateStream writeRawData:avatarPng]; } } diff --git a/SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncContactsMessage.m b/SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncContactsMessage.m index 05d07f93b..0612d2bc1 100644 --- a/SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncContactsMessage.m +++ b/SignalServiceKit/src/Messages/DeviceSyncing/OWSSyncContactsMessage.m @@ -13,6 +13,7 @@ #import "SignalAccount.h" #import "TSAttachment.h" #import "TSAttachmentStream.h" +#import "TextSecureKitEnv.h" NS_ASSUME_NONNULL_BEGIN @@ -71,6 +72,8 @@ NS_ASSUME_NONNULL_BEGIN - (NSData *)buildPlainTextAttachmentData { + id contactsManager = TextSecureKitEnv.sharedEnv.contactsManager; + // TODO use temp file stream to avoid loading everything into memory at once // First though, we need to re-engineer our attachment process to accept streams (encrypting with stream, // and uploading with streams). @@ -82,10 +85,11 @@ NS_ASSUME_NONNULL_BEGIN OWSRecipientIdentity *_Nullable recipientIdentity = [self.identityManager recipientIdentityForRecipientId:signalAccount.recipientId]; NSData *_Nullable profileKeyData = [self.profileManager profileKeyDataForRecipientId:signalAccount.recipientId]; - + [contactsOutputStream writeSignalAccount:signalAccount recipientIdentity:recipientIdentity - profileKeyData:profileKeyData]; + profileKeyData:profileKeyData + contactsManager:contactsManager]; } [contactsOutputStream flush]; diff --git a/SignalServiceKit/src/Protocols/ContactsManagerProtocol.h b/SignalServiceKit/src/Protocols/ContactsManagerProtocol.h index 7a6e5cc69..4954610d5 100644 --- a/SignalServiceKit/src/Protocols/ContactsManagerProtocol.h +++ b/SignalServiceKit/src/Protocols/ContactsManagerProtocol.h @@ -4,6 +4,7 @@ NS_ASSUME_NONNULL_BEGIN +@class CNContact; @class Contact; @class PhoneNumber; @class SignalAccount; @@ -20,6 +21,12 @@ NS_ASSUME_NONNULL_BEGIN - (NSComparisonResult)compareSignalAccount:(SignalAccount *)left withSignalAccount:(SignalAccount *)right NS_SWIFT_NAME(compare(signalAccount:with:)); +#pragma mark - CNContacts + +- (nullable CNContact *)cnContactWithId:(nullable NSString *)contactId; +- (nullable NSData *)avatarDataForCNContactId:(nullable NSString *)contactId; +- (nullable UIImage *)avatarImageForCNContactId:(nullable NSString *)contactId; + @end NS_ASSUME_NONNULL_END From 87ea1dcae12da1d1e13c0d1d185e7007e226f3f3 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jun 2018 13:14:21 -0400 Subject: [PATCH 08/11] Clean up ahead of PR. --- Signal/src/ViewControllers/ContactsPicker.swift | 4 ++-- SignalMessaging/contacts/OWSContactsManager.m | 4 ++-- SignalServiceKit/src/Contacts/Contact.h | 1 + SignalServiceKit/src/Contacts/Contact.m | 5 +++++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Signal/src/ViewControllers/ContactsPicker.swift b/Signal/src/ViewControllers/ContactsPicker.swift index c42f071a2..fbd920cbb 100644 --- a/Signal/src/ViewControllers/ContactsPicker.swift +++ b/Signal/src/ViewControllers/ContactsPicker.swift @@ -237,7 +237,7 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView let contact = Contact(systemContact: cnContact) cell.configure(contact: contact, subtitleType: subtitleCellType, showsWhenSelected: self.allowsMultipleSelection, contactsManager: self.contactsManager) - let isSelected = selectedContacts.contains(where: { $0.cnContactId == contact.cnContactId }) + let isSelected = selectedContacts.contains(where: { $0.uniqueId == contact.uniqueId }) cell.isSelected = isSelected // Make sure we preserve selection across tableView.reloadData which happens when toggling between @@ -256,7 +256,7 @@ public class ContactsPicker: OWSViewController, UITableViewDelegate, UITableView let deselectedContact = cell.contact! selectedContacts = selectedContacts.filter { - return $0.cnContactId != deselectedContact.cnContactId + return $0.uniqueId != deselectedContact.uniqueId } } diff --git a/SignalMessaging/contacts/OWSContactsManager.m b/SignalMessaging/contacts/OWSContactsManager.m index 95859de7a..ab1a734f6 100644 --- a/SignalMessaging/contacts/OWSContactsManager.m +++ b/SignalMessaging/contacts/OWSContactsManager.m @@ -318,13 +318,13 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification [self.dbReadConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { for (Contact *contact in contacts) { NSArray *signalRecipients = [contact signalRecipientsWithTransaction:transaction]; - contactIdToSignalRecipientsMap[contact.cnContactId] = signalRecipients; + contactIdToSignalRecipientsMap[contact.uniqueId] = signalRecipients; } }]; NSMutableSet *seenRecipientIds = [NSMutableSet new]; for (Contact *contact in contacts) { - NSArray *signalRecipients = contactIdToSignalRecipientsMap[contact.cnContactId]; + NSArray *signalRecipients = contactIdToSignalRecipientsMap[contact.uniqueId]; for (SignalRecipient *signalRecipient in [signalRecipients sortedArrayUsingSelector:@selector(compare:)]) { if ([seenRecipientIds containsObject:signalRecipient.recipientId]) { DDLogDebug(@"Ignoring duplicate contact: %@, %@", signalRecipient.recipientId, contact.fullName); diff --git a/SignalServiceKit/src/Contacts/Contact.h b/SignalServiceKit/src/Contacts/Contact.h index 1663fd8ee..ff7455f8d 100644 --- a/SignalServiceKit/src/Contacts/Contact.h +++ b/SignalServiceKit/src/Contacts/Contact.h @@ -26,6 +26,7 @@ NS_ASSUME_NONNULL_BEGIN @property (readonly, nonatomic) NSArray *parsedPhoneNumbers; @property (readonly, nonatomic) NSArray *userTextPhoneNumbers; @property (readonly, nonatomic) NSArray *emails; +@property (readonly, nonatomic) NSString *uniqueId; @property (nonatomic, readonly) BOOL isSignalContact; @property (nonatomic, readonly) NSString *cnContactId; diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index e079d2afa..2853ab2a7 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -117,6 +117,11 @@ NS_ASSUME_NONNULL_BEGIN return self; } +- (nullable NSString *)uniqueId +{ + return self.cnContactId; +} + + (nullable Contact *)contactWithVCardData:(NSData *)data { CNContact *_Nullable cnContact = [self cnContactWithVCardData:data]; From ebcc435c9f6583db0c9cd567a33d7dc696ab9db0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jun 2018 13:16:04 -0400 Subject: [PATCH 09/11] Clean up ahead of PR. --- SignalServiceKit/src/Contacts/Contact.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Contacts/Contact.m b/SignalServiceKit/src/Contacts/Contact.m index 2853ab2a7..c088eda73 100644 --- a/SignalServiceKit/src/Contacts/Contact.m +++ b/SignalServiceKit/src/Contacts/Contact.m @@ -117,7 +117,7 @@ NS_ASSUME_NONNULL_BEGIN return self; } -- (nullable NSString *)uniqueId +- (NSString *)uniqueId { return self.cnContactId; } From 63b6276c25ced849962f441957077a07ca08e76a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jun 2018 16:16:27 -0400 Subject: [PATCH 10/11] Clear LRUCache in background. --- Signal/src/ViewControllers/DebugUI/DebugUIContacts.m | 6 +++--- SignalMessaging/utils/LRUCache.swift | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIContacts.m b/Signal/src/ViewControllers/DebugUI/DebugUIContacts.m index 34ea4e2eb..a77549a58 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIContacts.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIContacts.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "DebugUIContacts.h" @@ -1164,7 +1164,7 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(count > 0); OWSAssert(batchCompletionHandler); - DDLogDebug(@"createRandomContactsBatch: %zd", count); + DDLogDebug(@"createRandomContactsBatch: %zu", count); CNAuthorizationStatus status = [CNContactStore authorizationStatusForEntityType:CNEntityTypeContacts]; if (status == CNAuthorizationStatusDenied || status == CNAuthorizationStatusRestricted) { @@ -1219,7 +1219,7 @@ NS_ASSUME_NONNULL_BEGIN } } - DDLogError(@"Saving fake contacts: %zd", contacts.count); + DDLogError(@"Saving fake contacts: %zu", contacts.count); NSError *saveError = nil; if (![store executeSaveRequest:request error:&saveError]) { diff --git a/SignalMessaging/utils/LRUCache.swift b/SignalMessaging/utils/LRUCache.swift index 9a252dcee..002c8d1a6 100644 --- a/SignalMessaging/utils/LRUCache.swift +++ b/SignalMessaging/utils/LRUCache.swift @@ -43,12 +43,22 @@ public class LRUCache { selector: #selector(didReceiveMemoryWarning), name: NSNotification.Name.UIApplicationDidReceiveMemoryWarning, object: nil) + NotificationCenter.default.addObserver(self, + selector: #selector(didEnterBackground), + name: NSNotification.Name.OWSApplicationDidEnterBackground, + object: nil) } deinit { NotificationCenter.default.removeObserver(self) } + @objc func didEnterBackground() { + SwiftAssertIsOnMainThread(#function) + + clear() + } + @objc func didReceiveMemoryWarning() { SwiftAssertIsOnMainThread(#function) From fbbb9276b9a16e3a71d16ab311a3379a68f77f15 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jun 2018 16:22:17 -0400 Subject: [PATCH 11/11] Respond to CR. --- SignalMessaging/contacts/OWSContactsManager.m | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/SignalMessaging/contacts/OWSContactsManager.m b/SignalMessaging/contacts/OWSContactsManager.m index ab1a734f6..5ba9a0e05 100644 --- a/SignalMessaging/contacts/OWSContactsManager.m +++ b/SignalMessaging/contacts/OWSContactsManager.m @@ -36,8 +36,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification @property (nonatomic, readonly) SystemContactsFetcher *systemContactsFetcher; @property (nonatomic, readonly) YapDatabaseConnection *dbReadConnection; @property (nonatomic, readonly) YapDatabaseConnection *dbWriteConnection; -@property (nonatomic, readonly) AnyLRUCache *cnContactCache; -@property (nonatomic, readonly) AnyLRUCache *cnContactAvatarCache; +@property (nonatomic, readonly) NSCache *cnContactCache; +@property (nonatomic, readonly) NSCache *cnContactAvatarCache; @end @@ -62,8 +62,10 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification _signalAccounts = @[]; _systemContactsFetcher = [SystemContactsFetcher new]; _systemContactsFetcher.delegate = self; - _cnContactCache = [[AnyLRUCache alloc] initWithMaxSize:50]; - _cnContactAvatarCache = [[AnyLRUCache alloc] initWithMaxSize:25]; + _cnContactCache = [NSCache new]; + _cnContactCache.countLimit = 50; + _cnContactAvatarCache = [NSCache new]; + _cnContactAvatarCache.countLimit = 25; OWSSingletonAssert(); @@ -150,11 +152,11 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification CNContact *_Nullable cnContact; @synchronized(self.cnContactCache) { - cnContact = (CNContact * _Nullable)[self.cnContactCache getWithKey:contactId]; + cnContact = [self.cnContactCache objectForKey:contactId]; if (!cnContact) { cnContact = [self.systemContactsFetcher fetchCNContactWithContactId:contactId]; if (cnContact) { - [self.cnContactCache setWithKey:contactId value:cnContact]; + [self.cnContactCache setObject:cnContact forKey:contactId]; } } } @@ -179,14 +181,14 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification UIImage *_Nullable avatarImage; @synchronized(self.cnContactAvatarCache) { - avatarImage = (UIImage * _Nullable)[self.cnContactAvatarCache getWithKey:contactId]; + avatarImage = [self.cnContactAvatarCache objectForKey:contactId]; if (!avatarImage) { NSData *_Nullable avatarData = [self avatarDataForCNContactId:contactId]; if (avatarData) { avatarImage = [UIImage imageWithData:avatarData]; } if (avatarImage) { - [self.cnContactAvatarCache setWithKey:contactId value:avatarImage]; + [self.cnContactAvatarCache setObject:avatarImage forKey:contactId]; } } } @@ -293,8 +295,8 @@ NSString *const OWSContactsManagerSignalAccountsDidChangeNotification dispatch_async(dispatch_get_main_queue(), ^{ self.allContacts = contacts; self.allContactsMap = [allContactsMap copy]; - [self.cnContactCache clear]; - [self.cnContactAvatarCache clear]; + [self.cnContactCache removeAllObjects]; + [self.cnContactAvatarCache removeAllObjects]; [self.avatarCache removeAllImages];