Summary: | [GStreamer] playbin uri getter is invalid | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Philippe Normand <pnormand> | ||||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | eric.carlson, feature-media-reviews, gustavo, jer.noble, menard, mrobinson, pnormand, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Philippe Normand
2013-04-03 09:49:13 PDT
Well, we store url in a member variable. let's just use it instead of trying to get the gobject property. That way no ifdef needed. Created attachment 196380 [details]
Patch
Comment on attachment 196380 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196380&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1299 > - RefPtr<SecurityOrigin> securityOrigin = SecurityOrigin::create(currentUrl); > + RefPtr<SecurityOrigin> securityOrigin = SecurityOrigin::create(m_url); > if (securityOrigin->canRequest(newUrl)) { > LOG_MEDIA_MESSAGE("New media url: %s", newUrl.string().utf8().data()); > Does this eventually call ::load(...) again? If not, I can see this being an issue if there is a chain of redirects like: http://hosta/file --> http://hostb/file --> file2. Hum, no ::load() is called only once per MediaPlayerPrivate instance :) (In reply to comment #4) > Hum, no ::load() is called only once per MediaPlayerPrivate instance :) Perhaps you can simply update m_url for each redirect then? (In reply to comment #5) > (In reply to comment #4) > > Hum, no ::load() is called only once per MediaPlayerPrivate instance :) > > Perhaps you can simply update m_url for each redirect then? Ah right that's a thing I forgot to do in this patch. Good catch! Created attachment 196382 [details]
Patch
Comment on attachment 196382 [details]
Patch
Would be nice to have a test, but perhaps it is difficult to simulate this kind of thing without a quicktime server.
Comment on attachment 196382 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196382&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1294 > + newUrl = KURL(KURL(), m_url.baseAsString() + newLocation); Ah! Just noticed this: You should consider trying newUrl = KURL(m_url, newLocation); The first argument is a base. :) It should be possible to have an http/tests/media test for this I think. I only need to find how to craft a .mov file embedding a redirect url :) Comment on attachment 196382 [details]
Patch
I will provide a test
Created attachment 197022 [details]
Patch
Created attachment 197066 [details]
Patch
Comment on attachment 197066 [details]
Patch
Okay. Looks good. My only nit is that reftest.mov does not have a very descriptive name. Do you think you can rename it to something like redirect-to-counting-captioned.mov?
Committed r148079: <http://trac.webkit.org/changeset/148079> |