NSAttributedString crashes when encoding text attachment cell for missing image.
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Apr 2019 21:06:49 +0000 (21:06 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Apr 2019 21:06:49 +0000 (21:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196504
rdar://problem/49161281

Reviewed by Tim Horton.

Clean up and fix a couple of errors and crashes in the missing image path of our
attributed string converter.

Fixes include:
* Removed manual call to release on a RetainPtr, leading to autorelease pool crash.
* No longer try to load an image that is missing on disk and has long been renamed.
* No longer use a NSTextAttachmentCell in the Mac code path which can't be encoded
  for sending to the UIProcess, so it was pretty useless in the web content process.
* Stopped using NSFileWrapper for the missing image so the attachment can contain the
  retina versions of the missing image.
* Simplified bundle finding code, since WebCore is assumed to be loaded.
* Fix leak of attachment by adding missing adoptNS().

* editing/cocoa/HTMLConverter.mm:
(HTMLConverter::_addAttachmentForElement): Unify and simplify missing image path.
(_NSFirstPathForDirectoriesInDomains): Deleted.
(_NSSystemLibraryPath): Deleted.
(_webKitBundle): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243758 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/editing/cocoa/HTMLConverter.mm

index f171a68..0a9e043 100644 (file)
@@ -1,3 +1,30 @@
+2019-04-02  Timothy Hatcher  <timothy@apple.com>
+
+        NSAttributedString crashes when encoding text attachment cell for missing image.
+        https://bugs.webkit.org/show_bug.cgi?id=196504
+        rdar://problem/49161281
+
+        Reviewed by Tim Horton.
+
+        Clean up and fix a couple of errors and crashes in the missing image path of our
+        attributed string converter.
+
+        Fixes include:
+        * Removed manual call to release on a RetainPtr, leading to autorelease pool crash.
+        * No longer try to load an image that is missing on disk and has long been renamed.
+        * No longer use a NSTextAttachmentCell in the Mac code path which can't be encoded
+          for sending to the UIProcess, so it was pretty useless in the web content process.
+        * Stopped using NSFileWrapper for the missing image so the attachment can contain the
+          retina versions of the missing image.
+        * Simplified bundle finding code, since WebCore is assumed to be loaded.
+        * Fix leak of attachment by adding missing adoptNS().
+
+        * editing/cocoa/HTMLConverter.mm:
+        (HTMLConverter::_addAttachmentForElement): Unify and simplify missing image path.
+        (_NSFirstPathForDirectoriesInDomains): Deleted.
+        (_NSSystemLibraryPath): Deleted.
+        (_webKitBundle): Deleted.
+
 2019-04-02  Chris Dumez  <cdumez@apple.com>
 
         [Fetch API] Allow used body replacement in Request constructor
index e94d978..b7ab890 100644 (file)
@@ -98,6 +98,7 @@ SOFT_LINK_CLASS(UIFoundation, NSTextTab)
 #define PlatformNSColorClass        getNSColorClass()
 #define PlatformFont                UIFont
 #define PlatformFontClass           PAL::getUIFontClass()
+#define PlatformImageClass          PAL::getUIImageClass()
 
 #else
 
@@ -113,6 +114,7 @@ SOFT_LINK_CLASS(UIFoundation, NSTextTab)
 #define PlatformNSColorClass        NSColor
 #define PlatformFont                NSFont
 #define PlatformFontClass           NSFont
+#define PlatformImageClass          NSImage
 
 #endif
 
@@ -772,28 +774,6 @@ bool HTMLConverterCaches::floatPropertyValueForNode(Node& node, CSSPropertyID pr
     return false;
 }
 
-#if PLATFORM(IOS_FAMILY)
-static NSString *_NSFirstPathForDirectoriesInDomains(NSSearchPathDirectory directory, NSSearchPathDomainMask domainMask, BOOL expandTilde)
-{
-    NSArray *array = NSSearchPathForDirectoriesInDomains(directory, domainMask, expandTilde);
-    return [array count] >= 1 ? [array objectAtIndex:0] : nil;
-}
-
-static NSString *_NSSystemLibraryPath(void)
-{
-    return _NSFirstPathForDirectoriesInDomains(NSLibraryDirectory, NSSystemDomainMask, YES);
-}
-
-static NSBundle *_webKitBundle()
-{
-    // FIXME: This should probably use the WebCore bundle to avoid the layering violation.
-    NSBundle *bundle = [NSBundle bundleWithIdentifier:@"com.apple.WebKit"];
-    if (!bundle)
-        bundle = [NSBundle bundleWithPath:[_NSSystemLibraryPath() stringByAppendingPathComponent:@"Frameworks/WebKit.framework"]];
-    return bundle;
-}
-#endif
-
 static inline NSShadow *_shadowForShadowStyle(NSString *shadowStyle)
 {
     NSShadow *shadow = nil;
@@ -1359,19 +1339,15 @@ BOOL HTMLConverter::_addAttachmentForElement(Element& element, NSURL *url, BOOL
                 [attachment setIgnoresOrientation:YES];
 #endif
         } else {
+            NSBundle *webCoreBundle = [NSBundle bundleWithIdentifier:@"com.apple.WebCore"];
 #if PLATFORM(IOS_FAMILY)
-            [attachment release];
-            NSURL *missingImageURL = [_webKitBundle() URLForResource:@"missing_image" withExtension:@"tiff"];
-            ASSERT_WITH_MESSAGE(missingImageURL != nil, "Unable to find missing_image.tiff!");
-            NSFileWrapper *missingImageFileWrapper = [[[NSFileWrapper alloc] initWithURL:missingImageURL options:0 error:NULL] autorelease];
-            attachment = [[PlatformNSTextAttachment alloc] initWithFileWrapper:missingImageFileWrapper];
+            UIImage *missingImage = [PlatformImageClass imageNamed:@"missingImage" inBundle:webCoreBundle compatibleWithTraitCollection:nil];
 #else
-            static NSImage *missingImage = nil;
-            NSTextAttachmentCell *cell;
-            cell = [[NSTextAttachmentCell alloc] initImageCell:missingImage];
-            [attachment setAttachmentCell:cell];
-            [cell release];
+            NSImage *missingImage = [webCoreBundle imageForResource:@"missingImage"];
 #endif
+            ASSERT_WITH_MESSAGE(missingImage != nil, "Unable to find missingImage.");
+            attachment = adoptNS([[PlatformNSTextAttachment alloc] initWithData:nil ofType:nil]);
+            attachment.get().image = missingImage;
         }
         [_attrStr replaceCharactersInRange:rangeToReplace withString:string.get()];
         rangeToReplace.length = [string length];