From c2787341aeead7d0a2f1334fe34db3efeca68f5f Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 15 Feb 2018 11:30:41 -0500 Subject: [PATCH 1/3] Fix handling of URLs in SAE. --- .../ShareViewController.swift | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index a21af0693..bc31902d1 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -90,7 +90,7 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed Logger.debug("\(strongSelf.logTag) setup is slow - showing loading screen") strongSelf.showPrimaryViewController(loadViewController) - }.retainUntilComplete() + }.retainUntilComplete() // We shouldn't set up our environment until after we've consulted isReadyForAppExtensions. AppSetup.setupEnvironment({ @@ -493,19 +493,19 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed strongSelf.loadViewController = nil strongSelf.showPrimaryViewController(conversationPicker) Logger.info("showing picker with attachment: \(attachment)") - }.catch {[weak self] error in - AssertIsOnMainThread() - guard let strongSelf = self else { return } + }.catch {[weak self] error in + AssertIsOnMainThread() + guard let strongSelf = self else { return } - let alertTitle = NSLocalizedString("SHARE_EXTENSION_UNABLE_TO_BUILD_ATTACHMENT_ALERT_TITLE", - comment: "Shown when trying to share content to a Signal user for the share extension. Followed by failure details.") - OWSAlerts.showAlert(withTitle: alertTitle, - message: error.localizedDescription, - buttonTitle: CommonStrings.cancelButton) { _ in - strongSelf.shareViewWasCancelled() - } - owsFail("\(strongSelf.logTag) building attachment failed with error: \(error)") - }.retainUntilComplete() + let alertTitle = NSLocalizedString("SHARE_EXTENSION_UNABLE_TO_BUILD_ATTACHMENT_ALERT_TITLE", + comment: "Shown when trying to share content to a Signal user for the share extension. Followed by failure details.") + OWSAlerts.showAlert(withTitle: alertTitle, + message: error.localizedDescription, + buttonTitle: CommonStrings.cancelButton) { _ in + strongSelf.shareViewWasCancelled() + } + owsFail("\(strongSelf.logTag) building attachment failed with error: \(error)") + }.retainUntilComplete() } private class func itemMatchesSpecificUtiType(itemProvider: NSItemProvider, utiType: String) -> Bool { @@ -612,7 +612,7 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed let loadCompletion: NSItemProvider.CompletionHandler = { [weak self] (value, error) in - guard let strongSelf = self else { return } + guard let strongSelf = self else { return } guard error == nil else { reject(error!) @@ -698,7 +698,11 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed } // See comments on NSItemProvider+OWS.h. - itemProvider.loadData(forTypeIdentifier: srcUtiType, options: nil, completionHandler: loadCompletion) + if srcUtiType == kUTTypeURL as String { + itemProvider.loadItem(forTypeIdentifier: srcUtiType, options: nil, completionHandler: loadCompletion) + } else { + itemProvider.loadData(forTypeIdentifier: srcUtiType, options: nil, completionHandler: loadCompletion) + } return promise.then { [weak self] (itemUrl: URL, utiType: String) -> Promise in guard let strongSelf = self else { @@ -712,7 +716,7 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed } else { return itemUrl } - }() + }() Logger.debug("\(strongSelf.logTag) building DataSource with url: \(url), utiType: \(utiType)") From 7ea1f3d926a758eb6645e13dde24c18a65ae72fa Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 15 Feb 2018 13:34:00 -0500 Subject: [PATCH 2/3] Fix handling of file types in SAE. --- Signal.xcodeproj/project.pbxproj | 6 ----- .../ShareViewController.swift | 9 +------ .../SignalShareExtension-Bridging-Header.h | 1 - .../utils/NSItemProvider+OWS.h | 26 ------------------- .../utils/NSItemProvider+OWS.m | 24 ----------------- 5 files changed, 1 insertion(+), 65 deletions(-) delete mode 100644 SignalShareExtension/utils/NSItemProvider+OWS.h delete mode 100644 SignalShareExtension/utils/NSItemProvider+OWS.m diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 01cd92888..bb7f74882 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -10,7 +10,6 @@ 2AE2882E4C2B96BFFF9EE27C /* Pods_SignalShareExtension.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0F94C85CB0B235DA37F68ED0 /* Pods_SignalShareExtension.framework */; }; 340B02BA1FA0D6C700F9CFEC /* ConversationViewItemTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 340B02B91FA0D6C700F9CFEC /* ConversationViewItemTest.m */; }; 340CB2271EAC25820001CAA1 /* UpdateGroupViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 340CB2261EAC25820001CAA1 /* UpdateGroupViewController.m */; }; - 341F1BEE20111A5300111571 /* NSItemProvider+OWS.m in Sources */ = {isa = PBXBuildFile; fileRef = 341F1BEC20111A5200111571 /* NSItemProvider+OWS.m */; }; 341F2C0F1F2B8AE700D07D6B /* DebugUIMisc.m in Sources */ = {isa = PBXBuildFile; fileRef = 341F2C0E1F2B8AE700D07D6B /* DebugUIMisc.m */; }; 3430FE181F7751D4000EC51B /* GiphyAPI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3430FE171F7751D4000EC51B /* GiphyAPI.swift */; }; 34330A5A1E7875FB00DF2FB9 /* fontawesome-webfont.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 34330A591E7875FB00DF2FB9 /* fontawesome-webfont.ttf */; }; @@ -485,8 +484,6 @@ 340CB2251EAC25820001CAA1 /* UpdateGroupViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = UpdateGroupViewController.h; sourceTree = ""; }; 340CB2261EAC25820001CAA1 /* UpdateGroupViewController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = UpdateGroupViewController.m; sourceTree = ""; }; 341458471FBE11C4005ABCF9 /* fa */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fa; path = translations/fa.lproj/Localizable.strings; sourceTree = ""; }; - 341F1BEC20111A5200111571 /* NSItemProvider+OWS.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "NSItemProvider+OWS.m"; sourceTree = ""; }; - 341F1BED20111A5300111571 /* NSItemProvider+OWS.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "NSItemProvider+OWS.h"; sourceTree = ""; }; 341F2C0D1F2B8AE700D07D6B /* DebugUIMisc.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DebugUIMisc.h; sourceTree = ""; }; 341F2C0E1F2B8AE700D07D6B /* DebugUIMisc.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = DebugUIMisc.m; sourceTree = ""; }; 3430FE171F7751D4000EC51B /* GiphyAPI.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GiphyAPI.swift; sourceTree = ""; }; @@ -1100,8 +1097,6 @@ 34480B2F1FD0921000BC14EF /* utils */ = { isa = PBXGroup; children = ( - 341F1BED20111A5300111571 /* NSItemProvider+OWS.h */, - 341F1BEC20111A5200111571 /* NSItemProvider+OWS.m */, 34480B341FD0929200BC14EF /* ShareAppExtensionContext.h */, 34480B351FD0929200BC14EF /* ShareAppExtensionContext.m */, ); @@ -2762,7 +2757,6 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - 341F1BEE20111A5300111571 /* NSItemProvider+OWS.m in Sources */, 4535186B1FC635DD00210559 /* ShareViewController.swift in Sources */, 34480B361FD0929200BC14EF /* ShareAppExtensionContext.m in Sources */, 3461284B1FD0B94000532771 /* SAELoadViewController.swift in Sources */, diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index bc31902d1..f1a1c95ea 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -690,19 +690,12 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed } else { // It's unavoidable that we may sometimes receives data types that we // don't know how to handle. - // - // See comments on NSItemProvider+OWS.h. let unexpectedTypeError = ShareViewControllerError.assertionError(description: "unexpected value: \(String(describing: value))") reject(unexpectedTypeError) } } - // See comments on NSItemProvider+OWS.h. - if srcUtiType == kUTTypeURL as String { - itemProvider.loadItem(forTypeIdentifier: srcUtiType, options: nil, completionHandler: loadCompletion) - } else { - itemProvider.loadData(forTypeIdentifier: srcUtiType, options: nil, completionHandler: loadCompletion) - } + itemProvider.loadItem(forTypeIdentifier: srcUtiType, options: nil, completionHandler: loadCompletion) return promise.then { [weak self] (itemUrl: URL, utiType: String) -> Promise in guard let strongSelf = self else { diff --git a/SignalShareExtension/SignalShareExtension-Bridging-Header.h b/SignalShareExtension/SignalShareExtension-Bridging-Header.h index 198d68085..ca17b26ea 100644 --- a/SignalShareExtension/SignalShareExtension-Bridging-Header.h +++ b/SignalShareExtension/SignalShareExtension-Bridging-Header.h @@ -6,7 +6,6 @@ #import // Separate iOS Frameworks from other imports. -#import "NSItemProvider+OWS.h" #import "ShareAppExtensionContext.h" #import #import diff --git a/SignalShareExtension/utils/NSItemProvider+OWS.h b/SignalShareExtension/utils/NSItemProvider+OWS.h deleted file mode 100644 index d026fde35..000000000 --- a/SignalShareExtension/utils/NSItemProvider+OWS.h +++ /dev/null @@ -1,26 +0,0 @@ -// -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. -// - -NS_ASSUME_NONNULL_BEGIN - -@interface NSItemProvider (OWS) - -// NSItemProvider.loadItem(forTypeIdentifier:...) is unsafe to call from Swift, -// since it can yield values of arbitrary type. It has a highly unusual design -// in which its behavior depends on the _type_ of the completion handler. -// loadItem(forTypeIdentifier:...) tries to satisfy the expected type of the -// completion handler. This "hinting" only works in Objective-C. In Swift, -// The type of the completion handler must agree with the param type. -// -// Therefore we use an Objective-C category to hint to NSItemProvider that we -// prefer an instance of NSData. -// -// See: https://developer.apple.com/documentation/foundation/nsitemprovider/1403900-loaditemfortypeidentifier -- (void)loadDataForTypeIdentifier:(NSString *)typeIdentifier - options:(nullable NSDictionary *)options - completionHandler:(nullable NSItemProviderCompletionHandler)completionHandler; - -@end - -NS_ASSUME_NONNULL_END diff --git a/SignalShareExtension/utils/NSItemProvider+OWS.m b/SignalShareExtension/utils/NSItemProvider+OWS.m deleted file mode 100644 index 2d6e3478a..000000000 --- a/SignalShareExtension/utils/NSItemProvider+OWS.m +++ /dev/null @@ -1,24 +0,0 @@ -// -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. -// - -#import "NSItemProvider+OWS.h" - -NS_ASSUME_NONNULL_BEGIN - -@implementation NSItemProvider (OWS) - -- (void)loadDataForTypeIdentifier:(NSString *)typeIdentifier - options:(nullable NSDictionary *)options - completionHandler:(nullable NSItemProviderCompletionHandler)completionHandler -{ - [self loadItemForTypeIdentifier:typeIdentifier - options:options - completionHandler:^(NSData *_Nullable item, NSError *__null_unspecified error) { - completionHandler(item, error); - }]; -} - -@end - -NS_ASSUME_NONNULL_END From 2e1b8a7b8ae96e501171a5f3dcce6764dd127b1a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 15 Feb 2018 13:49:02 -0500 Subject: [PATCH 3/3] Respond to CR. --- .../ShareViewController.swift | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index f1a1c95ea..eb4d90f84 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -90,7 +90,7 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed Logger.debug("\(strongSelf.logTag) setup is slow - showing loading screen") strongSelf.showPrimaryViewController(loadViewController) - }.retainUntilComplete() + }.retainUntilComplete() // We shouldn't set up our environment until after we've consulted isReadyForAppExtensions. AppSetup.setupEnvironment({ @@ -493,19 +493,19 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed strongSelf.loadViewController = nil strongSelf.showPrimaryViewController(conversationPicker) Logger.info("showing picker with attachment: \(attachment)") - }.catch {[weak self] error in - AssertIsOnMainThread() - guard let strongSelf = self else { return } + }.catch {[weak self] error in + AssertIsOnMainThread() + guard let strongSelf = self else { return } - let alertTitle = NSLocalizedString("SHARE_EXTENSION_UNABLE_TO_BUILD_ATTACHMENT_ALERT_TITLE", - comment: "Shown when trying to share content to a Signal user for the share extension. Followed by failure details.") - OWSAlerts.showAlert(withTitle: alertTitle, - message: error.localizedDescription, - buttonTitle: CommonStrings.cancelButton) { _ in - strongSelf.shareViewWasCancelled() - } - owsFail("\(strongSelf.logTag) building attachment failed with error: \(error)") - }.retainUntilComplete() + let alertTitle = NSLocalizedString("SHARE_EXTENSION_UNABLE_TO_BUILD_ATTACHMENT_ALERT_TITLE", + comment: "Shown when trying to share content to a Signal user for the share extension. Followed by failure details.") + OWSAlerts.showAlert(withTitle: alertTitle, + message: error.localizedDescription, + buttonTitle: CommonStrings.cancelButton) { _ in + strongSelf.shareViewWasCancelled() + } + owsFail("\(strongSelf.logTag) building attachment failed with error: \(error)") + }.retainUntilComplete() } private class func itemMatchesSpecificUtiType(itemProvider: NSItemProvider, utiType: String) -> Bool { @@ -709,7 +709,7 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed } else { return itemUrl } - }() + }() Logger.debug("\(strongSelf.logTag) building DataSource with url: \(url), utiType: \(utiType)")