[iOS] DocumentWriter::createDocument can spend ~100ms unnecessarily converting image...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Oct 2017 22:37:33 +0000 (22:37 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Oct 2017 22:37:33 +0000 (22:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178618
<rdar://problem/35108852>

Reviewed by Said Abou-Hallawa.

Currently, in setting up a new Document, DocumentWriter::createDocument() always asks whether or not the
Document should be a PDF document by calling MIMETypeRegistry::isPDFMIMEType(), which forces lazy initialization
of every MIME type dictionary (e.g. image types, PDF types, JavaScript types, etc.). As evidenced by traces,
this can be an expensive operation on certain devices.

This patch implements two optimizations. First, we refactor the initializeSupportedImageMIMETypes() helper to
stop asking for MIMETypeForImageSourceType for each of the supported UTIs. This is because the known MIME types
corresponding to these hard-coded UTI types is a fixed set anyways, so we can simply iterate over a constant
array of MIME types and populate the supported image (and image resource) types. Also, add assertions to ensure
that we keep allowed image MIME types in sync with allowed image UTIs.

The second optimization removes initializeMIMETypeRegistry() altogether in favor of calling just the
initialize*MIMETypes() functions needed to ensure the information required. For instance, getPDFMIMETypes()
currently calls initializeMIMETypeRegistry() if the pdfMIMETypes dictionary doesn't exist, when it really only
needs to ensure that the pdfMIMETypes is initialized, for which initializePDFMIMETypes() is sufficient.

* platform/MIMETypeRegistry.cpp:
(WebCore::initializeSupportedImageMIMETypes):
(WebCore::initializeSupportedJavaScriptMIMETypes):
(WebCore::initializePDFMIMETypes):
(WebCore::initializeSupportedNonImageMimeTypes):
(WebCore::initializeUnsupportedTextMIMETypes):

Move MIME type dictionary creation into initialize*MIMETypes() helpers. Additionally, remove
initializePDFAndPostScriptMIMETypes, which is no longer necessary.

(WebCore::MIMETypeRegistry::isSupportedImageMIMEType):
(WebCore::MIMETypeRegistry::isSupportedImageResourceMIMEType):
(WebCore::MIMETypeRegistry::isSupportedJavaScriptMIMEType):
(WebCore::MIMETypeRegistry::isSupportedNonImageMIMEType):
(WebCore::MIMETypeRegistry::isUnsupportedTextMIMEType):
(WebCore::MIMETypeRegistry::isPDFOrPostScriptMIMEType):

Tweak to check that the type isPDFMIMEType(), or that it's otherwise "application/postscript".

(WebCore::MIMETypeRegistry::isPDFMIMEType):
(WebCore::MIMETypeRegistry::getSupportedImageMIMETypes):
(WebCore::MIMETypeRegistry::getSupportedImageResourceMIMETypes):
(WebCore::MIMETypeRegistry::getSupportedNonImageMIMETypes):
(WebCore::MIMETypeRegistry::getPDFMIMETypes):
(WebCore::MIMETypeRegistry::getUnsupportedTextMIMETypes):

Call only the relevant MIME type initializers when needed.

(WebCore::initializePostScriptMIMETypes): Deleted.
(WebCore::initializeMIMETypeRegistry): Deleted.
(WebCore::MIMETypeRegistry::getPDFAndPostScriptMIMETypes): Deleted.

Remove an unused and unexported function.

* platform/MIMETypeRegistry.h:

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

Source/WebCore/ChangeLog
Source/WebCore/platform/MIMETypeRegistry.cpp
Source/WebCore/platform/MIMETypeRegistry.h

index e3146bc..84c336f 100644 (file)
@@ -1,3 +1,63 @@
+2017-10-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] DocumentWriter::createDocument can spend ~100ms unnecessarily converting image UTIs to MIME types
+        https://bugs.webkit.org/show_bug.cgi?id=178618
+        <rdar://problem/35108852>
+
+        Reviewed by Said Abou-Hallawa.
+
+        Currently, in setting up a new Document, DocumentWriter::createDocument() always asks whether or not the
+        Document should be a PDF document by calling MIMETypeRegistry::isPDFMIMEType(), which forces lazy initialization
+        of every MIME type dictionary (e.g. image types, PDF types, JavaScript types, etc.). As evidenced by traces,
+        this can be an expensive operation on certain devices.
+
+        This patch implements two optimizations. First, we refactor the initializeSupportedImageMIMETypes() helper to
+        stop asking for MIMETypeForImageSourceType for each of the supported UTIs. This is because the known MIME types
+        corresponding to these hard-coded UTI types is a fixed set anyways, so we can simply iterate over a constant
+        array of MIME types and populate the supported image (and image resource) types. Also, add assertions to ensure
+        that we keep allowed image MIME types in sync with allowed image UTIs.
+
+        The second optimization removes initializeMIMETypeRegistry() altogether in favor of calling just the
+        initialize*MIMETypes() functions needed to ensure the information required. For instance, getPDFMIMETypes()
+        currently calls initializeMIMETypeRegistry() if the pdfMIMETypes dictionary doesn't exist, when it really only
+        needs to ensure that the pdfMIMETypes is initialized, for which initializePDFMIMETypes() is sufficient.
+
+        * platform/MIMETypeRegistry.cpp:
+        (WebCore::initializeSupportedImageMIMETypes):
+        (WebCore::initializeSupportedJavaScriptMIMETypes):
+        (WebCore::initializePDFMIMETypes):
+        (WebCore::initializeSupportedNonImageMimeTypes):
+        (WebCore::initializeUnsupportedTextMIMETypes):
+
+        Move MIME type dictionary creation into initialize*MIMETypes() helpers. Additionally, remove
+        initializePDFAndPostScriptMIMETypes, which is no longer necessary.
+
+        (WebCore::MIMETypeRegistry::isSupportedImageMIMEType):
+        (WebCore::MIMETypeRegistry::isSupportedImageResourceMIMEType):
+        (WebCore::MIMETypeRegistry::isSupportedJavaScriptMIMEType):
+        (WebCore::MIMETypeRegistry::isSupportedNonImageMIMEType):
+        (WebCore::MIMETypeRegistry::isUnsupportedTextMIMEType):
+        (WebCore::MIMETypeRegistry::isPDFOrPostScriptMIMEType):
+
+        Tweak to check that the type isPDFMIMEType(), or that it's otherwise "application/postscript".
+
+        (WebCore::MIMETypeRegistry::isPDFMIMEType):
+        (WebCore::MIMETypeRegistry::getSupportedImageMIMETypes):
+        (WebCore::MIMETypeRegistry::getSupportedImageResourceMIMETypes):
+        (WebCore::MIMETypeRegistry::getSupportedNonImageMIMETypes):
+        (WebCore::MIMETypeRegistry::getPDFMIMETypes):
+        (WebCore::MIMETypeRegistry::getUnsupportedTextMIMETypes):
+
+        Call only the relevant MIME type initializers when needed.
+
+        (WebCore::initializePostScriptMIMETypes): Deleted.
+        (WebCore::initializeMIMETypeRegistry): Deleted.
+        (WebCore::MIMETypeRegistry::getPDFAndPostScriptMIMETypes): Deleted.
+
+        Remove an unused and unexported function.
+
+        * platform/MIMETypeRegistry.h:
+
 2017-10-23  Andy Estes  <aestes@apple.com>
 
         [Payment Request] Take the JSC API lock before creating the PaymentResponse.details object
index 97d9ef2..4a9a208 100644 (file)
@@ -36,6 +36,7 @@
 #if USE(CG)
 #include "ImageSourceCG.h"
 #include "UTIRegistry.h"
+#include "UTIUtilities.h"
 #include <wtf/RetainPtr.h>
 #endif
 
@@ -60,43 +61,42 @@ static HashSet<String, ASCIICaseInsensitiveHash>* supportedJavaScriptMIMETypes;
 static HashSet<String, ASCIICaseInsensitiveHash>* supportedNonImageMIMETypes;
 static HashSet<String, ASCIICaseInsensitiveHash>* supportedMediaMIMETypes;
 static HashSet<String, ASCIICaseInsensitiveHash>* pdfMIMETypes;
-static HashSet<String, ASCIICaseInsensitiveHash>* pdfAndPostScriptMIMETypes;
 static HashSet<String, ASCIICaseInsensitiveHash>* unsupportedTextMIMETypes;
 
 static void initializeSupportedImageMIMETypes()
 {
 #if USE(CG)
-    HashSet<String>& imageUTIs = allowedImageUTIs();
-    for (auto& imageUTI : imageUTIs) {
-        String mimeType = MIMETypeForImageSourceType(imageUTI);
+    supportedImageResourceMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
+    supportedImageMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
+
+    // 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) {
+        supportedImageMIMETypes->add(ASCIILiteral { mimeType });
+        supportedImageResourceMIMETypes->add(ASCIILiteral { mimeType });
+    }
+
+#ifndef NDEBUG
+    for (auto& uti : allowedImageUTIs()) {
+        auto mimeType = MIMETypeForImageSourceType(uti);
         if (!mimeType.isEmpty()) {
-            supportedImageMIMETypes->add(mimeType);
-            supportedImageResourceMIMETypes->add(mimeType);
+            ASSERT(supportedImageMIMETypes->contains(mimeType));
+            ASSERT(supportedImageResourceMIMETypes->contains(mimeType));
         }
     }
 
-    // On Tiger and Leopard, com.microsoft.bmp doesn't have a MIME type in the registry.
-    supportedImageMIMETypes->add("image/bmp");
-    supportedImageResourceMIMETypes->add("image/bmp");
+    for (auto& mime : *supportedImageMIMETypes)
+        ASSERT_UNUSED(mime, allowedImageUTIs().contains(UTIFromMIMEType(mime)));
+#endif
 
     // Favicons don't have a MIME type in the registry either.
-    supportedImageMIMETypes->add("image/vnd.microsoft.icon");
     supportedImageMIMETypes->add("image/x-icon");
-    supportedImageResourceMIMETypes->add("image/vnd.microsoft.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");
 
-    //  We don't want to try to treat all binary data as an image
-    supportedImageMIMETypes->remove("application/octet-stream");
-    supportedImageResourceMIMETypes->remove("application/octet-stream");
-
-    //  Don't treat pdf/postscript as images directly
-    supportedImageMIMETypes->remove("application/pdf");
-    supportedImageMIMETypes->remove("application/postscript");
-
 #if PLATFORM(IOS)
     // Add malformed image mimetype for compatibility with Mail and to handle malformed mimetypes from the net
     // These were removed for <rdar://problem/6564538> Re-enable UTI code in WebCore now that MobileCoreServices exists
@@ -203,6 +203,8 @@ static void initializeSupportedJavaScriptMIMETypes()
         "text/x-javascript",
         "text/x-ecmascript"
     };
+
+    supportedJavaScriptMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
     for (auto* type : types)
         supportedJavaScriptMIMETypes->add(type);
 }
