Attachment 372820 Details for Bug 198960 – Patch

[patch] Patch

bug-198960-20190625092606.patch (text/plain), 13.44 KB, created by Rob Buis on 2019-06-25 00:26:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Rob Buis

Created: 2019-06-25 00:26:07 PDT

Size: 13.44 KB

patch

obsolete

>Subversion Revision: 246781
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e54e326efc2bf11e128d7381707896b884fb7179..88f7a73d9b3b0a614dccee1fe651bd4e6439bad4 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-06-25  Rob Buis  <rbuis@igalia.com>
>+
>+        Introduce LinkLoadParameters
>+        https://bugs.webkit.org/show_bug.cgi?id=198960
>+
>+        Reviewed by FrÃ©dÃ©ric Wang.
>+
>+        Avoid the long parameters lists in LinkLoader by moving them
>+        to LinkLoadParameters.
>+
>+        No new tests because there is no behavior change.
>+
>+        * html/HTMLLinkElement.cpp:
>+        (WebCore::HTMLLinkElement::process):
>+        * loader/LinkLoader.cpp:
>+        (WebCore::LinkLoader::loadLinksFromHeader):
>+        (WebCore::LinkLoader::preconnectIfNeeded):
>+        (WebCore::LinkLoader::preloadIfNeeded):
>+        (WebCore::LinkLoader::prefetchIfNeeded):
>+        (WebCore::LinkLoader::loadLink):
>+        * loader/LinkLoader.h:
>+
> 2019-06-24  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Text autosizing] [iPadOS] Revise our heuristics to determine idempotent text autosizing candidates
>diff --git a/Source/WebCore/html/HTMLLinkElement.cpp b/Source/WebCore/html/HTMLLinkElement.cpp
>index 63e0f89015ff26965f749a0279c8accdc5f00667..c2c00a26288ba4c14e75b032be3838c70ebbc5c3 100644
>--- a/Source/WebCore/html/HTMLLinkElement.cpp
>+++ b/Source/WebCore/html/HTMLLinkElement.cpp
>@@ -262,7 +262,18 @@ void HTMLLinkElement::process()
> 
>     URL url = getNonEmptyURLAttribute(hrefAttr);
> 
>-    if (!m_linkLoader.loadLink(m_relAttribute, url, attributeWithoutSynchronization(asAttr), attributeWithoutSynchronization(mediaAttr), attributeWithoutSynchronization(typeAttr), attributeWithoutSynchronization(crossoriginAttr), attributeWithoutSynchronization(imagesrcsetAttr),  attributeWithoutSynchronization(imagesizesAttr), document()))
>+    LinkLoadParameters params {
>+        m_relAttribute,
>+        url,
>+        attributeWithoutSynchronization(asAttr),
>+        attributeWithoutSynchronization(mediaAttr),
>+        attributeWithoutSynchronization(typeAttr),
>+        attributeWithoutSynchronization(crossoriginAttr),
>+        attributeWithoutSynchronization(imagesrcsetAttr),
>+        attributeWithoutSynchronization(imagesizesAttr)
>+    };
>+
>+    if (!m_linkLoader.loadLink(params, document()))
>         return;
> 
>     bool treatAsStyleSheet = m_relAttribute.isStyleSheet
>diff --git a/Source/WebCore/loader/LinkLoader.cpp b/Source/WebCore/loader/LinkLoader.cpp
>index acb36c20c278d52bd5deb2af197948869e86cc5c..d638668e45c08809790a8881e10de605dcc2f881 100644
>--- a/Source/WebCore/loader/LinkLoader.cpp
>+++ b/Source/WebCore/loader/LinkLoader.cpp
>@@ -110,8 +110,10 @@ void LinkLoader::loadLinksFromHeader(const String& headerValue, const URL& baseU
>         // Sanity check to avoid re-entrancy here.
>         if (equalIgnoringFragmentIdentifier(url, baseURL))
>             continue;
>-        preconnectIfNeeded(relAttribute, url, document, header.crossOrigin());
>-        preloadIfNeeded(relAttribute, url, document, header.as(), header.media(), header.mimeType(), header.crossOrigin(), header.imageSrcSet(), header.imageSizes(), nullptr);
>+
>+        LinkLoadParameters params { relAttribute, url, header.as(), header.media(), header.mimeType(), header.crossOrigin(), header.imageSrcSet(), header.imageSizes() };
>+        preconnectIfNeeded(params, document);
>+        preloadIfNeeded(params, document, nullptr);
>     }
> }
> 
>@@ -210,13 +212,14 @@ bool LinkLoader::isSupportedType(CachedResource::Type resourceType, const String
>     return false;
> }
> 
>-void LinkLoader::preconnectIfNeeded(const LinkRelAttribute& relAttribute, const URL& href, Document& document, const String& crossOrigin)
>+void LinkLoader::preconnectIfNeeded(const LinkLoadParameters& params, Document& document)
> {
>-    if (!relAttribute.isLinkPreconnect || !href.isValid() || !href.protocolIsInHTTPFamily() || !document.frame())
>+    const URL href = params.href;
>+    if (!params.relAttribute.isLinkPreconnect || !href.isValid() || !params.href.protocolIsInHTTPFamily() || !document.frame())
>         return;
>     ASSERT(document.settings().linkPreconnectEnabled());
>     StoredCredentialsPolicy storageCredentialsPolicy = StoredCredentialsPolicy::Use;
>-    if (equalIgnoringASCIICase(crossOrigin, "anonymous") && document.securityOrigin().canAccess(SecurityOrigin::create(href)))
>+    if (equalIgnoringASCIICase(params.crossOrigin, "anonymous") && document.securityOrigin().canAccess(SecurityOrigin::create(href)))
>         storageCredentialsPolicy = StoredCredentialsPolicy::DoNotUse;
>     ASSERT(document.frame()->loader().networkingContext());
>     platformStrategies()->loaderStrategy()->preconnectTo(document.frame()->loader(), href, storageCredentialsPolicy, [weakDocument = makeWeakPtr(document), href](ResourceError error) {
>@@ -230,39 +233,39 @@ void LinkLoader::preconnectIfNeeded(const LinkRelAttribute& relAttribute, const
>     });
> }
> 
>-std::unique_ptr<LinkPreloadResourceClient> LinkLoader::preloadIfNeeded(const LinkRelAttribute& relAttribute, const URL& href, Document& document, const String& as, const String& media, const String& mimeType, const String& crossOriginMode, const String& imageSrcSet, const String& imageSizes, LinkLoader* loader)
>+std::unique_ptr<LinkPreloadResourceClient> LinkLoader::preloadIfNeeded(const LinkLoadParameters& params, Document& document, LinkLoader* loader)
> {
>-    if (!document.loader() || !relAttribute.isLinkPreload)
>+    if (!document.loader() || !params.relAttribute.isLinkPreload)
>         return nullptr;
> 
>     ASSERT(RuntimeEnabledFeatures::sharedFeatures().linkPreloadEnabled());
>-    auto type = LinkLoader::resourceTypeFromAsAttribute(as);
>+    auto type = LinkLoader::resourceTypeFromAsAttribute(params.as);
>     if (!type) {
>         document.addConsoleMessage(MessageSource::Other, MessageLevel::Error, "<link rel=preload> must have a valid `as` value"_s);
>         return nullptr;
>     }
>     URL url;
>-    if (RuntimeEnabledFeatures::sharedFeatures().linkPreloadResponsiveImagesEnabled() && type == CachedResource::Type::ImageResource && !imageSrcSet.isEmpty()) {
>-        auto sourceSize = SizesAttributeParser(imageSizes, document).length();
>-        auto candidate = bestFitSourceForImageAttributes(document.deviceScaleFactor(), href.string(), imageSrcSet, sourceSize);
>+    if (RuntimeEnabledFeatures::sharedFeatures().linkPreloadResponsiveImagesEnabled() && type == CachedResource::Type::ImageResource && !params.imageSrcSet.isEmpty()) {
>+        auto sourceSize = SizesAttributeParser(params.imageSizes, document).length();
>+        auto candidate = bestFitSourceForImageAttributes(document.deviceScaleFactor(), params.href.string(), params.imageSrcSet, sourceSize);
>         url = document.completeURL(URL({ }, candidate.string.toString()));
>     } else
>-        url = document.completeURL(href);
>+        url = document.completeURL(params.href);
> 
>     if (!url.isValid()) {
>-        if (imageSrcSet.isEmpty())
>+        if (params.imageSrcSet.isEmpty())
>             document.addConsoleMessage(MessageSource::Other, MessageLevel::Error, "<link rel=preload> has an invalid `href` value"_s);
>         else
>             document.addConsoleMessage(MessageSource::Other, MessageLevel::Error, "<link rel=preload> has an invalid `imagesrcset` value"_s);
>         return nullptr;
>     }
>-    if (!MediaQueryEvaluator::mediaAttributeMatches(document, media))
>+    if (!MediaQueryEvaluator::mediaAttributeMatches(document, params.media))
>         return nullptr;
>-    if (!isSupportedType(type.value(), mimeType))
>+    if (!isSupportedType(type.value(), params.mimeType))
>         return nullptr;
> 
>     auto options = CachedResourceLoader::defaultCachedResourceOptions();
>-    auto linkRequest = createPotentialAccessControlRequest(url, document, crossOriginMode, WTFMove(options));
>+    auto linkRequest = createPotentialAccessControlRequest(url, document, params.crossOrigin, WTFMove(options));
>     linkRequest.setPriority(CachedResource::defaultPriorityForResourceType(type.value()));
>     linkRequest.setInitiator("link");
>     linkRequest.setIgnoreForRequestCount(true);
>@@ -278,9 +281,9 @@ std::unique_ptr<LinkPreloadResourceClient> LinkLoader::preloadIfNeeded(const Lin
>     return nullptr;
> }
> 
>-void LinkLoader::prefetchIfNeeded(const LinkRelAttribute& relAttribute, const URL& href, Document& document)
>+void LinkLoader::prefetchIfNeeded(const LinkLoadParameters& params, Document& document)
> {
>-    if (!relAttribute.isLinkPrefetch || !href.isValid() || !document.frame() || !m_client.shouldLoadLink())
>+    if (!params.relAttribute.isLinkPrefetch || !params.href.isValid() || !document.frame() || !m_client.shouldLoadLink())
>         return;
> 
>     ASSERT(RuntimeEnabledFeatures::sharedFeatures().linkPrefetchEnabled());
>@@ -302,7 +305,7 @@ void LinkLoader::prefetchIfNeeded(const LinkRelAttribute& relAttribute, const UR
>     options.mode = FetchOptions::Mode::Navigate;
>     options.serviceWorkersMode = ServiceWorkersMode::None;
>     options.cachingPolicy = CachingPolicy::DisallowCaching;
>-    m_cachedLinkResource = document.cachedResourceLoader().requestLinkResource(type, CachedResourceRequest(ResourceRequest(document.completeURL(href)), options, priority)).value_or(nullptr);
>+    m_cachedLinkResource = document.cachedResourceLoader().requestLinkResource(type, CachedResourceRequest(ResourceRequest { document.completeURL(params.href) }, options, priority)).value_or(nullptr);
>     if (m_cachedLinkResource)
>         m_cachedLinkResource->addClient(*this);
> }
>@@ -313,26 +316,26 @@ void LinkLoader::cancelLoad()
>         m_preloadResourceClient->clear();
> }
> 
>-bool LinkLoader::loadLink(const LinkRelAttribute& relAttribute, const URL& href, const String& as, const String& media, const String& mimeType, const String& crossOrigin, const String& imageSrcSet, const String& imageSizes, Document& document)
>+bool LinkLoader::loadLink(const LinkLoadParameters& params, Document& document)
> {
>-    if (relAttribute.isDNSPrefetch) {
>+    if (params.relAttribute.isDNSPrefetch) {
>         // FIXME: The href attribute of the link element can be in "//hostname" form, and we shouldn't attempt
>         // to complete that as URL <https://bugs.webkit.org/show_bug.cgi?id=48857>.
>-        if (document.settings().dnsPrefetchingEnabled() && href.isValid() && !href.isEmpty() && document.frame())
>-            document.frame()->loader().client().prefetchDNS(href.host().toString());
>+        if (document.settings().dnsPrefetchingEnabled() && params.href.isValid() && !params.href.isEmpty() && document.frame())
>+            document.frame()->loader().client().prefetchDNS(params.href.host().toString());
>     }
> 
>-    preconnectIfNeeded(relAttribute, href, document, crossOrigin);
>+    preconnectIfNeeded(params, document);
> 
>     if (m_client.shouldLoadLink()) {
>-        auto resourceClient = preloadIfNeeded(relAttribute, href, document, as, media, mimeType, crossOrigin, imageSrcSet, imageSizes, this);
>+        auto resourceClient = preloadIfNeeded(params, document, this);
>         if (m_preloadResourceClient)
>             m_preloadResourceClient->clear();
>         if (resourceClient)
>             m_preloadResourceClient = WTFMove(resourceClient);
>     }
> 
>-    prefetchIfNeeded(relAttribute, href, document);
>+    prefetchIfNeeded(params, document);
> 
>     return true;
> }
>diff --git a/Source/WebCore/loader/LinkLoader.h b/Source/WebCore/loader/LinkLoader.h
>index c6dfb844af5c20ce34168ae3995dae4feefb267b..b36d59d00919e5999cedb1ed1409818c086884f7 100644
>--- a/Source/WebCore/loader/LinkLoader.h
>+++ b/Source/WebCore/loader/LinkLoader.h
>@@ -35,6 +35,7 @@
> #include "CachedResourceClient.h"
> #include "CachedResourceHandle.h"
> #include "LinkLoaderClient.h"
>+#include "LinkRelAttribute.h"
> 
> #include <wtf/WeakPtr.h>
> 
>@@ -43,14 +44,23 @@ namespace WebCore {
> class Document;
> class LinkPreloadResourceClient;
> 
>-struct LinkRelAttribute;
>+struct LinkLoadParameters {
>+    LinkRelAttribute relAttribute;
>+    URL href;
>+    String as;
>+    String media;
>+    String mimeType;
>+    String crossOrigin;
>+    String imageSrcSet;
>+    String imageSizes;
>+};
> 
> class LinkLoader : private CachedResourceClient, public CanMakeWeakPtr<LinkLoader> {
> public:
>     explicit LinkLoader(LinkLoaderClient&);
>     virtual ~LinkLoader();
> 
>-    bool loadLink(const LinkRelAttribute&, const URL&, const String& as, const String& media, const String& type, const String& crossOrigin, const String& imageSrcSet, const String& imageSizes, Document&);
>+    bool loadLink(const LinkLoadParameters&, Document&);
>     static Optional<CachedResource::Type> resourceTypeFromAsAttribute(const String& as);
> 
>     enum class MediaAttributeCheck { MediaAttributeEmpty, MediaAttributeNotEmpty, SkipMediaAttributeCheck };
>@@ -62,9 +72,9 @@ public:
> 
> private:
>     void notifyFinished(CachedResource&) override;
>-    static void preconnectIfNeeded(const LinkRelAttribute&, const URL& href, Document&, const String& crossOrigin);
>-    static std::unique_ptr<LinkPreloadResourceClient> preloadIfNeeded(const LinkRelAttribute&, const URL& href, Document&, const String& as, const String& media, const String& type, const String& crossOriginMode, const String& imageSrcSet, const String& imageSizes, LinkLoader*);
>-    void prefetchIfNeeded(const LinkRelAttribute&, const URL& href, Document&);
>+    static void preconnectIfNeeded(const LinkLoadParameters&, Document&);
>+    static std::unique_ptr<LinkPreloadResourceClient> preloadIfNeeded(const LinkLoadParameters&, Document&, LinkLoader*);
>+    void prefetchIfNeeded(const LinkLoadParameters&, Document&);
> 
>     LinkLoaderClient& m_client;
>     CachedResourceHandle<CachedResource> m_cachedLinkResource;

Actions: View | Formatted Diff | Diff

Attachments on bug 198960: 372335 | 372338 | 372346 | 372820