Title: [119940] trunk
Revision
119940
Author
jer.no...@apple.com
Date
2012-06-10 11:18:44 -0700 (Sun, 10 Jun 2012)

Log Message

REGRESSION: Setting invalid media "src" does not cause "error" event
https://bugs.webkit.org/show_bug.cgi?id=88423

Reviewed by Eric Carlson.

Source/WebCore:

Test: http/tests/media/video-src-invalid-error.html

Two problems here.  When the loadTimer is scheduled, the m_pendingLoadFlags
variable is set to specify what type of load is scheduled.  It is cleared
when the loadTimer fires. But, when prepareForLoad() stops the loadTimer,
it does not clear the m_pendingLoadFlags variable, so the next time
scheduleLoad() is called (when the src is changed to an invalid URL)
prepareForLoad() is not called again.

Second problem: Due to a bug in QTKit (<rdar://problem/11606415>), the
QTMovieLoadStateChangedNotification is never fired for an invalid http
URL if QTMovieOpenAsyncRequiredAttribute:YES is not passed when creating
the QTMovie.

Add a new utility method which both stops the m_loadTimer and clears the
m_pendingLoadFlags, and use it in all the places where m_loadTimer was
stopped explicitly:

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::prepareForLoad):
(WebCore::HTMLMediaElement::stopLoadTimer): Added
(WebCore::HTMLMediaElement::userCancelledLoad):
* html/HTMLMediaElement.h:

And pass QTMovieOpenAsyncRequiredAttribute:YES when creating the QTMovie:

* platform/graphics/mac/MediaPlayerPrivateQTKit.mm:
(WebCore::MediaPlayerPrivateQTKit::commonMovieAttributes):

LayoutTests:

* http/tests/media/video-src-invalid-error-expected.txt: Added.
* http/tests/media/video-src-invalid-error.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (119939 => 119940)


--- trunk/LayoutTests/ChangeLog	2012-06-10 18:14:40 UTC (rev 119939)
+++ trunk/LayoutTests/ChangeLog	2012-06-10 18:18:44 UTC (rev 119940)
@@ -1,3 +1,13 @@
+2012-06-06  Jer Noble  <jer.no...@apple.com>
+
+        REGRESSION: Setting invalid media "src" does not cause "error" event
+        https://bugs.webkit.org/show_bug.cgi?id=88423
+
+        Reviewed by Eric Carlson.
+
+        * http/tests/media/video-src-invalid-error-expected.txt: Added.
+        * http/tests/media/video-src-invalid-error.html: Added.
+
 2012-06-10  Zan Dobersek  <zandober...@gmail.com>
 
         Unreviewed GTK gardening. Updating a few animations tests baselines,

Added: trunk/LayoutTests/http/tests/media/video-src-invalid-error-expected.txt (0 => 119940)


--- trunk/LayoutTests/http/tests/media/video-src-invalid-error-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/media/video-src-invalid-error-expected.txt	2012-06-10 18:18:44 UTC (rev 119940)
@@ -0,0 +1,8 @@
+Test that changing 'src' attribute to an invalid URL results in an error event.
+
+RUN(video.setAttribute('src', 'invalid-url'))
+
+EVENT(error)
+EXPECTED (video.error.code == '4') OK
+END OF TEST
+

Added: trunk/LayoutTests/http/tests/media/video-src-invalid-error.html (0 => 119940)


--- trunk/LayoutTests/http/tests/media/video-src-invalid-error.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/media/video-src-invalid-error.html	2012-06-10 18:18:44 UTC (rev 119940)
@@ -0,0 +1,36 @@
+<html>
+    <head>
+        <script src=""
+
+        <script>
+            function stalled()
+            {
+                failTest("stalled but should not");
+                consoleWrite("");
+            }
+
+            function errorEvent()
+            {
+                testExpected("video.error.code", MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
+                endTest();
+            }
+
+            function setup()
+            {
+                findMediaElement();
+                waitForEvent('stalled', stalled);
+                waitForEvent('error', errorEvent);
+                run("video.setAttribute('src', 'invalid-url')");
+                consoleWrite("");
+            }
+
+        </script>
+    </head>
+<body _onload_="setup()">
+
+    <video controls></video>
+
+    <p>Test that changing 'src' attribute to an invalid URL results in an error event.</p>
+
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (119939 => 119940)


--- trunk/Source/WebCore/ChangeLog	2012-06-10 18:14:40 UTC (rev 119939)
+++ trunk/Source/WebCore/ChangeLog	2012-06-10 18:18:44 UTC (rev 119940)
@@ -1,5 +1,41 @@
 2012-06-06  Jer Noble  <jer.no...@apple.com>
 
+        REGRESSION: Setting invalid media "src" does not cause "error" event
+        https://bugs.webkit.org/show_bug.cgi?id=88423
+
+        Reviewed by Eric Carlson.
+
+        Test: http/tests/media/video-src-invalid-error.html
+
+        Two problems here.  When the loadTimer is scheduled, the m_pendingLoadFlags
+        variable is set to specify what type of load is scheduled.  It is cleared
+        when the loadTimer fires. But, when prepareForLoad() stops the loadTimer,
+        it does not clear the m_pendingLoadFlags variable, so the next time
+        scheduleLoad() is called (when the src is changed to an invalid URL)
+        prepareForLoad() is not called again.
+
+        Second problem: Due to a bug in QTKit (<rdar://problem/11606415>), the
+        QTMovieLoadStateChangedNotification is never fired for an invalid http
+        URL if QTMovieOpenAsyncRequiredAttribute:YES is not passed when creating
+        the QTMovie.
+
+        Add a new utility method which both stops the m_loadTimer and clears the 
+        m_pendingLoadFlags, and use it in all the places where m_loadTimer was 
+        stopped explicitly:
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::prepareForLoad):
+        (WebCore::HTMLMediaElement::stopLoadTimer): Added
+        (WebCore::HTMLMediaElement::userCancelledLoad):
+        * html/HTMLMediaElement.h:
+
+        And pass QTMovieOpenAsyncRequiredAttribute:YES when creating the QTMovie:
+
+        * platform/graphics/mac/MediaPlayerPrivateQTKit.mm:
+        (WebCore::MediaPlayerPrivateQTKit::commonMovieAttributes):
+
+2012-06-06  Jer Noble  <jer.no...@apple.com>
+
         Add logging functions to MediaPlayerPrivateQTKit.
         https://bugs.webkit.org/show_bug.cgi?id=88425
 

Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (119939 => 119940)


--- trunk/Source/WebCore/html/HTMLMediaElement.cpp	2012-06-10 18:14:40 UTC (rev 119939)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp	2012-06-10 18:18:44 UTC (rev 119940)
@@ -665,7 +665,7 @@
 
     // Perform the cleanup required for the resource load algorithm to run.
     stopPeriodicTimers();
-    m_loadTimer.stop();
+    stopLoadTimer(MediaResource);
     m_sentEndEvent = false;
     m_sentStalledEvent = false;
     m_haveFiredLoadedData = false;
@@ -3715,6 +3715,14 @@
     m_playbackProgressTimer.stop();
 }
 
