Changeset 261968 in webkit


Ignore:
Timestamp:
May 20, 2020 3:59:51 PM (4 years ago)
Author:
commit-queue@webkit.org
Message:

Remove implicit URL->String conversion operators
​https://bugs.webkit.org/show_bug.cgi?id=211033

Patch by Alex Christensen <​achristensen@webkit.org> on 2020-05-20
Reviewed by Darin Adler.

Source/WebCore:

  • accessibility/AccessibilityRenderObject.cpp:

(WebCore::AccessibilityRenderObject::stringValueForMSAA const):

  • html/DOMURL.cpp:

(WebCore::DOMURL::create):

  • html/HTMLPlugInElement.cpp:

(WebCore::pluginReplacementForType):

  • html/URLUtils.h:

(WebCore::URLUtils<T>::protocol const):
(WebCore::URLUtils<T>::setUsername):
(WebCore::URLUtils<T>::setPassword):

  • page/Location.cpp:

(WebCore::Location::setProtocol):
(WebCore::Location::setHost):
(WebCore::Location::setHostname):
(WebCore::Location::setPort):
(WebCore::Location::setPathname):
(WebCore::Location::setSearch):
(WebCore::Location::setHash):

  • platform/graphics/MediaPlayer.cpp:

(WebCore::MediaPlayer::load):

Source/WebKitLegacy/mac:

  • DOM/DOMHTMLBaseElement.mm:

(-[DOMHTMLBaseElement href]):

Source/WTF:

These operators have been the cause of many subtle bugs related to type inference that are hard to see in the code,
as well as performance bugs where we unnecessarily re-parse parsed URLs.
After my recent cleanup this was easier than I thought it would be.

  • wtf/URL.h:

(WTF::URL::operator const String& const): Deleted.
(WTF::URL::operator StringView const): Deleted.
(WTF::URL::operator NSString * const): Deleted.

