Attachment 359989 Details for Bug 193756 – Patch

[patch] Patch

bug-193756-20190123200043.patch (text/plain), 6.94 KB, created by Darin Adler on 2019-01-23 20:00:44 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Darin Adler

Created: 2019-01-23 20:00:44 PST

Size: 6.94 KB

patch

obsolete

>Subversion Revision: 240403
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 366e4bbfe4da0582be83234a9159bb81eba0a393..2df6621f3882b60032e6c545eeb61e9ab1888677 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-01-23  Darin Adler  <darin@apple.com>
>+
>+        Simplify and streamline code that creates an appropriate document based on MIME type
>+        https://bugs.webkit.org/show_bug.cgi?id=193756
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * dom/DOMImplementation.cpp:
>+        (WebCore::DOMImplementation::createDocument): Use equalLettersIgnoringASCIICase rather
>+        than == for all the MIME type checks. Use MIMETypeRegistry::isSupportedImageMIMEType
>+        instead of Image::supportsType. Rearranged checks so that all the combinations that
>+        that take precedence over plug-ins are checked first, fixing some unimportant edge
>+        cases where the plug-in database is initialized and doesn't need to be. Straightened
>+        out the logic for various special types so that the checks are more independent from
>+        each other and hence easier to understand.
>+
> 2019-01-23  Benjamin Poulain  <benjamin@webkit.org>
> 
>         <rdar://problem/27686430> Revert workaround AVPlayer.setMuted bug on macOS
>diff --git a/Source/WebCore/dom/DOMImplementation.cpp b/Source/WebCore/dom/DOMImplementation.cpp
>index a9832c820fb25a71a7008a0823a1623f4550455d..5182bc4c3c6fbb5349224b8900b38a15a2074ff2 100644
>--- a/Source/WebCore/dom/DOMImplementation.cpp
>+++ b/Source/WebCore/dom/DOMImplementation.cpp
>@@ -133,65 +133,61 @@ Ref<HTMLDocument> DOMImplementation::createHTMLDocument(const String& title)
> 
> Ref<Document> DOMImplementation::createDocument(const String& type, Frame* frame, const URL& url)
> {
>-    // FIXME: Confusing to have this here with public DOM APIs for creating documents. This is different enough that it should perhaps be moved.
>-    // FIXME: This function is doing case insensitive comparisons on MIME types. Should do equalLettersIgnoringASCIICase instead.
>+    // FIXME: Inelegant to have this here just because this is the home of DOM APIs for creating documents.
>+    // This is internal, not a DOM API. Maybe we should put it in a new class called DocumentFactory,
>+    // because of the analogy with HTMLElementFactory.
> 
>-    // Plugins cannot take HTML and XHTML from us, and we don't even need to initialize the plugin database for those.
>-    if (type == "text/html")
>+    // Plug-ins cannot take over for HTML, XHTML, plain text, or non-PDF images.
>+    if (equalLettersIgnoringASCIICase(type, "text/html"))
>         return HTMLDocument::create(frame, url);
>-    if (type == "application/xhtml+xml")
>+    if (equalLettersIgnoringASCIICase(type, "application/xhtml+xml"))
>         return XMLDocument::createXHTML(frame, url);
>-
>-#if ENABLE(FTPDIR)
>-    // Plugins cannot take FTP from us either
>-    if (type == "application/x-ftp-directory")
>-        return FTPDirectoryDocument::create(frame, url);
>-#endif
>-
>-    // If we want to useImageDocumentForSubframePDF, we'll let that override plugin support.
>-    if (frame && !frame->isMainFrame() && MIMETypeRegistry::isPDFMIMEType(type) && frame->settings().useImageDocumentForSubframePDF())
>+    if (equalLettersIgnoringASCIICase(type, "text/plain"))
>+        return TextDocument::create(frame, url);
>+    if (MIMETypeRegistry::isSupportedImageMIMEType(type) && !MIMETypeRegistry::isPDFOrPostScriptMIMEType(type)) {
>+        // FIXME: What guarantees that frame is non-null here?
>         return ImageDocument::create(*frame, url);
>-
>-    PluginData* pluginData = nullptr;
>-    auto allowedPluginTypes = PluginData::OnlyApplicationPlugins;
>-    if (frame && frame->page()) {
>-        if (frame->loader().subframeLoader().allowPlugins())
>-            allowedPluginTypes = PluginData::AllPlugins;
>-
>-        pluginData = &frame->page()->pluginData();
>     }
> 
>-    // PDF is one image type for which a plugin can override built-in support.
>-    // We do not want QuickTime to take over all image types, obviously.
>-    if (MIMETypeRegistry::isPDFOrPostScriptMIMEType(type) && pluginData && pluginData->supportsWebVisibleMimeType(type, allowedPluginTypes))
>-        return PluginDocument::create(frame, url);
>-    if (Image::supportsType(type))
>+    // Use of image documents for subframe PDFs will override a PDF plug-in.
>+    if (frame && !frame->isMainFrame() && MIMETypeRegistry::isPDFMIMEType(type) && frame->settings().useImageDocumentForSubframePDF())
>         return ImageDocument::create(*frame, url);
> 
> #if ENABLE(VIDEO)
>-    // Check to see if the type can be played by our MediaPlayer, if so create a MediaDocument
>-    // Key system is not applicable here.
>+    // Check to see if the type can be played by our MediaPlayer, if so create a MediaDocument.
>+    // (Key system is not applicable here.)
>     MediaEngineSupportParameters parameters;
>-    parameters.type = ContentType(type);
>+    parameters.type = ContentType { type };
>     parameters.url = url;
>     if (MediaPlayer::supportsType(parameters))
>         return MediaDocument::create(frame, url);
> #endif
> 
>-    // Everything else except text/plain can be overridden by plugins. In particular, Adobe SVG Viewer should be used for SVG, if installed.
>-    // Disallowing plug-ins to use text/plain prevents plug-ins from hijacking a fundamental type that the browser is expected to handle,
>-    // and also serves as an optimization to prevent loading the plug-in database in the common case.
>-    if (type != "text/plain" && ((pluginData && pluginData->supportsWebVisibleMimeType(type, allowedPluginTypes)) || (frame && frame->loader().client().shouldAlwaysUsePluginDocument(type))))
>+#if ENABLE(FTPDIR)
>+    // There's no need to support plug-ins taking over for FTP directories either.
>+    if (equalLettersIgnoringASCIICase(type, "application/x-ftp-directory"))
>+        return FTPDirectoryDocument::create(frame, url);
>+#endif
>+
>+    if (frame && frame->loader().client().shouldAlwaysUsePluginDocument(type))
>         return PluginDocument::create(frame, url);
>+
>+    // The following is the relatively costly lookup that requires initializing the plug-in database.
>+    if (frame && frame->page()) {
>+        auto allowedPluginTypes = frame->loader().subframeLoader().allowPlugins()
>+            ? PluginData::AllPlugins : PluginData::OnlyApplicationPlugins;
>+        if (frame->page()->pluginData().supportsWebVisibleMimeType(type, allowedPluginTypes))
>+            return PluginDocument::create(frame, url);
>+    }
>+
>+    // Everything here after the plug-in check can be overridden by plug-ins.
>+    // For example, plug-ins can take over support for SVG or PDF.
>     if (MIMETypeRegistry::isTextMIMEType(type))
>         return TextDocument::create(frame, url);
>-
>-    if (type == "image/svg+xml")
>+    if (equalLettersIgnoringASCIICase(type, "image/svg+xml"))
>         return SVGDocument::create(frame, url);
>-
>     if (MIMETypeRegistry::isXMLMIMEType(type))
>         return XMLDocument::create(frame, url);
>-
>     return HTMLDocument::create(frame, url);
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193756: 359989 | 360583