Changeset 87539 in webkit


Ignore:
Timestamp:
May 27, 2011 12:53:45 PM (13 years ago)
Author:
abarth@webkit.org
Message:

2011-05-27 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

HTMLVideoElement::currentSrc() should return a KURL
​https://bugs.webkit.org/show_bug.cgi?id=61578

I suspect we got into this mess because the author of this code didn't
know about the URL attribute in WebKit IDL, which is super useful!

Bad news: The line of code in question seems to have another bug, which
I've documented in a FIXME. Let the yak shaving continue!

  • html/HTMLMediaElement.cpp: (WebCore::urlForLogging): (WebCore::HTMLMediaElement::loadResource): (WebCore::HTMLMediaElement::isSafeToLoadURL): (WebCore::HTMLMediaElement::selectNextSourceChild): (WebCore::HTMLMediaElement::getPluginProxyParams):
  • html/HTMLMediaElement.h: (WebCore::HTMLMediaElement::currentSrc): (WebCore::HTMLMediaElement::currentURL):
  • html/canvas/CanvasRenderingContext.cpp: (WebCore::CanvasRenderingContext::checkOrigin):
  • rendering/HitTestResult.cpp: (WebCore::HitTestResult::absoluteMediaURL):
    • This complete URL call was unnecessary because currentSrc is already absolute.
Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r87538 r87539  
     12011-05-27  Adam Barth  <abarth@webkit.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        HTMLVideoElement::currentSrc() should return a KURL
     6        https://bugs.webkit.org/show_bug.cgi?id=61578
     7
     8        I suspect we got into this mess because the author of this code didn't
     9        know about the URL attribute in WebKit IDL, which is super useful!
     10
     11        Bad news: The line of code in question seems to have another bug, which
     12        I've documented in a FIXME.  Let the yak shaving continue!
     13
     14        * html/HTMLMediaElement.cpp:
     15        (WebCore::urlForLogging):
     16        (WebCore::HTMLMediaElement::loadResource):
     17        (WebCore::HTMLMediaElement::isSafeToLoadURL):
     18        (WebCore::HTMLMediaElement::selectNextSourceChild):
     19        (WebCore::HTMLMediaElement::getPluginProxyParams):
     20        * html/HTMLMediaElement.h:
     21        (WebCore::HTMLMediaElement::currentSrc):
     22        (WebCore::HTMLMediaElement::currentURL):
     23        * html/canvas/CanvasRenderingContext.cpp:
     24        (WebCore::CanvasRenderingContext::checkOrigin):
     25        * rendering/HitTestResult.cpp:
     26        (WebCore::HitTestResult::absoluteMediaURL):
     27            - This complete URL call was unnecessary because currentSrc is
     28              already absolute.
     29
    1302011-05-27  Mikhail Naganov  <mnaganov@chromium.org>
    231

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

    r87479 r87539  
    8686
    8787#if !LOG_DISABLED
    88 static String urlForLogging(const String& url)
     88static const char* urlForLogging(const KURL& url)
    8989{
    9090    static const unsigned maximumURLLengthForLogging = 128;
    9191
    92     if (url.length() < maximumURLLengthForLogging)
    93         return url;
    94     return url.substring(0, maximumURLLengthForLogging) + "...";
    95 }
    96 
    97 static const char *boolString(bool val)
     92    if (url.string().length() < maximumURLLengthForLogging)
     93        return url.string().utf8().data();
     94    return String(url.string().substring(0, maximumURLLengthForLogging) + "...").utf8().data();
     95}
     96
     97static const char* boolString(bool val)
    9898{
    9999    return val ? "true" : "false";
    … …  
    458458}
    459459
    460 String HTMLMediaElement::currentSrc() const
    461 {
    462     return m_currentSrc;
    463 }
    464 
    465460HTMLMediaElement::NetworkState HTMLMediaElement::networkState() const
    466461{
    … …  
    681676    ASSERT(isSafeToLoadURL(initialURL, Complain));
    682677
    683     LOG(Media, "HTMLMediaElement::loadResource(%s, %s)", urlForLogging(initialURL.string()).utf8().data(), contentType.raw().utf8().data());
     678    LOG(Media, "HTMLMediaElement::loadResource(%s, %s)", urlForLogging(initialURL), contentType.raw().utf8().data());
    684679
    685680    Frame* frame = document()->frame();
    … …  
    696691    m_currentSrc = url;
    697692
    698     LOG(Media, "HTMLMediaElement::loadResource - m_currentSrc -> %s", urlForLogging(m_currentSrc).utf8().data());
     693    LOG(Media, "HTMLMediaElement::loadResource - m_currentSrc -> %s", urlForLogging(m_currentSrc));
    699694
    700695    if (m_sendProgressEvents)
    … …  
    713708    updateVolume();
    714709
    715     m_player->load(m_currentSrc, contentType);
     710    m_player->load(m_currentSrc.string(), contentType);
    716711
    717712    // If there is no poster to display, allow the media engine to render video frames as soon as
    … …  
    726721{
    727722    if (!url.isValid()) {
    728         LOG(Media, "HTMLMediaElement::isSafeToLoadURL(%s) -> FALSE because url is invalid", urlForLogging(url.string()).utf8().data());
     723        LOG(Media, "HTMLMediaElement::isSafeToLoadURL(%s) -> FALSE because url is invalid", urlForLogging(url));
    729724        return false;
    730725    }
    … …  
    734729        if (actionIfInvalid == Complain)
    735730            FrameLoader::reportLocalLoadFailed(frame, url.string());
    736         LOG(Media, "HTMLMediaElement::isSafeToLoadURL(%s) -> FALSE rejected by SecurityOrigin", urlForLogging(url.string()).utf8().data());
     731        LOG(Media, "HTMLMediaElement::isSafeToLoadURL(%s) -> FALSE rejected by SecurityOrigin", urlForLogging(url));
    737732        return false;
    738733    }
    … …  
    17371732#if !LOG_DISABLED
    17381733        if (shouldLog)
    1739             LOG(Media, "HTMLMediaElement::selectNextSourceChild - 'src' is %s", urlForLogging(mediaURL).utf8().data());
     1734            LOG(Media, "HTMLMediaElement::selectNextSourceChild - 'src' is %s", urlForLogging(mediaURL));
    17401735#endif
    17411736        if (mediaURL.isEmpty())
    … …  
    17881783#if !LOG_DISABLED
    17891784    if (shouldLog)
    1790         LOG(Media, "HTMLMediaElement::selectNextSourceChild -> %p, %s", m_currentSourceNode, canUse ? urlForLogging(mediaURL.string()).utf8().data() : "");
     1785        LOG(Media, "HTMLMediaElement::selectNextSourceChild -> %p, %s", m_currentSourceNode, canUse ? urlForLogging(mediaURL) : "");
    17911786#endif
    17921787    return canUse ? mediaURL : KURL();
    … …  
    18001795    if (source->hasTagName(sourceTag)) {
    18011796        KURL url = source->getNonEmptyURLAttribute(srcAttr);
    1802         LOG(Media, "HTMLMediaElement::sourceWasAdded - 'src' is %s", urlForLogging(url).utf8().data());
     1797        LOG(Media, "HTMLMediaElement::sourceWasAdded - 'src' is %s", urlForLogging(url));
    18031798    }
    18041799#endif
    … …  
    18471842    if (source->hasTagName(sourceTag)) {
    18481843        KURL url = source->getNonEmptyURLAttribute(srcAttr);
    1849         LOG(Media, "HTMLMediaElement::sourceWillBeRemoved - 'src' is %s", urlForLogging(url).utf8().data());
     1844        LOG(Media, "HTMLMediaElement::sourceWillBeRemoved - 'src' is %s", urlForLogging(url));
    18501845    }
    18511846#endif
    … …  
    24512446        url = selectNextSourceChild(0, DoNothing);
    24522447
    2453     m_currentSrc = url.string();
     2448    m_currentSrc = url;
    24542449    if (url.isValid() && frame && frame->loader()->willLoadMediaElementURL(url)) {
    24552450        names.append("_media_element_src_");
    2456         values.append(m_currentSrc);
     2451        values.append(m_currentSrc.string());
    24572452    }
    24582453}

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

    r87125 r87539  
    8787// network state
    8888    void setSrc(const String&);
    89     String currentSrc() const;
     89    const KURL& currentSrc() const { return m_currentSrc; }
    9090
    9191    enum NetworkState { NETWORK_EMPTY, NETWORK_IDLE, NETWORK_LOADING, NETWORK_NO_SOURCE };
    … …  
    348348    ReadyState m_readyState;
    349349    ReadyState m_readyStateMaximum;
    350     String m_currentSrc;
    351    
     350    KURL m_currentSrc;
     351
    352352    RefPtr<MediaError> m_error;
    353353

  • trunk/Source/WebCore/html/HTMLMediaElement.idl

    r79301 r87539  
    3232    // network state
    3333    attribute [Reflect, URL] DOMString src;
    34     readonly attribute DOMString currentSrc;
     34    readonly attribute [URL] DOMString currentSrc;
    3535   
    3636    const unsigned short NETWORK_EMPTY = 0;

  • trunk/Source/WebCore/html/canvas/CanvasRenderingContext.cpp

    r87473 r87539  
    7070{
    7171#if ENABLE(VIDEO)
    72     // FIXME: HTMLVideoElement::currentSrc() should return a KURL.
    73     // https://bugs.webkit.org/show_bug.cgi?id=61578
    74     checkOrigin(KURL(ParsedURLString, video->currentSrc()));
     72    // FIXME: This check is likely wrong when a redirect is involved. We need
     73    // to test the finalURL. Please be careful when fixing this issue not to
     74    // make currentSrc be the final URL because then the
     75    // HTMLMediaElement.currentSrc DOM API would leak redirect destinations!
     76    checkOrigin(video->currentSrc());
    7577    if (canvas()->originClean() && video && !video->hasSingleSecurityOrigin())
    7678        canvas()->setOriginTainted();

  • trunk/Source/WebCore/rendering/HitTestResult.cpp

    r87018 r87539  
    313313#if ENABLE(VIDEO)
    314314    if (HTMLMediaElement* mediaElt = mediaElement())
    315         return m_innerNonSharedNode->document()->completeURL(stripLeadingAndTrailingHTMLSpaces(mediaElt->currentSrc()));
     315        return mediaElt->currentSrc();
    316316    return KURL();
    317317#else
Note: See TracChangeset for help on using the changeset viewer.