Title: [116783] trunk
Revision
116783
Author
tom...@google.com
Date
2012-05-11 10:09:28 -0700 (Fri, 11 May 2012)

Log Message

MediaStream API: Fix a reference counting issue in UserMediaRequest
https://bugs.webkit.org/show_bug.cgi?id=86210

Reviewed by Abhishek Arya.

.:

* ManualTests/user-media-request-crash.html: Added.

Source/WebCore:

When contextDestroyed() is called on UserMediaRequest it does a callback to the
page client. If the receiving code clears their stored copy the UserMediaRequest
object is destroyed in the middle of the call.

Currently only testable manually against chrome, preferably with asan turned on.
I have added a manual test that verifies the fix, but I have started work
to make DumpRenderTree able to test this and many other things. The first patch is here:
https://bugs.webkit.org/show_bug.cgi?id=86215

* Modules/mediastream/UserMediaRequest.cpp:
(WebCore::UserMediaRequest::contextDestroyed):

Modified Paths

Added Paths

Diff

Modified: trunk/ChangeLog (116782 => 116783)


--- trunk/ChangeLog	2012-05-11 17:05:23 UTC (rev 116782)
+++ trunk/ChangeLog	2012-05-11 17:09:28 UTC (rev 116783)
@@ -1,3 +1,12 @@
+2012-05-11  Tommy Widenflycht  <tom...@google.com>
+
+        MediaStream API: Fix a reference counting issue in UserMediaRequest
+        https://bugs.webkit.org/show_bug.cgi?id=86210
+
+        Reviewed by Abhishek Arya.
+
+        * ManualTests/user-media-request-crash.html: Added.
+
 2012-05-11  Christophe Dumez  <christophe.du...@intel.com>
 
         Web Intents code only supports V8

Added: trunk/ManualTests/user-media-request-crash.html (0 => 116783)


--- trunk/ManualTests/user-media-request-crash.html	                        (rev 0)
+++ trunk/ManualTests/user-media-request-crash.html	2012-05-11 17:09:28 UTC (rev 116783)
@@ -0,0 +1,28 @@
+<html>
+<head>
+<meta http-equiv="refresh" content="2"/>
+
+<script>
+function error() {
+}
+
+function getUserMedia(dictionary, callback) {
+    try {
+        navigator.webkitGetUserMedia(dictionary, callback, error);
+    } catch (e) {
+    }
+}
+
+function gotStream1(s) {
+}
+
+function start() {
+    getUserMedia({audio:true}, gotStream1);
+}
+</script>
+
+</head>
+<body _onload_="start()">
+<h1>Test passes if it does not crash.</h1>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (116782 => 116783)


--- trunk/Source/WebCore/ChangeLog	2012-05-11 17:05:23 UTC (rev 116782)
+++ trunk/Source/WebCore/ChangeLog	2012-05-11 17:09:28 UTC (rev 116783)
@@ -1,3 +1,22 @@
+2012-05-11  Tommy Widenflycht  <tom...@google.com>
+
+        MediaStream API: Fix a reference counting issue in UserMediaRequest
+        https://bugs.webkit.org/show_bug.cgi?id=86210
+
+        Reviewed by Abhishek Arya.
+
+        When contextDestroyed() is called on UserMediaRequest it does a callback to the
+        page client. If the receiving code clears their stored copy the UserMediaRequest
+        object is destroyed in the middle of the call.
+
+        Currently only testable manually against chrome, preferably with asan turned on.
+        I have added a manual test that verifies the fix, but I have started work
+        to make DumpRenderTree able to test this and many other things. The first patch is here:
+        https://bugs.webkit.org/show_bug.cgi?id=86215
+
+        * Modules/mediastream/UserMediaRequest.cpp:
+        (WebCore::UserMediaRequest::contextDestroyed):
+
 2012-05-11  Min Qin  <qin...@google.com>
 
         split MediaPlayer::enterFullscreen into 2 seperate functions

Modified: trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp (116782 => 116783)


--- trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp	2012-05-11 17:05:23 UTC (rev 116782)
+++ trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp	2012-05-11 17:09:28 UTC (rev 116783)
@@ -111,6 +111,8 @@
 
 void UserMediaRequest::contextDestroyed()
 {
+    RefPtr<UserMediaRequest> protect(this);
+
     if (m_controller) {
         m_controller->cancelUserMediaRequest(this);
         m_controller = 0;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to