Work towards <rdar://problem/31286130>
Created attachment 307074 [details] Part two
Created attachment 307093 [details] Patch
Created attachment 307137 [details] Patch
Comment on attachment 307137 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307137&action=review > Source/WebCore/ChangeLog:22 > + (-[WebItemProviderPasteboard tryToLoadProvidedContentIntoFileURLs:]): I renamed this method in the source, but didn't update the ChangeLogs -- I'll fix this.
Comment on attachment 307137 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307137&action=review > Source/WebCore/platform/ios/WebItemProviderPasteboard.h:34 > +typedef void (^WebItemProviderFileLoadBlock)(NSArray <NSURL *> *); I think our style is to not include a space between NSArray and <. > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:267 > + NSString *temporaryFilename = suggestedName ?: (NSString *)String::number(cryptographicallyRandomNumber()); If each temporary file is stored inside a unique directory name, and a suggested name isn't provided, do we need to use a cryptographically random name for the temporary file? Could it just be a fixed name? > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:280 > + if (!UTTypeConformsTo((__bridge CFStringRef)identifier, kUTTypeContent)) Is a bridging cast needed here? > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:298 > + dispatch_group_t fileLoadingGroup = dispatch_group_create(); Can we use OSObjectPtr here? > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:303 > + dispatch_group_async(fileLoadingGroup, dispatch_get_main_queue(), ^() { No need to specify () in a no-argument block. > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:313 > + [[NSFileManager defaultManager] linkItemAtURL:url toURL:destinationURL.get() error:&hardLinkError]; > + if (!hardLinkError) It's idiomatic to check the return value of -linkItemAtURL:toURL:error: instead of checking for a non-nil error. We guarantee that error will be non-nil when an error occurs, but not that it'll be nil when no error occurs. > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:321 > + dispatch_group_notify(fileLoadingGroup, dispatch_get_main_queue(), ^() { No need to specify () in a no-argument block.
Created attachment 307154 [details] Patch
Comment on attachment 307154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307154&action=review You should note all the reviewers in your something somewhere and figure out if it's enough. > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:297 > + auto fileLoadingGroup = adoptOSObject(dispatch_group_create()); extra space after = > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:302 > + dispatch_group_async(fileLoadingGroup.get(), dispatch_get_main_queue(), ^ { You're gonna use some lambadas. > Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:309 > + [[NSFileManager defaultManager] removeItemAtURL:destinationURL.get() error:nil]; You're going to get rid of this.
Comment on attachment 307154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307154&action=review >> Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:297 >> + auto fileLoadingGroup = adoptOSObject(dispatch_group_create()); > > extra space after = Good catch. Fixed. >> Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:302 >> + dispatch_group_async(fileLoadingGroup.get(), dispatch_get_main_queue(), ^ { > > You're gonna use some lambadas. Will do! >> Source/WebCore/platform/ios/WebItemProviderPasteboard.mm:309 >> + [[NSFileManager defaultManager] removeItemAtURL:destinationURL.get() error:nil]; > > You're going to get rid of this. Yep! (Dan Bernstein pointed out earlier that there is no need to try and clear out the destination URL since _webkit_createTemporaryDirectoryWithTemplatePrefix will either succeed and create a new directory, or fail and return nil, and this code the nil case by not adding a URL to the list of completed URLs.)
Created attachment 307190 [details] Patch for landing
Comment on attachment 307190 [details] Patch for landing Clearing flags on attachment: 307190 Committed r215389: <http://trac.webkit.org/changeset/215389>
Internal iOS build fix in <http://trac.webkit.org/changeset/215390>.