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 1ac34b9d59aeff54dab756b48358367027096ab8..991930d991adf5f301e44b41a45466c0e9b6d561 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 c556935a98fd5998e4694637b959d71b699eb6c9..89d4e62a6420563ed95990792308c75956941820 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 63d02e29f73c32b17d4030dab647c8a33a339aaf..6c5faf4fea8f8190e21e49bf2892619eb2c2e73c 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;