Location:
trunk/Source
Files:
29 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r261926 r261968  
     12020-05-20  Alex Christensen  <achristensen@webkit.org>
     2
     3        Remove implicit URL->String conversion operators
     4        https://bugs.webkit.org/show_bug.cgi?id=211033
     5
     6        Reviewed by Darin Adler.
     7
     8        These operators have been the cause of many subtle bugs related to type inference that are hard to see in the code,
     9        as well as performance bugs where we unnecessarily re-parse parsed URLs.
     10        After my recent cleanup this was easier than I thought it would be.
     11
     12        * wtf/URL.h:
     13        (WTF::URL::operator const String& const): Deleted.
     14        (WTF::URL::operator StringView const): Deleted.
     15        (WTF::URL::operator NSString * const): Deleted.
     16
    1172020-05-20  Antoine Quint  <graouts@apple.com>
    218

  • trunk/Source/WTF/wtf/URL.h

    r260709 r261968  
    167167    unsigned pathAfterLastSlash() const;
    168168
    169     operator const String&() const { return m_string; }
    170     operator StringView() const { return m_string; }
    171 
    172169#if USE(CF)
    173170    WTF_EXPORT_PRIVATE URL(CFURLRef);
    … …  
    178175    WTF_EXPORT_PRIVATE URL(NSURL *);
    179176    WTF_EXPORT_PRIVATE operator NSURL *() const;
    180 #endif
    181 
    182 #ifdef __OBJC__
    183     operator NSString *() const { return m_string; }
    184177#endif
    185178

  • trunk/Source/WebCore/ChangeLog

    r261967 r261968  
     12020-05-20  Alex Christensen  <achristensen@webkit.org>
     2
     3        Remove implicit URL->String conversion operators
     4        https://bugs.webkit.org/show_bug.cgi?id=211033
     5
     6        Reviewed by Darin Adler.
     7
     8        * accessibility/AccessibilityRenderObject.cpp:
     9        (WebCore::AccessibilityRenderObject::stringValueForMSAA const):
     10        * html/DOMURL.cpp:
     11        (WebCore::DOMURL::create):
     12        * html/HTMLPlugInElement.cpp:
     13        (WebCore::pluginReplacementForType):
     14        * html/URLUtils.h:
     15        (WebCore::URLUtils<T>::protocol const):
     16        (WebCore::URLUtils<T>::setUsername):
     17        (WebCore::URLUtils<T>::setPassword):
     18        * page/Location.cpp:
     19        (WebCore::Location::setProtocol):
     20        (WebCore::Location::setHost):
     21        (WebCore::Location::setHostname):
     22        (WebCore::Location::setPort):
     23        (WebCore::Location::setPathname):
     24        (WebCore::Location::setSearch):
     25        (WebCore::Location::setHash):
     26        * platform/graphics/MediaPlayer.cpp:
     27        (WebCore::MediaPlayer::load):
     28
    1292020-05-20  Sam Weinig  <weinig@apple.com>
    230

  • trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

    r261776 r261968  
    36653665        Element* anchor = anchorElement();
    36663666        if (is<HTMLAnchorElement>(anchor))
    3667             return downcast<HTMLAnchorElement>(*anchor).href();
     3667            return downcast<HTMLAnchorElement>(*anchor).href().string();
    36683668    }
    36693669

  • trunk/Source/WebCore/html/DOMURL.cpp

    r258869 r261968  
    4040namespace WebCore {
    4141
    42 inline DOMURL::DOMURL(URL&& completeURL, URL&& baseURL)
    43     : m_baseURL(WTFMove(baseURL))
     42inline DOMURL::DOMURL(URL&& completeURL, const URL& baseURL)
     43    : m_baseURL(baseURL)
    4444    , m_url(WTFMove(completeURL))
    4545{
    4646}
    4747
     48ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const URL& base)
     49{
     50    if (!base.isValid())
     51        return Exception { TypeError };
     52    URL completeURL { base, url };
     53    if (!completeURL.isValid())
     54        return Exception { TypeError };
     55    return adoptRef(*new DOMURL(WTFMove(completeURL), base));
     56}
     57
    4858ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const String& base)
    4959{
    50     URL baseURL { URL { }, base };
    51     if (!baseURL.isValid())
    52         return Exception { TypeError };
    53     URL completeURL { baseURL, url };
    54     if (!completeURL.isValid())
    55         return Exception { TypeError };
    56     return adoptRef(*new DOMURL(WTFMove(completeURL), WTFMove(baseURL)));
     60    return create(url, URL { URL { }, base });
    5761}
    5862

  • trunk/Source/WebCore/html/DOMURL.h

    r260724 r261968  
    4242    static ExceptionOr<Ref<DOMURL>> create(const String& url, const String& base);
    4343    static ExceptionOr<Ref<DOMURL>> create(const String& url, const DOMURL& base);
     44    static ExceptionOr<Ref<DOMURL>> create(const String& url, const URL& base);
    4445    static ExceptionOr<Ref<DOMURL>> create(const String& url);
    4546    ~DOMURL();
    … …  
    5960
    6061private:
    61     DOMURL(URL&& completeURL, URL&& baseURL);
     62    DOMURL(URL&& completeURL, const URL& baseURL);
    6263
    6364    URL fullURL() const final { return m_url; }

  • trunk/Source/WebCore/html/HTMLPlugInElement.cpp

    r260707 r261968  
    365365    String type = mimeType;
    366366    if (type.isEmpty() && url.protocolIsData())
    367         type = mimeTypeFromDataURL(url);
     367        type = mimeTypeFromDataURL(url.string());
    368368   
    369369    if (type.isEmpty() && !extension.isEmpty()) {

  • trunk/Source/WebCore/page/Location.cpp

    r260709 r261968  
    137137    if (!url.setProtocol(protocol))
    138138        return Exception { SyntaxError };
    139     return setLocation(activeWindow, firstWindow, url);
     139    return setLocation(activeWindow, firstWindow, url.string());
    140140}
    141141
    … …  
    147147    URL url = frame->document()->url();
    148148    url.setHostAndPort(host);
    149     return setLocation(activeWindow, firstWindow, url);
     149    return setLocation(activeWindow, firstWindow, url.string());
    150150}
    151151
    … …  
    157157    URL url = frame->document()->url();
    158158    url.setHost(hostname);
    159     return setLocation(activeWindow, firstWindow, url);
     159    return setLocation(activeWindow, firstWindow, url.string());
    160160}
    161161
    … …  
    167167    URL url = frame->document()->url();
    168168    url.setPort(parseUInt16(portString));
    169     return setLocation(activeWindow, firstWindow, url);
     169    return setLocation(activeWindow, firstWindow, url.string());
    170170}
    171171
    … …  
    177177    URL url = frame->document()->url();
    178178    url.setPath(pathname);
    179     return setLocation(activeWindow, firstWindow, url);
     179    return setLocation(activeWindow, firstWindow, url.string());
    180180}
    181181
    … …  
    187187    URL url = frame->document()->url();
    188188    url.setQuery(search);
    189     return setLocation(activeWindow, firstWindow, url);
     189    return setLocation(activeWindow, firstWindow, url.string());
    190190}
    191191
    … …  
    207207    if (equalIgnoringNullity(oldFragmentIdentifier, url.fragmentIdentifier()))
    208208        return { };
    209     return setLocation(activeWindow, firstWindow, url);
     209    return setLocation(activeWindow, firstWindow, url.string());
    210210}
    211211

  • trunk/Source/WebCore/platform/graphics/MediaPlayer.cpp

    r261013 r261968  
    442442    if (containerType.isEmpty() || containerType == applicationOctetStream() || containerType == textPlain()) {
    443443        if (m_url.protocolIsData())
    444             m_contentType = ContentType(mimeTypeFromDataURL(m_url));
     444            m_contentType = ContentType(mimeTypeFromDataURL(m_url.string()));
    445445        else {
    446446            auto lastPathComponent = url.lastPathComponent();

  • trunk/Source/WebCore/platform/gtk/DragDataGtk.cpp

    r222025 r261968  
    7777        return String();
    7878    if (filenamePolicy != ConvertFilenames) {
    79         URL url(URL(), m_platformDragData->url());
    80         if (url.isLocalFile())
    81             return String();
     79        if (m_platformDragData->url().isLocalFile())
     80            return { };
    8281    }
    8382
    84     String url(m_platformDragData->url());
    8583    if (title)
    8684        *title = m_platformDragData->urlLabel();
    87     return url;
     85    return m_platformDragData->url().string();
    8886}
    8987

  • trunk/Source/WebCore/platform/gtk/PasteboardGtk.cpp

    r261923 r261968  
    381381        return m_selectionData->uriList();
    382382    case ClipboardDataTypeURL:
    383         return m_selectionData->url();
     383        return m_selectionData->url().string();
    384384    case ClipboardDataTypeMarkup:
    385385        return m_selectionData->markup();

  • trunk/Source/WebCore/platform/gtk/SelectionData.cpp

    r261792 r261968  
    8181    m_url = url;
    8282    if (m_uriList.isEmpty())
    83         m_uriList = url;
     83        m_uriList = url.string();
    8484
    8585    if (!hasText())
    … …  
    9191    String actualLabel(label);
    9292    if (actualLabel.isEmpty())
    93         actualLabel = url;
     93        actualLabel = url.string();
    9494
    9595    StringBuilder markup;
    … …  
    109109
    110110    if (hasURL())
    111         return url();
     111        return url().string();
    112112
    113113    return emptyString();

  • trunk/Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp

    r257078 r261968  
    127127        cacheUrl.removeFragmentIdentifier();
    128128
    129         if (CurlCacheManager::singleton().getCachedResponse(cacheUrl, m_response)) {
     129        if (CurlCacheManager::singleton().getCachedResponse(cacheUrl.string(), m_response)) {
    130130            if (d()->m_addedCacheValidationHeaders) {
    131131                m_response.setHTTPStatusCode(200);

  • trunk/Source/WebCore/platform/network/curl/NetworkStorageSessionCurl.cpp

    r257078 r261968  
    152152void NetworkStorageSession::deleteCookie(const URL& url, const String& name) const
    153153{
    154     cookieDatabase().deleteCookie(url, name);
     154    cookieDatabase().deleteCookie(url.string(), name);
    155155}
    156156

  • trunk/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp

    r260707 r261968  
    128128    cacheUrl.removeFragmentIdentifier();
    129129
    130     if (cache.isCached(cacheUrl)) {
    131         cache.addCacheEntryClient(cacheUrl, this);
    132 
    133         for (const auto& entry : cache.requestHeaders(cacheUrl))
     130    if (cache.isCached(cacheUrl.string())) {
     131        cache.addCacheEntryClient(cacheUrl.string(), this);
     132
     133        for (const auto& entry : cache.requestHeaders(cacheUrl.string()))
    134134            request.addHTTPHeaderField(entry.key, entry.value);
    135135

  • trunk/Source/WebCore/platform/network/curl/SocketStreamHandleImplCurl.cpp

    r256728 r261968  
    123123        m_client.didFailToReceiveSocketStreamData(*this);
    124124    else
    125         m_client.didFailSocketStream(*this, SocketStreamError(errorCode, m_url, CurlHandle::errorDescription(errorCode)));
     125        m_client.didFailSocketStream(*this, SocketStreamError(errorCode, m_url.string(), CurlHandle::errorDescription(errorCode)));
    126126}
    127127

  • trunk/Source/WebKit/UIProcess/Cocoa/SOAuthorization/RedirectSOAuthorizationSession.mm

    r251600 r261968  
    7979            auto html = makeString("<script>location = '", response.httpHeaderFields().get(HTTPHeaderName::Location), "'</script>").utf8();
    8080            auto data = IPC::DataReference(reinterpret_cast<const uint8_t*>(html.data()), html.length());
    81             page->loadData(data, "text/html"_s, "UTF-8"_s, navigationAction->request().url(), nullptr, navigationAction->shouldOpenExternalURLsPolicy());
     81            page->loadData(data, "text/html"_s, "UTF-8"_s, navigationAction->request().url().string(), nullptr, navigationAction->shouldOpenExternalURLsPolicy());
    8282            return;
    8383        }

  • trunk/Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm

    r261478 r261968  
    343343        return;
    344344
    345     NSURL *targetURL = [NSURL URLWithString:_positionInformation->url];
     345    NSURL *targetURL = _positionInformation->url;
    346346    NSString *urlScheme = [targetURL scheme];
    347347    BOOL isJavaScriptURL = [urlScheme length] && [urlScheme caseInsensitiveCompare:@"javascript"] == NSOrderedSame;

  • trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

    r261966 r261968  
    96589658    NSURL *targetURL = controller.previewData[UIPreviewDataLink];
    96599659    URL coreTargetURL = targetURL;
    9660     bool isValidURLForImagePreview = !coreTargetURL.isEmpty() && (WTF::protocolIsInHTTPFamily(coreTargetURL) || WTF::protocolIs(coreTargetURL, "data"));
     9660    bool isValidURLForImagePreview = !coreTargetURL.isEmpty() && (coreTargetURL.protocolIsInHTTPFamily() || coreTargetURL.protocolIs("data"));
    96619661
    96629662    if ([_previewItemController type] == UIPreviewItemTypeLink) {

  • trunk/Source/WebKit/UIProcess/ios/WKGeolocationProviderIOS.mm

    r257571 r261968  
    6969
    7070namespace WebKit {
    71 void decidePolicyForGeolocationRequestFromOrigin(WebCore::SecurityOrigin*, const String& urlString, id<WebAllowDenyPolicyListener>, UIView*);
     71void decidePolicyForGeolocationRequestFromOrigin(WebCore::SecurityOrigin*, const URL&, id<WebAllowDenyPolicyListener>, UIView*);
    7272};
    7373

  • trunk/Source/WebKit/UIProcess/ios/WKGeolocationProviderIOSObjCSecurityOrigin.mm

    r252536 r261968  
    4545namespace WebKit {
    4646
    47 void decidePolicyForGeolocationRequestFromOrigin(WebCore::SecurityOrigin*, const String& urlString, id<WebAllowDenyPolicyListener>, UIView *);
     47void decidePolicyForGeolocationRequestFromOrigin(WebCore::SecurityOrigin*, const URL&, id<WebAllowDenyPolicyListener>, UIView *);
    4848
    49 void decidePolicyForGeolocationRequestFromOrigin(WebCore::SecurityOrigin* origin, const String& urlString, id<WebAllowDenyPolicyListener> listener, UIView *view)
     49void decidePolicyForGeolocationRequestFromOrigin(WebCore::SecurityOrigin* origin, const URL& url, id<WebAllowDenyPolicyListener> listener, UIView *view)
    5050{
    5151    RetainPtr<WebSecurityOrigin> securityOrigin = adoptNS([[WebSecurityOrigin alloc] _initWithWebCoreSecurityOrigin:origin]);
    52     RetainPtr<NSURL> requestUrl = adoptNS([[NSURL alloc] initWithString:urlString]);
    5352    RetainPtr<UIWebGeolocationPolicyDecider> decider = [UIWebGeolocationPolicyDecider sharedPolicyDecider];
    5453    if ([decider respondsToSelector:@selector(decidePolicyForGeolocationRequestFromOrigin:requestingURL:view:listener:)])
    55         [decider decidePolicyForGeolocationRequestFromOrigin:securityOrigin.get() requestingURL:requestUrl.get() view:view listener:listener];
     54        [decider decidePolicyForGeolocationRequestFromOrigin:securityOrigin.get() requestingURL:url view:view listener:listener];
    5655    else
    57         [decider decidePolicyForGeolocationRequestFromOrigin:securityOrigin.get() requestingURL:requestUrl.get() window:view.window listener:listener];
     56        [decider decidePolicyForGeolocationRequestFromOrigin:securityOrigin.get() requestingURL:url window:view.window listener:listener];
    5857}
    5958

  • trunk/Source/WebKit/UIProcess/ios/WKPDFView.mm

    r261457 r261968  
    530530
    531531    NSDictionary *representations = @{
    532         (NSString *)kUTTypeUTF8PlainText : (NSString *)_positionInformation.url,
     532        (NSString *)kUTTypeUTF8PlainText : (NSString *)_positionInformation.url.string(),
    533533        (NSString *)kUTTypeURL : (NSURL *)_positionInformation.url,
    534534    };

  • trunk/Source/WebKit/WebProcess/InjectedBundle/API/glib/WebKitWebPage.cpp

    r258663 r261968  
    317317                errorMessage.append(error.localizedDescription());
    318318            }
    319             webkitWebPageDidSendConsoleMessage(m_webPage, MessageSource::Network, MessageLevel::Error, errorMessage.toString(), 0, error.failingURL());
     319            webkitWebPageDidSendConsoleMessage(m_webPage, MessageSource::Network, MessageLevel::Error, errorMessage.toString(), 0, error.failingURL().string());
    320320        }
    321321    }

  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r261966 r261968  
    26612661
    26622662    info.isImage = true;
    2663     info.imageURL = element.document().completeURL(renderImage.cachedImage()->url());
     2663    info.imageURL = element.document().completeURL(renderImage.cachedImage()->url().string());
    26642664    info.isAnimatedImage = image->isAnimated();
    26652665
    … …  
    31513151        HTMLFormElement* form = element.form();
    31523152        if (form)
    3153             information.formAction = form->getURLAttribute(WebCore::HTMLNames::actionAttr);
     3153            information.formAction = form->getURLAttribute(WebCore::HTMLNames::actionAttr).string();
    31543154        if (auto autofillElements = WebCore::AutofillElements::computeAutofillElements(element)) {
    31553155            information.acceptsAutofilledLoginCredentials = true;

  • trunk/Source/WebKitLegacy/mac/ChangeLog

    r261948 r261968  
     12020-05-20  Alex Christensen  <achristensen@webkit.org>
     2
     3        Remove implicit URL->String conversion operators
     4        https://bugs.webkit.org/show_bug.cgi?id=211033
     5
     6        Reviewed by Darin Adler.
     7
     8        * DOM/DOMHTMLBaseElement.mm:
     9        (-[DOMHTMLBaseElement href]):
     10
    1112020-05-20  Simon Fraser  <simon.fraser@apple.com>
    212

  • trunk/Source/WebKitLegacy/mac/DOM/DOMHTMLBaseElement.mm

    r247570 r261968  
    4343{
    4444    WebCore::JSMainThreadNullState state;
    45     return IMPL->href();
     45    return IMPL->href().string();
    4646}
    4747

  • trunk/Source/WebKitLegacy/win/Plugins/PluginStream.cpp

    r248846 r261968  
    144144    // Some plugins (Flash) expect that javascript URLs are passed back decoded as this is the
    145145    // format used when requesting the URL.
    146     if (protocolIsJavaScript(responseURL))
     146    if (responseURL.protocolIsJavaScript())
    147147        m_stream.url = fastStrDup(decodeURLEscapeSequences(responseURL.string()).utf8().data());
    148148    else

  • trunk/Source/WebKitLegacy/win/Plugins/PluginView.cpp

    r260317 r261968  
    104104static String scriptStringIfJavaScriptURL(const URL& url)
    105105{
    106     if (!protocolIsJavaScript(url))
     106    if (!url.protocolIsJavaScript())
    107107        return String();
    108108

  • trunk/Source/WebKitLegacy/win/WebCoreSupport/WebFrameLoaderClient.cpp

    r261586 r261968  
    702702        COMPtr<IWebURLRequest> urlRequest(AdoptCOM, WebMutableURLRequest::createInstance(loader->originalRequestCopy()));
    703703       
    704         COMPtr<IWebNavigationData> navigationData(AdoptCOM, WebNavigationData::createInstance(
    705             loader->urlForHistory(), loader->title().string, urlRequest.get(), urlResponse.get(), loader->substituteData().isValid(), loader->clientRedirectSourceForHistory()));
     704        COMPtr<IWebNavigationData> navigationData(AdoptCOM, WebNavigationData::createInstance(loader->urlForHistory().string(), loader->title().string, urlRequest.get(), urlResponse.get(), loader->substituteData().isValid(), loader->clientRedirectSourceForHistory()));
    706705
    707706        historyDelegate->didNavigateWithNavigationData(webView, navigationData.get(), m_webFrame);
Note: See TracChangeset for help on using the changeset viewer.