+void HTMLMediaElement::stopLoadTimer(PendingLoadFlags flags)
+{
+    m_pendingLoadFlags &= ~flags;
+
+    if (!m_pendingLoadFlags)
+        m_loadTimer.stop();
+}
+
 void HTMLMediaElement::userCancelledLoad()
 {
     LOG(Media, "HTMLMediaElement::userCancelledLoad");
@@ -3729,9 +3737,8 @@
     m_player.clear();
 #endif
     stopPeriodicTimers();
-    m_loadTimer.stop();
+    stopLoadTimer(MediaResource | TextTrackResource);
     m_loadState = WaitingForSource;
-    m_pendingLoadFlags = 0;
 
     // 2 - Set the error attribute to a new MediaError object whose code attribute is set to MEDIA_ERR_ABORTED.
     m_error = MediaError::create(MediaError::MEDIA_ERR_ABORTED);

Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (119939 => 119940)


--- trunk/Source/WebCore/html/HTMLMediaElement.h	2012-06-10 18:14:40 UTC (rev 119939)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h	2012-06-10 18:18:44 UTC (rev 119940)
@@ -439,6 +439,9 @@
     void startProgressEventTimer();
     void stopPeriodicTimers();
 
+    typedef unsigned PendingLoadFlags;
+    void stopLoadTimer(PendingLoadFlags);
+
     void seek(float time, ExceptionCode&);
     void finishSeek();
     void checkIfSeekNeeded();
@@ -600,7 +603,6 @@
     double m_fragmentStartTime;
     double m_fragmentEndTime;
 
-    typedef unsigned PendingLoadFlags;
     PendingLoadFlags m_pendingLoadFlags;
 
     bool m_playing : 1;

Modified: trunk/Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm (119939 => 119940)


--- trunk/Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm	2012-06-10 18:14:40 UTC (rev 119939)
+++ trunk/Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm	2012-06-10 18:18:44 UTC (rev 119940)
@@ -97,6 +97,7 @@
 SOFT_LINK_POINTER(QTKit, QTMovieApertureModeClean, NSString *)
 SOFT_LINK_POINTER(QTKit, QTMovieApertureModeAttribute, NSString *)
 
+SOFT_LINK_POINTER_OPTIONAL(QTKit, QTMovieOpenAsyncRequiredAttribute, NSString *)
 SOFT_LINK_POINTER_OPTIONAL(QTKit, QTSecurityPolicyNoLocalToRemoteSiteAttribute, NSString *)
 SOFT_LINK_POINTER_OPTIONAL(QTKit, QTSecurityPolicyNoRemoteToLocalSiteAttribute, NSString *)
 
@@ -122,6 +123,7 @@
 #define QTMovieLoadStateAttribute getQTMovieLoadStateAttribute()
 #define QTMovieLoadStateDidChangeNotification getQTMovieLoadStateDidChangeNotification()
 #define QTMovieNaturalSizeAttribute getQTMovieNaturalSizeAttribute()
+#define QTMovieOpenAsyncRequiredAttribute getQTMovieOpenAsyncRequiredAttribute()
 #define QTMovieCurrentSizeAttribute getQTMovieCurrentSizeAttribute()
 #define QTMoviePreventExternalURLLinksAttribute getQTMoviePreventExternalURLLinksAttribute()
 #define QTMovieRateChangesPreservePitchAttribute getQTMovieRateChangesPreservePitchAttribute()
@@ -254,6 +256,9 @@
     } else
         [movieAttributes setValue:[NSNumber numberWithBool:YES] forKey:QTSecurityPolicyNoCrossSiteAttribute];
 
+    if (QTMovieOpenAsyncRequiredAttribute)
+        [movieAttributes setValue:[NSNumber numberWithBool:YES] forKey:QTMovieOpenAsyncRequiredAttribute];
+
     if (m_preload < MediaPlayer::Auto)
         [movieAttributes setValue:[NSNumber numberWithBool:YES] forKey:@"QTMovieLimitReadAheadAttribute"];
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to