Title: [133252] trunk
- Revision
- 133252
- Author
- fisch...@chromium.org
- Date
- 2012-11-01 18:38:30 -0700 (Thu, 01 Nov 2012)
Log Message
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):
Modified Paths
Added Paths
Diff
Modified: trunk/ChangeLog (133251 => 133252)
--- trunk/ChangeLog 2012-11-02 01:36:33 UTC (rev 133251)
+++ trunk/ChangeLog 2012-11-02 01:38:30 UTC (rev 133252)
@@ -1,3 +1,14 @@
+2012-11-01 Ami Fischman <fisch...@chromium.org>
+
+ 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 <bda...@apple.com>
https://bugs.webkit.org/show_bug.cgi?id=100917
Added: trunk/ManualTests/media-players-are-dropped-on-error.html (0 => 133252)
--- trunk/ManualTests/media-players-are-dropped-on-error.html (rev 0)
+++ trunk/ManualTests/media-players-are-dropped-on-error.html 2012-11-02 01:38:30 UTC (rev 133252)
@@ -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 <video> elements).
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (133251 => 133252)
--- trunk/Source/WebCore/ChangeLog 2012-11-02 01:36:33 UTC (rev 133251)
+++ trunk/Source/WebCore/ChangeLog 2012-11-02 01:38:30 UTC (rev 133252)
@@ -1,3 +1,21 @@
+2012-11-01 Ami Fischman <fisch...@chromium.org>
+
+ 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 <tse...@chromium.org>
XSS blocker false positive when page contains <iframe src=""
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (133251 => 133252)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2012-11-02 01:36:33 UTC (rev 133251)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2012-11-02 01:38:30 UTC (rev 133252)
@@ -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)
@@ -3669,13 +3671,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);
@@ -3713,6 +3709,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;
@@ -4263,7 +4271,7 @@
if (m_audioSourceNode)
m_audioSourceNode->lock();
#endif
-
+
m_player = MediaPlayer::create(this);
#if ENABLE(MEDIA_SOURCE)
Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (133251 => 133252)
--- trunk/Source/WebCore/html/HTMLMediaElement.h 2012-11-02 01:36:33 UTC (rev 133251)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h 2012-11-02 01:38:30 UTC (rev 133252)
@@ -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
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
