Title: [133512] branches/chromium/1312
Revision
133512
Author
[email protected]
Date
2012-11-05 12:34:39 -0800 (Mon, 05 Nov 2012)

Log Message

Merge 133252 - HTMLMediaPlayer should free m_player when src is set/changed
https://bugs.webkit.org/show_bug.cgi?id=99647

Reviewed by Eric Carlson.

.:

* ManualTests/media-players-are-dropped-on-error.html: Added.
    Various scenarios are tested to make sure players aren't
    leaked in different ways for each of them.

Source/WebCore:

New ManualTest added; manual since leaking media players doesn't have layoutTestController-visible effects.

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::parseAttribute): clearMediaPlayer() when src is set/changed
(WebCore::HTMLMediaElement::userCancelledLoad): use new clearMediaPlayer() helper
(WebCore::HTMLMediaElement::clearMediaPlayer): clear m_player and associated timers/flags
(WebCore):
(WebCore::HTMLMediaElement::createMediaPlayer): whitespace-only change
* html/HTMLMediaElement.h: new method: createMediaPlayer().
(HTMLMediaElement):


[email protected]
Review URL: https://codereview.chromium.org/11275156

Modified Paths

Added Paths

Diff

Modified: branches/chromium/1312/ChangeLog (133511 => 133512)


--- branches/chromium/1312/ChangeLog	2012-11-05 20:28:42 UTC (rev 133511)
+++ branches/chromium/1312/ChangeLog	2012-11-05 20:34:39 UTC (rev 133512)
@@ -1,3 +1,76 @@
+2012-11-01  Ami Fischman  <[email protected]>
+
+        HTMLMediaPlayer should free m_player when src is set/changed
+        https://bugs.webkit.org/show_bug.cgi?id=99647
+
+        Reviewed by Eric Carlson.
+
+        * ManualTests/media-players-are-dropped-on-error.html: Added.
+            Various scenarios are tested to make sure players aren't
+            leaked in different ways for each of them.
+
+2012-11-01  Beth Dakin  <[email protected]>
+
+        https://bugs.webkit.org/show_bug.cgi?id=100917
+        There should be a way to dump the scrolling tree from the layout tests
+
+        Reviewed by Simon Fraser.
+
+        * Source/autotools/symbols.filter:
+
+2012-10-31  Thiago Marcos P. Santos  <[email protected]>
+
+        Added viewport at-rule to the CSS parser and tokenizer
+        https://bugs.webkit.org/show_bug.cgi?id=95961
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Enable CSS Device Adaptation by default on EFL.
+
+        * Source/cmake/OptionsEfl.cmake:
+
+2012-10-31  Ian Vollick  <[email protected]>
+
+        Add support for text-based repaint testing
+        https://bugs.webkit.org/show_bug.cgi?id=100584
+
+        Reviewed by Simon Fraser.
+
+        Allows tracked repaint rects to be dumped as text.
+
+        * Source/autotools/symbols.filter:
+          Exports for:
+            FrameView::setTracksRepaints(bool)
+            Frame::trackedRepaintRectsAsText() const
+
+
+2012-10-30  Vivek Galatage  <[email protected]>
+
+        Add files generated by Windows to ignore list for git repository
+        https://bugs.webkit.org/show_bug.cgi?id=100729
+
+        Reviewed by Gyuyoung Kim.
+
+        Adding the additional files generated by windows port to the ignore list
+
+        * .gitignore:
+
+2012-10-30  Carlos Garcia Campos  <[email protected]>
+
+        [GTK] Add a configure option to build with -g1
+        https://bugs.webkit.org/show_bug.cgi?id=100670
+
+        Reviewed by Martin Robinson.
+
+        Add min and full options to the --enable-debug-symbols configure
+        option. Using --enable-debug-symbols=min will use -g1 instead of
+        -g (which is actually -g2). The first level is enough for most of
+        the cases, like getting a backtrace, and it's the only way to
+        build WebKit with debug symbols in a 32 bit system. The option
+        full is actually the same than yes for backwards compatibility.
+
+        * configure.ac:
+
 2012-10-26  Rob Buis  <[email protected]>
 
         [BlackBerry] Platform Abstraction for WebKit Resource/Image Loading

Copied: branches/chromium/1312/ManualTests/media-players-are-dropped-on-error.html (from rev 133252, trunk/ManualTests/media-players-are-dropped-on-error.html) (0 => 133512)