@@ -213,15 +215,12 @@ static void initializePDFMIMETypes()
         "application/pdf",
         "text/pdf"
     };
+
+    pdfMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
     for (auto& type : types)
         pdfMIMETypes->add(type);
 }
 
-static void initializePostScriptMIMETypes()
-{
-    pdfAndPostScriptMIMETypes->add("application/postscript");
-}
-
 static void initializeSupportedNonImageMimeTypes()
 {
     static const char* const types[] = {
@@ -247,6 +246,10 @@ static void initializeSupportedNonImageMimeTypes()
         // This can result in cross-site scripting vulnerabilities.
     };
 
+    if (!supportedJavaScriptMIMETypes)
+        initializeSupportedJavaScriptMIMETypes();
+
+    supportedNonImageMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash> { *supportedJavaScriptMIMETypes };
     for (auto& type : types)
         supportedNonImageMIMETypes->add(type);
 
@@ -415,30 +418,10 @@ static void initializeUnsupportedTextMIMETypes()
         "text/vnd.sun.j2me.app-descriptor",
 #endif
     };
-    for (auto& type : types)
-        unsupportedTextMIMETypes->add(ASCIILiteral { type });
-}
-
-static void initializeMIMETypeRegistry()
-{
-    supportedJavaScriptMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
-    initializeSupportedJavaScriptMIMETypes();
-
-    supportedNonImageMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>(*supportedJavaScriptMIMETypes);
-    initializeSupportedNonImageMimeTypes();
-
-    supportedImageResourceMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
-    supportedImageMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
-    initializeSupportedImageMIMETypes();
-
-    pdfMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
-    initializePDFMIMETypes();
-
-    pdfAndPostScriptMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>(*pdfMIMETypes);
-    initializePostScriptMIMETypes();
 
     unsupportedTextMIMETypes = new HashSet<String, ASCIICaseInsensitiveHash>;
-    initializeUnsupportedTextMIMETypes();
+    for (auto& type : types)
+        unsupportedTextMIMETypes->add(ASCIILiteral { type });
 }
 
 String MIMETypeRegistry::getMIMETypeForPath(const String& path)
