ASSERTION FAILED: ASSERT(!containsImage || MIMETypeRegistry::isSupportedImageResource...
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Mar 2018 21:01:35 +0000 (21:01 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Mar 2018 21:01:35 +0000 (21:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184161

Reviewed by Per Arne Vollan.

.:

* ManualTests/DragInlinePDFImageDocument.html: Added.
* ManualTests/resources/simple.pdf: Added.

Source/WebCore:

Fixes an assertion failure when quiting an app that uses a Legacy WebKit web view
after dragging-and-dropping a PDF embedded using an HTML image element into the
same web view.

When performing a drag-and-drop of a PDF document image (WebCore::PDFDocumentImage) we create a WebArchive
from the main frame's WebHTMLView and promise AppKit that will provide a Rich Text Format (RTF) document
from this archive if needed. For some reason, on app termination AppKit requests that the WebHTMLView
fullfill its RTF document promise for the WebArchive created at the start of the drag operation. WebKit
expects that the created WebArchive is either for an inline image (e.g. <img>) or an image document that
has a supported image resource MIME type (by querying MIMETypeRegistry::isSupportedImageResourceMIMEType())
and checks for these cases in this order. PDF/PostScript are not listed in the set of supported image
resource MIME types. So, the first check fails and WebKit assumes that the WebArchive was created from
an image document of a supported image resource MIME type. However, the WebArchive was created from a
WebHTMLView and has MIME type text/html. Therefore the assertion fails. We need to add PDF and PostScript
to the set of supported image resource MIME types so that WebKit does not fall back to the WebHTMLView
code path. Historically, PDF and PostScript were in the set supported image resource MIME types. Over time
the set of MIME types for image resouces (images loaded as a document) became identical to the set of MIME
types for images loaded inline (e.g. <img>) and this set omitted the MIME types for PDF and PostScript.

Additionally it is sufficient to implement MIMETypeRegistry::isSupportedImageResourceMIMEType() in terms
of MIMETypeRegistry::isSupportedImageMIMEType() and MIMETypeRegistry::isPDFOrPostScriptMIMEType() instead
of allocating a dedicated HashSet for the supported image resource MIME types (as we currently do).

* dom/DOMImplementation.cpp:
(WebCore::DOMImplementation::createDocument): Assert that PDF is a supported image MIME type before
instantiating an ImageDocument.
* platform/MIMETypeRegistry.cpp:
(WebCore::initializeSupportedImageMIMETypes): Remove unnecessary allocation of a HashSet for the support
image resource MIME types.
(WebCore::MIMETypeRegistry::isSupportedImageResourceMIMEType): Write in terms of MIMETypeRegistry::isSupportedImageMIMEType()
and MIMETypeRegistry::isPDFOrPostScriptMIMEType().
(WebCore::MIMETypeRegistry::getSupportedImageResourceMIMETypes): Deleted.
* platform/MIMETypeRegistry.h:

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

ChangeLog
ManualTests/DragInlinePDFImageDocument.html [new file with mode: 0644]
ManualTests/resources/simple.pdf [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/DOMImplementation.cpp
Source/WebCore/platform/MIMETypeRegistry.cpp
Source/WebCore/platform/MIMETypeRegistry.h

index 0fc31a4..847aa45 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2018-03-30  Daniel Bates  <dabates@apple.com>
+
+        ASSERTION FAILED: ASSERT(!containsImage || MIMETypeRegistry::isSupportedImageResourceMIMEType([resource MIMEType])) in -[NSPasteboard(WebExtras) _web_writePromisedRTFDFromArchive:containsImage:]
+        https://bugs.webkit.org/show_bug.cgi?id=184161
+
+        Reviewed by Per Arne Vollan.
+
+        * ManualTests/DragInlinePDFImageDocument.html: Added.
+        * ManualTests/resources/simple.pdf: Added.
+
 2018-03-28  Tim Horton  <timothy_horton@apple.com>
 
         Make it possible to disable building the tools with Make
diff --git a/ManualTests/DragInlinePDFImageDocument.html b/ManualTests/DragInlinePDFImageDocument.html
new file mode 100644 (file)
index 0000000..99f7eaf
--- /dev/null
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This is a test for <a href="https://bugs.webkit.org/show_bug.cgi?id=184161">Bug 184161</a>. Perform the following using a debug build of Legacy WebKit:</p>
+<ol>
+    <li>Drag and drop the PDF (the content demarcated with a black border on the page below) within the web view.</li>
+    <li>Quit this app.</li>
+</ol>
+<p>This test PASSED if this app does not crash with an assertion failure.</p>
+<img src="resources/simple.pdf" style="border:1px solid black">
+</body>
+</html>
diff --git a/ManualTests/resources/simple.pdf b/ManualTests/resources/simple.pdf
new file mode 100644 (file)
index 0000000..8f2eeaf
Binary files /dev/null and b/ManualTests/resources/simple.pdf differ
index 3bae427..b5bf781 100644 (file)
@@ -1,3 +1,44 @@
+2018-03-30  Daniel Bates  <dabates@apple.com>
+
+        ASSERTION FAILED: ASSERT(!containsImage || MIMETypeRegistry::isSupportedImageResourceMIMEType([resource MIMEType])) in -[NSPasteboard(WebExtras) _web_writePromisedRTFDFromArchive:containsImage:]
+        https://bugs.webkit.org/show_bug.cgi?id=184161
+
+        Reviewed by Per Arne Vollan.
+
+        Fixes an assertion failure when quiting an app that uses a Legacy WebKit web view
+        after dragging-and-dropping a PDF embedded using an HTML image element into the
+        same web view.
+
+        When performing a drag-and-drop of a PDF document image (WebCore::PDFDocumentImage) we create a WebArchive
+        from the main frame's WebHTMLView and promise AppKit that will provide a Rich Text Format (RTF) document
+        from this archive if needed. For some reason, on app termination AppKit requests that the WebHTMLView
+        fullfill its RTF document promise for the WebArchive created at the start of the drag operation. WebKit
+        expects that the created WebArchive is either for an inline image (e.g. <img>) or an image document that
+        has a supported image resource MIME type (by querying MIMETypeRegistry::isSupportedImageResourceMIMEType())
+        and checks for these cases in this order. PDF/PostScript are not listed in the set of supported image
+        resource MIME types. So, the first check fails and WebKit assumes that the WebArchive was created from
+        an image document of a supported image resource MIME type. However, the WebArchive was created from a
+        WebHTMLView and has MIME type text/html. Therefore the assertion fails. We need to add PDF and PostScript
+        to the set of supported image resource MIME types so that WebKit does not fall back to the WebHTMLView
+        code path. Historically, PDF and PostScript were in the set supported image resource MIME types. Over time
+        the set of MIME types for image resouces (images loaded as a document) became identical to the set of MIME
+        types for images loaded inline (e.g. <img>) and this set omitted the MIME types for PDF and PostScript.
+
+        Additionally it is sufficient to implement MIMETypeRegistry::isSupportedImageResourceMIMEType() in terms
+        of MIMETypeRegistry::isSupportedImageMIMEType() and MIMETypeRegistry::isPDFOrPostScriptMIMEType() instead
+        of allocating a dedicated HashSet for the supported image resource MIME types (as we currently do).
+
+        * dom/DOMImplementation.cpp:
+        (WebCore::DOMImplementation::createDocument): Assert that PDF is a supported image MIME type before
+        instantiating an ImageDocument.
+        * platform/MIMETypeRegistry.cpp:
+        (WebCore::initializeSupportedImageMIMETypes): Remove unnecessary allocation of a HashSet for the support
+        image resource MIME types.
+        (WebCore::MIMETypeRegistry::isSupportedImageResourceMIMEType): Write in terms of MIMETypeRegistry::isSupportedImageMIMEType()
+        and MIMETypeRegistry::isPDFOrPostScriptMIMEType().
+        (WebCore::MIMETypeRegistry::getSupportedImageResourceMIMETypes): Deleted.
+        * platform/MIMETypeRegistry.h:
+
 2018-03-29  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] CSSTransition objects should have fill: backwards to allow seeking prior to start time
index 326d568..3041fde 100644 (file)
@@ -150,8 +150,10 @@ Ref<Document> DOMImplementation::createDocument(const String& type, Frame* frame
 #endif
 
     // If we want to useImageDocumentForSubframePDF, we'll let that override plugin support.
-    if (frame && !frame->isMainFrame() && MIMETypeRegistry::isPDFMIMEType(type) && frame->settings().useImageDocumentForSubframePDF())
+    if (frame && !frame->isMainFrame() && MIMETypeRegistry::isPDFMIMEType(type) && frame->settings().useImageDocumentForSubframePDF()) {
+        ASSERT(Image::supportsType(type));
         return ImageDocument::create(*frame, url);
+    }
 
     PluginData* pluginData = nullptr;
     auto allowedPluginTypes = PluginData::OnlyApplicationPlugins;
index 017bd99..36a00b7 100644 (file)
@@ -55,7 +55,6 @@
 
 namespace WebCore {
 
-static HashSet<String, ASCIICaseInsensitiveHash>* supportedImageResourceMIMETypes;
 static HashSet<String, ASCIICaseInsensitiveHash>* supportedImageMIMETypes;
 static HashSet<String, ASCIICaseInsensitiveHash>* supportedImageMIMETypesForEncoding;
 static HashSet<String, ASCIICaseInsensitiveHash>* supportedJavaScriptMIMETypes;
@@ -66,24 +65,19 @@ static HashSet<String, ASCIICaseInsensitiveHash>* unsupportedTextMIMETypes;
 
 static void initializeSupportedImageMIMETypes()
 {
-    supportedImageResourceMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
     supportedImageMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
 
 #if USE(CG)
     // This represents the subset of allowed image UTIs for which CoreServices has a corresponding MIME type. Keep this in sync with allowedImageUTIs().
     static const char* const allowedImageMIMETypes[] = { "image/tiff", "image/gif", "image/jpeg", "image/vnd.microsoft.icon", "image/jp2", "image/png", "image/bmp" };
-    for (auto& mimeType : allowedImageMIMETypes) {
+    for (auto& mimeType : allowedImageMIMETypes)
         supportedImageMIMETypes->add(ASCIILiteral { mimeType });
-        supportedImageResourceMIMETypes->add(ASCIILiteral { mimeType });
-    }
 
 #ifndef NDEBUG
     for (auto& uti : allowedImageUTIs()) {
         auto mimeType = MIMETypeForImageSourceType(uti);
-        if (!mimeType.isEmpty()) {
+        if (!mimeType.isEmpty())
             ASSERT(supportedImageMIMETypes->contains(mimeType));
-            ASSERT(supportedImageResourceMIMETypes->contains(mimeType));
-        }
     }
 
 #if PLATFORM(COCOA)
@@ -94,11 +88,9 @@ static void initializeSupportedImageMIMETypes()
 
     // Favicons don't have a MIME type in the registry either.
     supportedImageMIMETypes->add("image/x-icon");
-    supportedImageResourceMIMETypes->add("image/x-icon");
 
     //  We only get one MIME type per UTI, hence our need to add these manually
     supportedImageMIMETypes->add("image/pjpeg");
-    supportedImageResourceMIMETypes->add("image/pjpeg");
 
 #if PLATFORM(IOS)
     // Add malformed image mimetype for compatibility with Mail and to handle malformed mimetypes from the net
@@ -122,10 +114,8 @@ static void initializeSupportedImageMIMETypes()
         "image/x-bmp", "image/x-win-bitmap", "image/x-windows-bmp", "image/ms-bmp", "image/x-ms-bmp",
         "application/bmp", "application/x-bmp", "application/x-win-bitmap",
     };
-    for (auto& type : malformedMIMETypes) {
+    for (auto& type : malformedMIMETypes)
         supportedImageMIMETypes->add(type);
-        supportedImageResourceMIMETypes->add(type);
-    }
 #endif
 
 #else
@@ -140,14 +130,11 @@ static void initializeSupportedImageMIMETypes()
         "image/x-icon",    // ico
         "image/x-xbitmap"  // xbm
     };
-    for (auto& type : types) {
+    for (auto& type : types)
         supportedImageMIMETypes->add(type);
-        supportedImageResourceMIMETypes->add(type);
-    }
 
 #if USE(WEBP)
     supportedImageMIMETypes->add("image/webp");
-    supportedImageResourceMIMETypes->add("image/webp");
 #endif
 
 #endif // USE(CG)
@@ -463,11 +450,7 @@ bool MIMETypeRegistry::isSupportedImageVideoOrSVGMIMEType(const String& mimeType
 
 bool MIMETypeRegistry::isSupportedImageResourceMIMEType(const String& mimeType)
 {
-    if (mimeType.isEmpty())
-        return false;
-    if (!supportedImageResourceMIMETypes)
-        initializeSupportedImageMIMETypes();
-    return supportedImageResourceMIMETypes->contains(getNormalizedMIMEType(mimeType));
+    return isSupportedImageMIMEType(mimeType) || isPDFOrPostScriptMIMEType(mimeType);
 }
 
 bool MIMETypeRegistry::isSupportedImageMIMETypeForEncoding(const String& mimeType)
@@ -658,13 +641,6 @@ const HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getSupportedI
     return *supportedImageMIMETypes;
 }
 
-const HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getSupportedImageResourceMIMETypes()
-{
-    if (!supportedImageResourceMIMETypes)
-        initializeSupportedImageMIMETypes();
-    return *supportedImageResourceMIMETypes;
-}
-
 HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getSupportedNonImageMIMETypes()
 {
     if (!supportedNonImageMIMETypes)
index 9244973..eb6dc52 100644 (file)
@@ -109,7 +109,6 @@ public:
     WEBCORE_EXPORT static HashSet<String, ASCIICaseInsensitiveHash>& getSupportedNonImageMIMETypes();
 
     WEBCORE_EXPORT const static HashSet<String, ASCIICaseInsensitiveHash>& getSupportedImageMIMETypes();
-    const static HashSet<String, ASCIICaseInsensitiveHash>& getSupportedImageResourceMIMETypes();
     WEBCORE_EXPORT const static HashSet<String, ASCIICaseInsensitiveHash>& getSupportedMediaMIMETypes();
     WEBCORE_EXPORT const static HashSet<String, ASCIICaseInsensitiveHash>& getPDFMIMETypes();
     WEBCORE_EXPORT const static HashSet<String, ASCIICaseInsensitiveHash>& getUnsupportedTextMIMETypes();