LinkLoader methods have too many parameters.
Created attachment 372335 [details] Patch
Created attachment 372338 [details] Patch
Comment on attachment 372338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372338&action=review > Source/WebCore/html/HTMLLinkElement.cpp:265 > + const LinkLoadParameters params( No need for const. Use uniform initializer syntax. > Source/WebCore/loader/LinkLoader.cpp:64 > +LinkLoadParameters::LinkLoadParameters(const LinkRelAttribute& relAttribute, const URL& href, const String& as, const String& media, const String& mimeType, const String& crossOrigin, const String& imageSrcSet, const String& imageSizes) Remove. > Source/WebCore/loader/LinkLoader.cpp:126 > + const LinkLoadParameters params(relAttribute, url, header.as(), header.media(), header.mimeType(), header.crossOrigin(), header.imageSrcSet(), header.imageSizes()); Ditto. > Source/WebCore/loader/LinkLoader.cpp:320 > + m_cachedLinkResource = document.cachedResourceLoader().requestLinkResource(type, CachedResourceRequest(ResourceRequest(document.completeURL(params.href)), options, priority)).value_or(nullptr); Use uniform initializer syntax. > Source/WebCore/loader/LinkLoader.cpp:331 > +bool LinkLoader::loadLink(const LinkLoadParameters& params, Document& document) params could be rvalue reference I think. > Source/WebCore/loader/LinkLoader.h:48 > + LinkLoadParameters(const LinkRelAttribute&, Remove. Use aggregate initialization.
Comment on attachment 372338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372338&action=review >> Source/WebCore/html/HTMLLinkElement.cpp:265 >> + const LinkLoadParameters params( > > No need for const. Use uniform initializer syntax. Done. >> Source/WebCore/loader/LinkLoader.cpp:64 >> +LinkLoadParameters::LinkLoadParameters(const LinkRelAttribute& relAttribute, const URL& href, const String& as, const String& media, const String& mimeType, const String& crossOrigin, const String& imageSrcSet, const String& imageSizes) > > Remove. Done. >> Source/WebCore/loader/LinkLoader.cpp:126 >> + const LinkLoadParameters params(relAttribute, url, header.as(), header.media(), header.mimeType(), header.crossOrigin(), header.imageSrcSet(), header.imageSizes()); > > Ditto. Done. >> Source/WebCore/loader/LinkLoader.cpp:320 >> + m_cachedLinkResource = document.cachedResourceLoader().requestLinkResource(type, CachedResourceRequest(ResourceRequest(document.completeURL(params.href)), options, priority)).value_or(nullptr); > > Use uniform initializer syntax. I assume you meant ResourceRequest here. >> Source/WebCore/loader/LinkLoader.cpp:331 >> +bool LinkLoader::loadLink(const LinkLoadParameters& params, Document& document) > > params could be rvalue reference I think. Since. we don't store the params in LinkLoader but just pass it around, I don't think that is needed. >> Source/WebCore/loader/LinkLoader.h:48 >> + LinkLoadParameters(const LinkRelAttribute&, > > Remove. Use aggregate initialization. Done.
Created attachment 372346 [details] Patch
Comment on attachment 372346 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372346&action=review > Source/WebCore/html/HTMLLinkElement.cpp:273 > + attributeWithoutSynchronization(imagesizesAttr) }; I would put the closing "};" on a new line. Not sure if check-webkit-style likes it or not... > Source/WebCore/loader/LinkLoader.cpp:218 > + if (!params.relAttribute.isLinkPreconnect || !href.isValid() || !params.href.protocolIsInHTTPFamily() || !document.frame()) I see you are using href.isValid(), params.href.protocolIsInHTTPFamily() and SecurityOrigin::create(href) Is introducing this local href variable actually needed?
Created attachment 372820 [details] Patch
Comment on attachment 372346 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372346&action=review >> Source/WebCore/html/HTMLLinkElement.cpp:273 >> + attributeWithoutSynchronization(imagesizesAttr) }; > > I would put the closing "};" on a new line. Not sure if check-webkit-style likes it or not... Done. >> Source/WebCore/loader/LinkLoader.cpp:218 >> + if (!params.relAttribute.isLinkPreconnect || !href.isValid() || !params.href.protocolIsInHTTPFamily() || !document.frame()) > > I see you are using href.isValid(), params.href.protocolIsInHTTPFamily() and SecurityOrigin::create(href) > > Is introducing this local href variable actually needed? It is not needed, but I think it makes things a bit easier to read, and it matches the old situation.
Comment on attachment 372820 [details] Patch Clearing flags on attachment: 372820 Committed r246786: <https://trac.webkit.org/changeset/246786>
All reviewed patches have been landed. Closing bug.
<rdar://problem/52100030>