@@ -458,7 +441,7 @@ bool MIMETypeRegistry::isSupportedImageMIMEType(const String& mimeType)
     if (mimeType.isEmpty())
         return false;
     if (!supportedImageMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeSupportedImageMIMETypes();
     return supportedImageMIMETypes->contains(getNormalizedMIMEType(mimeType));
 }
 
@@ -472,7 +455,7 @@ bool MIMETypeRegistry::isSupportedImageResourceMIMEType(const String& mimeType)
     if (mimeType.isEmpty())
         return false;
     if (!supportedImageResourceMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeSupportedImageMIMETypes();
     return supportedImageResourceMIMETypes->contains(getNormalizedMIMEType(mimeType));
 }
 
@@ -492,7 +475,7 @@ bool MIMETypeRegistry::isSupportedJavaScriptMIMEType(const String& mimeType)
     if (mimeType.isEmpty())
         return false;
     if (!supportedJavaScriptMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeSupportedNonImageMimeTypes();
     return supportedJavaScriptMIMETypes->contains(mimeType);
 }
 
@@ -537,7 +520,7 @@ bool MIMETypeRegistry::isSupportedNonImageMIMEType(const String& mimeType)
     if (mimeType.isEmpty())
         return false;
     if (!supportedNonImageMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeSupportedNonImageMimeTypes();
     return supportedNonImageMIMETypes->contains(mimeType);
 }
 