--- branches/chromium/1312/ManualTests/media-players-are-dropped-on-error.html	                        (rev 0)
+++ branches/chromium/1312/ManualTests/media-players-are-dropped-on-error.html	2012-11-05 20:34:39 UTC (rev 133512)
@@ -0,0 +1,40 @@
+<html>
+<head>
+<script>
+var urls = [
+    "file:///does not exist oh noes/test.mp4",
+    "../LayoutTests/media/content/test-25fps.mp4"
+];
+var kickoffFunctions = [
+    "load",
+    "play"
+];
+
+var mediaElementHolder = [];
+
+function releaseAndAddMediaElements() {
+    for (var i = 0; i < mediaElementHolder.length; ++i)
+        mediaElementHolder[i].src = ""
+    mediaElementHolder = [];
+
+    for (var i = 0; i < 5; ++i) {
+        for (var url in urls) {
+            for (var kickoffFunction in kickoffFunctions) {
+                var a = document.createElement('video');
+                a.controls = "controls";
+                a.src = ""
+                eval("a." + kickoffFunctions[kickoffFunction] + "();");
+                mediaElementHolder.push(a);
+            }
+        }
+    }
+}
+</script>
+</head>
+<body _onload_="setInterval('releaseAndAddMediaElements()', 100)">
+    Test that media players aren't leaked on error.
+    Load this page and verify the number of threads used by the browser doesn't
+    seem unreasonable (e.g. chrome uses 4-5 threads per video tag so staying
+    under 100 threads is "success", since this instantiates 20 &lt;video&gt; elements).
+</body>
+</html>

Modified: branches/chromium/1312/Source/WebCore/ChangeLog (133511 => 133512)


--- branches/chromium/1312/Source/WebCore/ChangeLog	2012-11-05 20:28:42 UTC (rev 133511)
+++ branches/chromium/1312/Source/WebCore/ChangeLog	2012-11-05 20:34:39 UTC (rev 133512)
@@ -1,5 +1,23 @@
-2012-10-30  Keishi Hattori  <[email protected]>
+2012-11-01  Ami Fischman  <[email protected]>
 
+        HTMLMediaPlayer should free m_player when src is set/changed
+        https://bugs.webkit.org/show_bug.cgi?id=99647
+
+        Reviewed by Eric Carlson.
+
+        New ManualTest added; manual since leaking media players doesn't have layoutTestController-visible effects.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::parseAttribute): clearMediaPlayer() when src is set/changed
+        (WebCore::HTMLMediaElement::userCancelledLoad): use new clearMediaPlayer() helper
+        (WebCore::HTMLMediaElement::clearMediaPlayer): clear m_player and associated timers/flags
+        (WebCore):
+        (WebCore::HTMLMediaElement::createMediaPlayer): whitespace-only change
+        * html/HTMLMediaElement.h: new method: createMediaPlayer().
+        (HTMLMediaElement):
+
+2012-11-01  Tom Sepez  <[email protected]>
+
         F4 inside <input type=time> should not open calendar picker
         https://bugs.webkit.org/show_bug.cgi?id=100730
 

Modified: branches/chromium/1312/Source/WebCore/html/HTMLMediaElement.cpp (133511 => 133512)


--- branches/chromium/1312/Source/WebCore/html/HTMLMediaElement.cpp	2012-11-05 20:28:42 UTC (rev 133511)
+++ branches/chromium/1312/Source/WebCore/html/HTMLMediaElement.cpp	2012-11-05 20:34:39 UTC (rev 133512)
@@ -363,8 +363,10 @@
 {
     if (attribute.name() == srcAttr) {
         // Trigger a reload, as long as the 'src' attribute is present.
-        if (fastHasAttribute(srcAttr))
+        if (fastHasAttribute(srcAttr)) {
+            clearMediaPlayer(MediaResource);
             scheduleLoad(MediaResource);
+        }
     } else if (attribute.name() == controlsAttr)
         configureMediaControls();
 #if PLATFORM(MAC)
@@ -3670,13 +3672,7 @@
     // If the media data fetching process is aborted by the user:
 
     // 1 - The user agent should cancel the fetching process.
-#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO)
-    m_player.clear();
-#endif
-    stopPeriodicTimers();
-    m_loadTimer.stop();
-    m_loadState = WaitingForSource;
-    m_pendingLoadFlags = 0;
+    clearMediaPlayer(-1);
 
     // 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);
@@ -3714,6 +3710,18 @@
 #endif
 }
 
+void HTMLMediaElement::clearMediaPlayer(unsigned flags)
+{
+#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO)
+    m_player.clear();
+#endif
+    stopPeriodicTimers();
+    m_loadTimer.stop();
+
+    m_pendingLoadFlags &= ~flags;
+    m_loadState = WaitingForSource;
+}
+
 bool HTMLMediaElement::canSuspend() const
 {
     return true; 
@@ -4261,7 +4269,7 @@
     if (m_audioSourceNode)
         m_audioSourceNode->lock();
 #endif
-        
+
     m_player = MediaPlayer::create(this);
 
 #if ENABLE(MEDIA_SOURCE)

Modified: branches/chromium/1312/Source/WebCore/html/HTMLMediaElement.h (133511 => 133512)


--- branches/chromium/1312/Source/WebCore/html/HTMLMediaElement.h	2012-11-05 20:28:42 UTC (rev 133511)
+++ branches/chromium/1312/Source/WebCore/html/HTMLMediaElement.h	2012-11-05 20:34:39 UTC (rev 133512)
@@ -464,6 +464,7 @@
     void scheduleNextSourceChild();
     void loadNextSourceChild();
     void userCancelledLoad();
+    void clearMediaPlayer(unsigned flags);
     bool havePotentialSourceChild();
     void noneSupported();
     void mediaEngineError(PassRefPtr<MediaError> err);
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to