WebCore:

        Reviewed by Adam.

        - Fix a crash when removing a loading media element from the tree.
        - Follow the spec by invoking pause() when element is removed from the tree instead of unloading.

        Tests: http/tests/media/remove-while-loading.html
               media/remove-from-document.html

        * html/HTMLMediaElement.cpp:
        (WebCore::HTMLMediaElement::removedFromDocument):

LayoutTests:

        Reviewed by Adam.

        Test that removing a media element when it is loading does not crash.
        Test that media is paused when it is removed from the tree.

        * http/tests/media/remove-while-loading-expected.txt: Added.
        * http/tests/media/remove-while-loading.html: Added.
        * media/remove-from-document-expected.txt: Added.
        * media/remove-from-document.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@28312 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8f5c4f4e95b7fcbcf95a2498adebea991c2d23b9..7d0a59e2fc75c5e071831f1fdf236731240689c8 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2007-12-01  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Adam.
+        
+        Test that removing a media element when it is loading does not crash.
+        Test that media is paused when it is removed from the tree.
+
+        * http/tests/media/remove-while-loading-expected.txt: Added.
+        * http/tests/media/remove-while-loading.html: Added.
+        * media/remove-from-document-expected.txt: Added.
+        * media/remove-from-document.html: Added.
+
 2007-12-01  Julien Chaffraix  <julien.chaffraix@gmail.com>
 
         Bug 16189: XMLHttpRequest::setRequestHeader() should not set certain headers

diff --git a/LayoutTests/http/tests/media/remove-while-loading-expected.txt b/LayoutTests/http/tests/media/remove-while-loading-expected.txt
new file mode 100644 (file)
index 0000000..b24a1dd
--- /dev/null
+++ b/LayoutTests/http/tests/media/remove-while-loading-expected.txt
@@ -0,0 +1,6 @@
+Test that removing a media element from the tree while loading does not crash.
+
+EVENT(loadedmetadata)
+RUN(document.body.removeChild(video))
+END OF TEST
+

diff --git a/LayoutTests/http/tests/media/remove-while-loading.html b/LayoutTests/http/tests/media/remove-while-loading.html
new file mode 100644 (file)
index 0000000..c7140a9
--- /dev/null
+++ b/LayoutTests/http/tests/media/remove-while-loading.html
@@ -0,0 +1,10 @@
+<video></video>
+<p>Test that removing a media element from the tree while loading does not crash.</p>
+<script src=../../../media/video-test.js></script>
+<script>
+waitForEvent('loadedmetadata', function () {
+    run("document.body.removeChild(video)");
+    endTestLater();
+} );
+video.src = "http://127.0.0.1:8000/media/video-load-and-stall.cgi?name=../../../media/content/test.mp4&stallAt=80000";
+</script>

diff --git a/LayoutTests/media/remove-from-document-expected.txt b/LayoutTests/media/remove-from-document-expected.txt
new file mode 100644 (file)
index 0000000..fb9c61e
--- /dev/null
+++ b/LayoutTests/media/remove-from-document-expected.txt
@@ -0,0 +1,10 @@
+Test that removing a media element from the tree pauses playback but does not unload the media.
+
+EVENT(load)
+TEST(video.networkState == HTMLMediaElement.LOADED) OK
+TEST(!video.paused) OK
+RUN(document.body.removeChild(video))
+TEST(video.networkState == HTMLMediaElement.LOADED) OK
+TEST(video.paused) OK
+END OF TEST
+

diff --git a/LayoutTests/media/remove-from-document.html b/LayoutTests/media/remove-from-document.html
new file mode 100644 (file)
index 0000000..0775a73
--- /dev/null
+++ b/LayoutTests/media/remove-from-document.html
@@ -0,0 +1,15 @@
+<video controls></video>
+<p>Test that removing a media element from the tree pauses playback but does not unload the media.</p>
+<script src=video-test.js></script>
+<script>
+waitForEvent('load', function () {
+    test("video.networkState == HTMLMediaElement.LOADED");
+    test("!video.paused");
+    run("document.body.removeChild(video)");
+    test("video.networkState == HTMLMediaElement.LOADED");
+    test("video.paused");
+    endTest();
+} );
+video.src = "content/test.mp4";
+video.play();
+</script>

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 6a73109b7a76d2734e0599987b863886897edda6..a004247d6aa0405407cb12aa9a560c64f245ec8f 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2007-12-01  Antti Koivisto  <antti@apple.com>
+
+        Reviewed by Adam.
+        
+        - Fix crash when removing a loading media element from the tree.
+        - Follow the spec by invoking pause() when element is removed from the tree instead of unloading.
+
+        Tests: http/tests/media/remove-while-loading.html
+               media/remove-from-document.html
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::removedFromDocument):
+
 2007-12-01  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Beth Dakin.

diff --git a/WebCore/html/HTMLMediaElement.cpp b/WebCore/html/HTMLMediaElement.cpp
index 14290face522f27aa03356ae5b84831bbb8ae251..654ba8108b2aa38a3a7570a3b14ddcbd431f5b1a 100644 (file)
--- a/WebCore/html/HTMLMediaElement.cpp
+++ b/WebCore/html/HTMLMediaElement.cpp
@@ -124,8 +124,10 @@ void HTMLMediaElement::insertedIntoDocument()
 
 void HTMLMediaElement::removedFromDocument()
 {
-    delete m_movie;
-    m_movie = 0;
+    // FIXME: pause() may invoke load() which seem like a strange thing to do as a side effect
+    // of removing an element. This might need to be fixed in the spec.
+    ExceptionCode ec;
+    pause(ec);
     HTMLElement::removedFromDocument();
 }