If shared memory cannot be mapped, the audio thread should not be started as it will call the audio callback with an invalid buffer.

This also shifts all the state checks into the start of StartThread for safety/clarity. Note that this changes the behavior very slightly in that shared memory will be mapped even if a callback is not provided. However, AFAICT this will not be the common case and IMO simplifies the logic here. If you disagree, I'm happy to add the additional check.

BUG=118346
TEST=Audio tests


Review URL: http://codereview.chromium.org/10107021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134150 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 23 insertions, 16 deletions

diff --git a/ppapi/shared_impl/ppb_audio_input_shared.cc b/ppapi/shared_impl/ppb_audio_input_shared.cc
index 7a7aeef..963e88b 100644
--- a/ppapi/shared_impl/ppb_audio_input_shared.cc
+++ b/ppapi/shared_impl/ppb_audio_input_shared.cc
@@ -181,9 +181,8 @@ void PPB_AudioInput_Shared::SetStartCaptureState() {
   DCHECK(!capturing_);
   DCHECK(!audio_input_thread_.get());
 
-  if (audio_input_callback_ && socket_.get())
-    StartThread();
   capturing_ = true;
+  StartThread();
 }
 
 void PPB_AudioInput_Shared::SetStopCaptureState() {
@@ -204,8 +203,10 @@ void PPB_AudioInput_Shared::SetStreamInfo(
   shared_memory_.reset(new base::SharedMemory(shared_memory_handle, false));
   shared_memory_size_ = shared_memory_size;
 
-  if (audio_input_callback_)
-    shared_memory_->Map(shared_memory_size_);
+  if (!shared_memory_->Map(shared_memory_size_)) {
+    PpapiGlobals::Get()->LogWithSource(pp_instance(), PP_LOGLEVEL_WARNING, "",
+      "Failed to map shared memory for PPB_AudioInput_Shared.");
+  }
 
   // There is a pending capture request before SetStreamInfo().
   if (capturing_) {
@@ -218,7 +219,11 @@ void PPB_AudioInput_Shared::SetStreamInfo(
 }
 
 void PPB_AudioInput_Shared::StartThread() {
-  DCHECK(audio_input_callback_);
+  // Don't start the thread unless all our state is set up correctly.
+  if (!audio_input_callback_ || !socket_.get() || !capturing_ ||
+      !shared_memory_->memory()) {
+    return;
+  }
   DCHECK(!audio_input_thread_.get());
   audio_input_thread_.reset(new base::DelegateSimpleThread(
       this, "plugin_audio_input_thread"));
diff --git a/ppapi/shared_impl/ppb_audio_shared.cc b/ppapi/shared_impl/ppb_audio_shared.cc
index 00742cc..1323d89 100644
--- a/ppapi/shared_impl/ppb_audio_shared.cc
+++ b/ppapi/shared_impl/ppb_audio_shared.cc
@@ -5,6 +5,7 @@
 #include "ppapi/shared_impl/ppb_audio_shared.h"
 
 #include "base/logging.h"
+#include "ppapi/shared_impl/ppapi_globals.h"
 
 using base::subtle::Atomic32;
 
@@ -65,9 +66,8 @@ void PPB_Audio_Shared::SetStartPlaybackState() {
   // notify us. This is a common case. In this case, we just set the playing_
   // flag and the playback will automatically start when that data is available
   // in SetStreamInfo.
-  if (callback_ && socket_.get())
-    StartThread();
   playing_ = true;
+  StartThread();
 }
 
 void PPB_Audio_Shared::SetStopPlaybackState() {
@@ -81,6 +81,7 @@ void PPB_Audio_Shared::SetStopPlaybackState() {
 }
 
 void PPB_Audio_Shared::SetStreamInfo(
+    PP_Instance instance,
     base::SharedMemoryHandle shared_memory_handle,
     size_t shared_memory_size,
     base::SyncSocket::Handle socket_handle) {
@@ -88,18 +89,18 @@ void PPB_Audio_Shared::SetStreamInfo(
   shared_memory_.reset(new base::SharedMemory(shared_memory_handle, false));
   shared_memory_size_ = shared_memory_size;
 
-  if (callback_) {
-    shared_memory_->Map(TotalSharedMemorySizeInBytes(
-        shared_memory_size_));
-
-    // In common case StartPlayback() was called before StreamCreated().
-    if (playing_)
-      StartThread();
+  if (!shared_memory_->Map(TotalSharedMemorySizeInBytes(shared_memory_size_))) {
+    PpapiGlobals::Get()->LogWithSource(instance, PP_LOGLEVEL_WARNING, "",
+      "Failed to map shared memory for PPB_Audio_Shared.");
   }
+
+  StartThread();
 }
 
 void PPB_Audio_Shared::StartThread() {
-  DCHECK(callback_);
+  // Don't start the thread unless all our state is set up correctly.
+  if (!playing_ || !callback_ || !socket_.get() || !shared_memory_->memory())
+    return;
   DCHECK(!audio_thread_.get());
   audio_thread_.reset(new base::DelegateSimpleThread(
       this, "plugin_audio_thread"));
diff --git a/ppapi/shared_impl/ppb_audio_shared.h b/ppapi/shared_impl/ppb_audio_shared.h
index aa06f6a..b6ee1ee 100644
--- a/ppapi/shared_impl/ppb_audio_shared.h
+++ b/ppapi/shared_impl/ppb_audio_shared.h
@@ -49,7 +49,8 @@ class PPAPI_SHARED_EXPORT PPB_Audio_Shared
 
   // Sets the shared memory and socket handles. This will automatically start
   // playback if we're currently set to play.
-  void SetStreamInfo(base::SharedMemoryHandle shared_memory_handle,
+  void SetStreamInfo(PP_Instance instance,
+                     base::SharedMemoryHandle shared_memory_handle,
                      size_t shared_memory_size,
                      base::SyncSocket::Handle socket_handle);