diff options
9 files changed, 55 insertions, 180 deletions
diff --git a/chrome/browser/browser_io_message_filter.cc b/chrome/browser/browser_io_message_filter.cc index 1137dfe..bd1fd500d 100644 --- a/chrome/browser/browser_io_message_filter.cc +++ b/chrome/browser/browser_io_message_filter.cc @@ -6,6 +6,7 @@ #include "base/logging.h" #include "base/process_util.h" +#include "chrome/browser/renderer_host/browser_render_process_host.h" BrowserIOMessageFilter::BrowserIOMessageFilter() : channel_(NULL) { } @@ -34,3 +35,7 @@ bool BrowserIOMessageFilter::Send(IPC::Message* msg) { delete msg; return false; } + +void BrowserIOMessageFilter::BadMessageReceived(uint32 msg_type) { + BrowserRenderProcessHost::BadMessageTerminateProcess(msg_type, peer_handle()); +} diff --git a/chrome/browser/browser_io_message_filter.h b/chrome/browser/browser_io_message_filter.h index 8063fa9..2f19850 100644 --- a/chrome/browser/browser_io_message_filter.h +++ b/chrome/browser/browser_io_message_filter.h @@ -29,6 +29,9 @@ class BrowserIOMessageFilter : public IPC::ChannelProxy::MessageFilter, protected: base::ProcessHandle peer_handle() { return peer_handle_; } + // Call this if a message couldn't be deserialized. This kills the renderer. + void BadMessageReceived(uint32 msg_type); + private: IPC::Channel* channel_; base::ProcessHandle peer_handle_; diff --git a/chrome/browser/renderer_host/audio_renderer_host.cc b/chrome/browser/renderer_host/audio_renderer_host.cc index b18c4c3..fc8c84b 100644 --- a/chrome/browser/renderer_host/audio_renderer_host.cc +++ b/chrome/browser/renderer_host/audio_renderer_host.cc @@ -48,54 +48,24 @@ AudioRendererHost::AudioEntry::~AudioEntry() {} /////////////////////////////////////////////////////////////////////////////// // AudioRendererHost implementations. -AudioRendererHost::AudioRendererHost() - : process_id_(0), - process_handle_(0), - ipc_sender_(NULL) { - // Increase the ref count of this object so it is active until we do - // Release(). - AddRef(); +AudioRendererHost::AudioRendererHost() { } AudioRendererHost::~AudioRendererHost() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); DCHECK(audio_entries_.empty()); - - // Make sure we received IPCChannelClosing() signal. - DCHECK(!ipc_sender_); - DCHECK(!process_handle_); -} - -void AudioRendererHost::Destroy() { - // Post a message to the thread where this object should live and do the - // actual operations there. - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - NewRunnableMethod(this, &AudioRendererHost::DoDestroy)); -} - -// Event received when IPC channel is connected to the renderer process. -void AudioRendererHost::IPCChannelConnected(int process_id, - base::ProcessHandle process_handle, - IPC::Message::Sender* ipc_sender) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - process_handle_ = process_handle; - ipc_sender_ = ipc_sender; } -// Event received when IPC channel is closing. -void AudioRendererHost::IPCChannelClosing() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - // Reset IPC related member variables. - ipc_sender_ = NULL; - process_handle_ = 0; +void AudioRendererHost::OnChannelClosing() { + BrowserIOMessageFilter::OnChannelClosing(); // Since the IPC channel is gone, close all requested audio streams. DeleteEntries(); } +void AudioRendererHost::OnDestruct() const { + BrowserThread::DeleteOnIOThread::Destruct(this); +} + /////////////////////////////////////////////////////////////////////////////// // media::AudioOutputController::EventHandler implementations. void AudioRendererHost::OnCreated(media::AudioOutputController* controller) { @@ -158,7 +128,7 @@ void AudioRendererHost::DoCompleteCreation( if (!entry) return; - if (!process_handle_) { + if (!peer_handle()) { NOTREACHED() << "Renderer process handle is invalid."; DeleteEntryOnError(entry); return; @@ -167,8 +137,8 @@ void AudioRendererHost::DoCompleteCreation( // Once the audio stream is created then complete the creation process by // mapping shared memory and sharing with the renderer process. base::SharedMemoryHandle foreign_memory_handle; - if (!entry->shared_memory.ShareToProcess(process_handle_, - &foreign_memory_handle)) { + if (!entry->shared_memory.ShareToProcess(peer_handle(), + &foreign_memory_handle)) { // If we failed to map and share the shared memory then close the audio // stream and send an error message. DeleteEntryOnError(entry); @@ -187,13 +157,13 @@ void AudioRendererHost::DoCompleteCreation( // If we failed to prepare the sync socket for the renderer then we fail // the construction of audio stream. - if (!reader->PrepareForeignSocketHandle(process_handle_, + if (!reader->PrepareForeignSocketHandle(peer_handle(), &foreign_socket_handle)) { DeleteEntryOnError(entry); return; } - SendMessage(new ViewMsg_NotifyLowLatencyAudioStreamCreated( + Send(new ViewMsg_NotifyLowLatencyAudioStreamCreated( entry->render_view_id, entry->stream_id, foreign_memory_handle, foreign_socket_handle, entry->shared_memory.created_size())); return; @@ -201,7 +171,7 @@ void AudioRendererHost::DoCompleteCreation( // The normal audio stream has created, send a message to the renderer // process. - SendMessage(new ViewMsg_NotifyAudioStreamCreated( + Send(new ViewMsg_NotifyAudioStreamCreated( entry->render_view_id, entry->stream_id, foreign_memory_handle, entry->shared_memory.created_size())); } @@ -216,7 +186,7 @@ void AudioRendererHost::DoSendPlayingMessage( ViewMsg_AudioStreamState_Params params; params.state = ViewMsg_AudioStreamState_Params::kPlaying; - SendMessage(new ViewMsg_NotifyAudioStreamStateChanged( + Send(new ViewMsg_NotifyAudioStreamStateChanged( entry->render_view_id, entry->stream_id, params)); } @@ -230,7 +200,7 @@ void AudioRendererHost::DoSendPausedMessage( ViewMsg_AudioStreamState_Params params; params.state = ViewMsg_AudioStreamState_Params::kPaused; - SendMessage(new ViewMsg_NotifyAudioStreamStateChanged( + Send(new ViewMsg_NotifyAudioStreamStateChanged( entry->render_view_id, entry->stream_id, params)); } @@ -246,9 +216,8 @@ void AudioRendererHost::DoRequestMoreData( DCHECK(!entry->controller->LowLatencyMode()); entry->pending_buffer_request = true; - SendMessage( - new ViewMsg_RequestAudioPacket( - entry->render_view_id, entry->stream_id, buffers_state)); + Send(new ViewMsg_RequestAudioPacket( + entry->render_view_id, entry->stream_id, buffers_state)); } void AudioRendererHost::DoHandleError(media::AudioOutputController* controller, @@ -264,13 +233,10 @@ void AudioRendererHost::DoHandleError(media::AudioOutputController* controller, /////////////////////////////////////////////////////////////////////////////// // IPC Messages handler -bool AudioRendererHost::OnMessageReceived(const IPC::Message& message, - bool* message_was_ok) { - if (!IsAudioRendererHostMessage(message)) - return false; - *message_was_ok = true; - - IPC_BEGIN_MESSAGE_MAP_EX(AudioRendererHost, message, *message_was_ok) +bool AudioRendererHost::OnMessageReceived(const IPC::Message& message) { + bool handled = true; + bool message_was_ok = true; + IPC_BEGIN_MESSAGE_MAP_EX(AudioRendererHost, message, message_was_ok) IPC_MESSAGE_HANDLER(ViewHostMsg_CreateAudioStream, OnCreateStream) IPC_MESSAGE_HANDLER(ViewHostMsg_PlayAudioStream, OnPlayStream) IPC_MESSAGE_HANDLER(ViewHostMsg_PauseAudioStream, OnPauseStream) @@ -279,27 +245,13 @@ bool AudioRendererHost::OnMessageReceived(const IPC::Message& message, IPC_MESSAGE_HANDLER(ViewHostMsg_NotifyAudioPacketReady, OnNotifyPacketReady) IPC_MESSAGE_HANDLER(ViewHostMsg_GetAudioVolume, OnGetVolume) IPC_MESSAGE_HANDLER(ViewHostMsg_SetAudioVolume, OnSetVolume) + IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP_EX() - return true; -} - -bool AudioRendererHost::IsAudioRendererHostMessage( - const IPC::Message& message) { - switch (message.type()) { - case ViewHostMsg_CreateAudioStream::ID: - case ViewHostMsg_PlayAudioStream::ID: - case ViewHostMsg_PauseAudioStream::ID: - case ViewHostMsg_FlushAudioStream::ID: - case ViewHostMsg_CloseAudioStream::ID: - case ViewHostMsg_NotifyAudioPacketReady::ID: - case ViewHostMsg_GetAudioVolume::ID: - case ViewHostMsg_SetAudioVolume::ID: - return true; - default: - break; - } - return false; + if (!message_was_ok) + BadMessageReceived(message.type()); + + return handled; } void AudioRendererHost::OnCreateStream( @@ -452,32 +404,11 @@ void AudioRendererHost::OnNotifyPacketReady( packet_size); } -void AudioRendererHost::DoDestroy() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - // Reset IPC releated members. - ipc_sender_ = NULL; - process_handle_ = 0; - - // Close all audio streams. - DeleteEntries(); - - // Decrease the reference to this object, which may lead to self-destruction. - Release(); -} - -void AudioRendererHost::SendMessage(IPC::Message* message) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - if (ipc_sender_) - ipc_sender_->Send(message); -} - void AudioRendererHost::SendErrorMessage(int32 render_view_id, int32 stream_id) { ViewMsg_AudioStreamState_Params state; state.state = ViewMsg_AudioStreamState_Params::kError; - SendMessage(new ViewMsg_NotifyAudioStreamStateChanged( + Send(new ViewMsg_NotifyAudioStreamStateChanged( render_view_id, stream_id, state)); } diff --git a/chrome/browser/renderer_host/audio_renderer_host.h b/chrome/browser/renderer_host/audio_renderer_host.h index 85311ab..3248205 100644 --- a/chrome/browser/renderer_host/audio_renderer_host.h +++ b/chrome/browser/renderer_host/audio_renderer_host.h @@ -6,17 +6,12 @@ // lives inside the render process and provide access to audio hardware. // // This class is owned by BrowserRenderProcessHost, and instantiated on UI -// thread, but all other operations and method calls (except Destroy()) happens -// in IO thread, so we need to be extra careful about the lifetime of this -// object. AudioManager is a singleton and created in IO thread, audio output -// streams are also created in the IO thread, so we need to destroy them also -// in IO thread. After this class is created, a task of OnInitialized() is -// posted on IO thread in which singleton of AudioManager is created and -// AddRef() is called to increase one ref count of this object. Owner of this -// class should call Destroy() before decrementing the ref count to this object, -// which essentially post a task of OnDestroyed() on IO thread. Inside -// OnDestroyed(), audio output streams are destroyed and Release() is called -// which may result in self-destruction. +// thread, but all other operations and method calls happen on IO thread, so we +// need to be extra careful about the lifetime of this object. AudioManager is a +// singleton and created in IO thread, audio output streams are also created in +// the IO thread, so we need to destroy them also in IO thread. After this class +// is created, a task of OnInitialized() is posted on IO thread in which +// singleton of AudioManager is created and. // // Here's an example of a typical IPC dialog for audio: // @@ -65,6 +60,7 @@ #include "base/ref_counted.h" #include "base/scoped_ptr.h" #include "base/shared_memory.h" +#include "chrome/browser/browser_io_message_filter.h" #include "chrome/browser/browser_thread.h" #include "ipc/ipc_message.h" #include "media/audio/audio_io.h" @@ -74,9 +70,7 @@ class AudioManager; struct ViewHostMsg_Audio_CreateStream_Params; -class AudioRendererHost : public base::RefCountedThreadSafe< - AudioRendererHost, - BrowserThread::DeleteOnIOThread>, +class AudioRendererHost : public BrowserIOMessageFilter, public media::AudioOutputController::EventHandler { public: typedef std::pair<int32, int> AudioEntryId; @@ -112,24 +106,11 @@ class AudioRendererHost : public base::RefCountedThreadSafe< // Called from UI thread from the owner of this object. AudioRendererHost(); - // Called from UI thread from the owner of this object to kick start - // destruction of streams in IO thread. - void Destroy(); - ///////////////////////////////////////////////////////////////////////////// - // The following public methods are called from ResourceMessageFilter in the - // IO thread. - - // Event received when IPC channel is connected with the renderer process. - void IPCChannelConnected(int process_id, base::ProcessHandle process_handle, - IPC::Message::Sender* ipc_sender); - - // Event received when IPC channel is closing. - void IPCChannelClosing(); - - // Returns true if the message is a audio related message and was processed. - // If it was, message_was_ok will be false iff the message was corrupt. - bool OnMessageReceived(const IPC::Message& message, bool* message_was_ok); + // BrowserIOMessageFilter implementation + virtual bool OnMessageReceived(const IPC::Message& message); + virtual void OnChannelClosing(); + virtual void OnDestruct() const; ///////////////////////////////////////////////////////////////////////////// // AudioOutputController::EventHandler implementations. @@ -187,9 +168,6 @@ class AudioRendererHost : public base::RefCountedThreadSafe< void OnNotifyPacketReady(const IPC::Message& msg, int stream_id, uint32 packet_size); - // Release all acquired resources and decrease reference to this object. - void DoDestroy(); - // Complete the process of creating an audio stream. This will set up the // shared memory or shared socket in low latency mode. void DoCompleteCreation(media::AudioOutputController* controller); @@ -206,10 +184,6 @@ class AudioRendererHost : public base::RefCountedThreadSafe< // Handle error coming from audio stream. void DoHandleError(media::AudioOutputController* controller, int error_code); - // A helper method to send an IPC message to renderer process on IO thread. - // This method is virtual for testing purpose. - virtual void SendMessage(IPC::Message* message); - // Send an error message to the renderer. void SendErrorMessage(int32 render_view_id, int32 stream_id); @@ -239,10 +213,6 @@ class AudioRendererHost : public base::RefCountedThreadSafe< // event is received. AudioEntry* LookupByController(media::AudioOutputController* controller); - int process_id_; - base::ProcessHandle process_handle_; - IPC::Message::Sender* ipc_sender_; - // A map of id to audio sources. AudioEntryMap audio_entries_; diff --git a/chrome/browser/renderer_host/audio_renderer_host_unittest.cc b/chrome/browser/renderer_host/audio_renderer_host_unittest.cc index 7c3b500..a71401f 100644 --- a/chrome/browser/renderer_host/audio_renderer_host_unittest.cc +++ b/chrome/browser/renderer_host/audio_renderer_host_unittest.cc @@ -67,7 +67,7 @@ class MockAudioRendererHost : public AudioRendererHost { // This method is used to dispatch IPC messages to the renderer. We intercept // these messages here and dispatch to our mock methods to verify the // conversation between this object and the renderer. - virtual void SendMessage(IPC::Message* message) { + virtual bool Send(IPC::Message* message) { CHECK(message); // In this method we dispatch the messages to the according handlers as if @@ -86,6 +86,7 @@ class MockAudioRendererHost : public AudioRendererHost { EXPECT_TRUE(handled); delete message; + return true; } // These handler methods do minimal things and delegate to the mock methods. @@ -175,20 +176,15 @@ class AudioRendererHostTest : public testing::Test { host_ = new MockAudioRendererHost(); // Simulate IPC channel connected. - host_->IPCChannelConnected(base::GetCurrentProcId(), - base::GetCurrentProcessHandle(), - NULL); + host_->OnChannelConnected(base::GetCurrentProcId()); } virtual void TearDown() { // Simulate closing the IPC channel. - host_->IPCChannelClosing(); - - // This task post a task to message_loop_ to do internal destruction on - // message_loop_. - host_->Destroy(); + host_->OnChannelClosing(); - // Release the reference to the mock object. + // Release the reference to the mock object. The object will be destructed + // on message_loop_. host_ = NULL; // We need to continue running message_loop_ to complete all destructions. diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc index 920013a..9c8846c 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.cc +++ b/chrome/browser/renderer_host/browser_render_process_host.cc @@ -273,10 +273,6 @@ BrowserRenderProcessHost::~BrowserRenderProcessHost() { queued_messages_.pop(); } - // Destroy the AudioRendererHost properly. - if (audio_renderer_host_.get()) - audio_renderer_host_->Destroy(); - ClearTransportDIBCache(); } @@ -377,22 +373,17 @@ bool BrowserRenderProcessHost::Init( } void BrowserRenderProcessHost::CreateMessageFilters() { - // Construct the AudioRendererHost with the IO thread. - audio_renderer_host_ = new AudioRendererHost(); - scoped_refptr<ResourceMessageFilter> resource_message_filter( new ResourceMessageFilter(g_browser_process->resource_dispatcher_host(), id(), - audio_renderer_host_.get(), PluginService::GetInstance(), g_browser_process->print_job_manager(), profile(), widget_helper_)); channel_->AddFilter(resource_message_filter); - scoped_refptr<PepperFileMessageFilter> pepper_file_message_filter( - new PepperFileMessageFilter(id(), profile())); - channel_->AddFilter(pepper_file_message_filter); + channel_->AddFilter(new AudioRendererHost()); + channel_->AddFilter(new PepperFileMessageFilter(id(), profile())); } int BrowserRenderProcessHost::GetNextRoutingID() { diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h index 4852141..0b3a78a 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.h +++ b/chrome/browser/renderer_host/browser_render_process_host.h @@ -22,7 +22,6 @@ #include "chrome/common/notification_registrar.h" #include "third_party/WebKit/WebKit/chromium/public/WebCache.h" -class AudioRendererHost; class CommandLine; class GURL; class RendererMainThread; @@ -185,9 +184,6 @@ class BrowserRenderProcessHost : public RenderProcessHost, // IO thread. scoped_refptr<RenderWidgetHelper> widget_helper_; - // The host of audio renderers in the renderer process. - scoped_refptr<AudioRendererHost> audio_renderer_host_; - // A map of transport DIB ids to cached TransportDIBs std::map<TransportDIB::Id, TransportDIB*> cached_dibs_; enum { diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc index b7dfd01..4ea3117 100644 --- a/chrome/browser/renderer_host/resource_message_filter.cc +++ b/chrome/browser/renderer_host/resource_message_filter.cc @@ -44,7 +44,6 @@ #include "chrome/browser/printing/print_job_manager.h" #include "chrome/browser/printing/printer_query.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/renderer_host/audio_renderer_host.h" #include "chrome/browser/renderer_host/blob_dispatcher_host.h" #include "chrome/browser/renderer_host/browser_render_process_host.h" #include "chrome/browser/renderer_host/database_dispatcher_host.h" @@ -243,7 +242,6 @@ class OpenChannelToPluginCallback : public PluginProcessHost::Client { ResourceMessageFilter::ResourceMessageFilter( ResourceDispatcherHost* resource_dispatcher_host, int child_id, - AudioRendererHost* audio_renderer_host, PluginService* plugin_service, printing::PrintJobManager* print_job_manager, Profile* profile, @@ -258,7 +256,6 @@ ResourceMessageFilter::ResourceMessageFilter( media_request_context_(profile->GetRequestContextForMedia()), extensions_request_context_(profile->GetRequestContextForExtensions()), render_widget_helper_(render_widget_helper), - audio_renderer_host_(audio_renderer_host), appcache_dispatcher_host_( new AppCacheDispatcherHost(profile->GetRequestContext())), ALLOW_THIS_IN_INITIALIZER_LIST(dom_storage_dispatcher_host_( @@ -297,7 +294,6 @@ ResourceMessageFilter::ResourceMessageFilter( request_context_ = profile_->GetRequestContext(); DCHECK(request_context_); DCHECK(media_request_context_); - DCHECK(audio_renderer_host_.get()); DCHECK(appcache_dispatcher_host_.get()); DCHECK(dom_storage_dispatcher_host_.get()); @@ -361,10 +357,6 @@ void ResourceMessageFilter::OnChannelConnected(int32 peer_pid) { } set_handle(peer_handle); - // Hook AudioRendererHost to this object after channel is connected so it can - // this object for sending messages. - audio_renderer_host_->IPCChannelConnected(id(), handle(), this); - WorkerService::GetInstance()->Initialize(resource_dispatcher_host_); appcache_dispatcher_host_->Initialize(this); dom_storage_dispatcher_host_->Init(id(), handle()); @@ -388,9 +380,6 @@ void ResourceMessageFilter::OnChannelClosing() { // Unhook us from all pending network requests so they don't get sent to a // deleted object. resource_dispatcher_host_->CancelRequestsForProcess(id()); - - // Unhook AudioRendererHost. - audio_renderer_host_->IPCChannelClosing(); } // Called on the IPC thread: @@ -402,7 +391,6 @@ bool ResourceMessageFilter::OnMessageReceived(const IPC::Message& msg) { appcache_dispatcher_host_->OnMessageReceived(msg, &msg_is_ok) || dom_storage_dispatcher_host_->OnMessageReceived(msg, &msg_is_ok) || indexed_db_dispatcher_host_->OnMessageReceived(msg) || - audio_renderer_host_->OnMessageReceived(msg, &msg_is_ok) || db_dispatcher_host_->OnMessageReceived(msg, &msg_is_ok) || mp_dispatcher->OnMessageReceived( msg, this, next_route_id_callback(), &msg_is_ok) || diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h index 38f8f15..051f95d6 100644 --- a/chrome/browser/renderer_host/resource_message_filter.h +++ b/chrome/browser/renderer_host/resource_message_filter.h @@ -32,7 +32,6 @@ #include "third_party/WebKit/WebKit/chromium/public/WebPopupType.h" class AppCacheDispatcherHost; -class AudioRendererHost; class BlobDispatcherHost; class ChromeURLRequestContext; class DatabaseDispatcherHost; @@ -98,7 +97,6 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter, // Create the filter. ResourceMessageFilter(ResourceDispatcherHost* resource_dispatcher_host, int child_id, - AudioRendererHost* audio_renderer_host, PluginService* plugin_service, printing::PrintJobManager* print_job_manager, Profile* profile, @@ -447,9 +445,6 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter, scoped_refptr<RenderWidgetHelper> render_widget_helper_; - // Object that should take care of audio related resource requests. - scoped_refptr<AudioRendererHost> audio_renderer_host_; - // Handles AppCache related messages. scoped_ptr<AppCacheDispatcherHost> appcache_dispatcher_host_; |