@@ -560,7 +543,7 @@ bool MIMETypeRegistry::isUnsupportedTextMIMEType(const String& mimeType)
     if (mimeType.isEmpty())
         return false;
     if (!unsupportedTextMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeUnsupportedTextMIMETypes();
     return unsupportedTextMIMETypes->contains(mimeType);
 }
 
@@ -618,11 +601,7 @@ bool MIMETypeRegistry::isJavaAppletMIMEType(const String& mimeType)
 
 bool MIMETypeRegistry::isPDFOrPostScriptMIMEType(const String& mimeType)
 {
-    if (mimeType.isEmpty())
-        return false;
-    if (!pdfAndPostScriptMIMETypes)
-        initializeMIMETypeRegistry();
-    return pdfAndPostScriptMIMETypes->contains(mimeType);
+    return isPDFMIMEType(mimeType) || mimeType == "application/postscript";
 }
 
 bool MIMETypeRegistry::isPDFMIMEType(const String& mimeType)
@@ -630,7 +609,7 @@ bool MIMETypeRegistry::isPDFMIMEType(const String& mimeType)
     if (mimeType.isEmpty())
         return false;
     if (!pdfMIMETypes)
-        initializeMIMETypeRegistry();
+        initializePDFMIMETypes();
     return pdfMIMETypes->contains(mimeType);
 }
 
@@ -651,14 +630,14 @@ bool MIMETypeRegistry::canShowMIMEType(const String& mimeType)
 HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getSupportedImageMIMETypes()
 {
     if (!supportedImageMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeSupportedImageMIMETypes();
     return *supportedImageMIMETypes;
 }
 
 HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getSupportedImageResourceMIMETypes()
 {
     if (!supportedImageResourceMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeSupportedImageMIMETypes();
     return *supportedImageResourceMIMETypes;
 }
 
@@ -672,7 +651,7 @@ HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getSupportedImageMI
 HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getSupportedNonImageMIMETypes()
 {
     if (!supportedNonImageMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeSupportedNonImageMimeTypes();
     return *supportedNonImageMIMETypes;
 }
 
@@ -687,21 +666,14 @@ HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getSupportedMediaMI
 HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getPDFMIMETypes()
 {
     if (!pdfMIMETypes)
-        initializeMIMETypeRegistry();
+        initializePDFMIMETypes();
     return *pdfMIMETypes;
 }
 
-HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getPDFAndPostScriptMIMETypes()
-{
-    if (!pdfAndPostScriptMIMETypes)
-        initializeMIMETypeRegistry();
-    return *pdfAndPostScriptMIMETypes;
-}
-
 HashSet<String, ASCIICaseInsensitiveHash>& MIMETypeRegistry::getUnsupportedTextMIMETypes()
 {
     if (!unsupportedTextMIMETypes)
-        initializeMIMETypeRegistry();
+        initializeUnsupportedTextMIMETypes();
     return *unsupportedTextMIMETypes;
 }
 
index a686f30..7872119 100644 (file)
@@ -111,7 +111,6 @@ public:
     WEBCORE_EXPORT static HashSet<String, ASCIICaseInsensitiveHash>& getSupportedNonImageMIMETypes();
     WEBCORE_EXPORT static HashSet<String, ASCIICaseInsensitiveHash>& getSupportedMediaMIMETypes();
     WEBCORE_EXPORT static HashSet<String, ASCIICaseInsensitiveHash>& getPDFMIMETypes();
-    static HashSet<String, ASCIICaseInsensitiveHash>& getPDFAndPostScriptMIMETypes();
     WEBCORE_EXPORT static HashSet<String, ASCIICaseInsensitiveHash>& getUnsupportedTextMIMETypes();
 
     // FIXME: WebKit coding style says we should not have the word "get" in the name of this function.