changes for proxy audio

chrome/renderer/pepper_plugin_delegate_impl.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <cmath>
 
 #include "chrome/renderer/pepper_plugin_delegate_impl.h"
 
 #include "app/l10n_util.h"
 #include "app/surface/transport_dib.h"
@@ skipping 105 matching lines @@
 
  private:
   WebKit::WebView* web_view_;
   ggl::Context* context_;
 };
 
 #endif  // ENABLE_GPU
 
 class PlatformAudioImpl
     : public pepper::PluginDelegate::PlatformAudio,
-      public AudioMessageFilter::Delegate {
+      public AudioMessageFilter::Delegate,
+      public base::RefCountedThreadSafe<PlatformAudioImpl> {
  public:
   explicit PlatformAudioImpl(scoped_refptr<AudioMessageFilter> filter)
-      : client_(NULL), filter_(filter), stream_id_(0) {
+      : client_(NULL), filter_(filter), stream_id_(0),
+        main_message_loop_(MessageLoop::current()) {
     DCHECK(filter_);
   }
 
   virtual ~PlatformAudioImpl() {
     // Make sure we have been shut down.
     DCHECK_EQ(0, stream_id_);
     DCHECK(!client_);
   }
 
   // Initialize this audio context. StreamCreated() will be called when the
@@ skipping 30 matching lines @@
 
   virtual void OnVolume(double volume) { }
 
   // The client to notify when the stream is created.
   pepper::PluginDelegate::PlatformAudio::Client* client_;
   // MessageFilter used to send/receive IPC.
   scoped_refptr<AudioMessageFilter> filter_;
   // Our ID on the MessageFilter.
   int32 stream_id_;
 
+  MessageLoop* main_message_loop_;

[neb, 2010/11/22 19:48:26]

I'm not convinced you need to store this since this should be the same as
filter_->message_loop(). You seem to be presuming that in the rest of the code.

[nfullagar, 2010/11/23 02:48:58]

Printing out filter_->message_loop() and main_message_loop_ from the constructor
show them to be different.

+
   DISALLOW_COPY_AND_ASSIGN(PlatformAudioImpl);
 };
 
 #ifdef ENABLE_GPU
 
 bool PlatformContext3DImpl::Init() {
   // Ignore initializing more than once.
   if (context_)
     return true;
 
@@ skipping 84 matching lines @@
   client_ = client;
 
   ViewHostMsg_Audio_CreateStream_Params params;
   params.params.format = AudioParameters::AUDIO_PCM_LINEAR;
   params.params.channels = 2;
   params.params.sample_rate = sample_rate;
   params.params.bits_per_sample = 16;
   params.params.samples_per_packet = sample_count;
 
   stream_id_ = filter_->AddDelegate(this);
+  AddRef();  // Take a reference for the filter.
+
   return filter_->Send(new ViewHostMsg_CreateAudioStream(0, stream_id_, params,
                                                          true));
 }
 
 void PlatformAudioImpl::OnLowLatencyCreated(
     base::SharedMemoryHandle handle, base::SyncSocket::Handle socket_handle,
     uint32 length) {
 #if defined(OS_WIN)
   DCHECK(handle);
   DCHECK(socket_handle);
 #else
   DCHECK_NE(-1, handle.fd);
   DCHECK_NE(-1, socket_handle);
 #endif
   DCHECK(length);
 
-  client_->StreamCreated(handle, length, socket_handle);
+  if (MessageLoop::current() == main_message_loop_) {
+    if (client_) {
+      client_->StreamCreated(handle, length, socket_handle);

[neb, 2010/11/22 19:48:26]

If the reference was taken to ensure that the object is alive while waiting for
CreateAudioStream() to return, then it should be released here after
StreamCreated() is called, and not in ShutDown().

The way I would go about writing this is to add another method called
DispatchStreamCreated() or something, that would do:

DispatchStreamCreated() {
  client->StreamCreated(handle, length, socket_handle);
  Release();
}

and then here I'd do:

filter_->message_loop()->PostTask(FROM_HERE, new RunnableMethod(this,
&PlatformAudioImpl::DispatchStreamCreated, handle, socket_handle, length));
    

[nfullagar, 2010/11/23 02:48:58]

What is here seems to be working, but I'd like to discuss more (maybe tomorrow?)

+    }
+  } else {
+    main_message_loop_->PostTask(FROM_HERE,
+        NewRunnableMethod(this, &PlatformAudioImpl::OnLowLatencyCreated,
+                          handle, socket_handle, length));
+  }
 }
 
 void PlatformAudioImpl::ShutDown() {
   // Make sure we don't call shutdown more than once.
   if (!stream_id_) {
     return;
   }
+
   filter_->Send(new ViewHostMsg_CloseAudioStream(0, stream_id_));
   filter_->RemoveDelegate(stream_id_);

[neb, 2010/11/22 19:48:26]

I think a way to avoid the race would be to move RemoveDelegate() into the
destructor and make sure OnLowLatencyCreated() handles being called after
ShutDown().

+
+  // Release reference taken for the filter on the IO thread so that
+  // we avoid race problems with OnLowLatencyCreated.
+  filter_->message_loop()->ReleaseSoon(FROM_HERE, this);

[neb, 2010/11/22 19:48:26]

You are releasing twice, it seams?

[nfullagar, 2010/11/23 02:48:58]

There's an extra AddRef() above (line 288.)  If I understand it correctly, this
puts a release on the message queue.  If there is a pending StreamCreated also
in the queue, it will be invoked first, then this release will occur.

+
   stream_id_ = 0;
   client_ = NULL;
+
+  Release();  // May end up deleting ourselves.

[neb, 2010/11/22 19:48:26]

I think this Release() doesn't belong here.

 }
 
 // Implements the VideoDecoder.
 class PlatformVideoDecoderImpl
     : public pepper::PluginDelegate::PlatformVideoDecoder {
  public:
   PlatformVideoDecoderImpl()
       : input_buffer_size_(0),
         next_dib_id_(0),
         dib_(NULL) {
@@ skipping 231 matching lines @@
 
 void PepperPluginDelegateImpl::SelectedFindResultChanged(int identifier,
                                                          int index) {
   render_view_->reportFindInPageSelection(
       identifier, index + 1, WebKit::WebRect());
 }
 
 pepper::PluginDelegate::PlatformAudio* PepperPluginDelegateImpl::CreateAudio(
     uint32_t sample_rate, uint32_t sample_count,
     pepper::PluginDelegate::PlatformAudio::Client* client) {
-  scoped_ptr<PlatformAudioImpl> audio(
+  scoped_refptr<PlatformAudioImpl> audio(
       new PlatformAudioImpl(render_view_->audio_message_filter()));
   if (audio->Initialize(sample_rate, sample_count, client)) {
     return audio.release();
   } else {
     return NULL;
   }
 }
 
 bool PepperPluginDelegateImpl::RunFileChooser(
     const WebKit::WebFileChooserParams& params,
@@ skipping 250 matching lines @@
 }
 
 void PepperPluginDelegateImpl::DidStopLoading() {
   render_view_->DidStopLoadingForPlugin();
 }
 
 void PepperPluginDelegateImpl::SetContentRestriction(int restrictions) {
   render_view_->Send(new ViewHostMsg_UpdateContentRestrictions(
       render_view_->routing_id(), restrictions));
 }