MediaStream API: Fix a reference counting issue in UserMediaRequest
authortommyw@google.com <tommyw@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 May 2012 17:09:28 +0000 (17:09 +0000)
committertommyw@google.com <tommyw@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 May 2012 17:09:28 +0000 (17:09 +0000)
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):

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

ChangeLog
ManualTests/user-media-request-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/UserMediaRequest.cpp

index 1ac34b9..991930d 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-05-11  Tommy Widenflycht  <tommyw@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.dumez@intel.com>
 
         Web Intents code only supports V8
diff --git a/ManualTests/user-media-request-crash.html b/ManualTests/user-media-request-crash.html
new file mode 100644 (file)
index 0000000..000f530
--- /dev/null
@@ -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>
index c556935..89d4e62 100644 (file)
@@ -1,3 +1,22 @@
+2012-05-11  Tommy Widenflycht  <tommyw@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  <qinmin@google.com>
 
         split MediaPlayer::enterFullscreen into 2 seperate functions
index 63d02e2..6c5faf4 100644 (file)
@@ -111,6 +111,8 @@ void UserMediaRequest::fail()
 
 void UserMediaRequest::contextDestroyed()
 {
+    RefPtr<UserMediaRequest> protect(this);
+
     if (m_controller) {
         m_controller->cancelUserMediaRequest(this);
         m_controller = 0;