diff options
author | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-10-01 17:07:06 +0000 |
---|---|---|
committer | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-10-01 17:07:06 +0000 |
commit | ba780c1bb6637c1ee4880129aee2e85987a8e2d3 (patch) | |
tree | a6b14800270e1230d0300ec0399bd1683cfaaf11 /content | |
parent | f1fda807df0c92c3dc19287bf14f435d0ca4ca91 (diff) | |
download | chromium_src-ba780c1bb6637c1ee4880129aee2e85987a8e2d3.zip chromium_src-ba780c1bb6637c1ee4880129aee2e85987a8e2d3.tar.gz chromium_src-ba780c1bb6637c1ee4880129aee2e85987a8e2d3.tar.bz2 |
Make BrowserMessageFilter not derive from IPC::ChannelProxy::MessageFilter. This allows us to hide the OnMessageReceived which shouldn't be overridden from child classes, and also avoid the pattern of requiring an overridden method to have to call to the base class.
R=scherkus@chromium.org, scherkus
Review URL: https://codereview.chromium.org/24514003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226251 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
45 files changed, 322 insertions, 238 deletions
diff --git a/content/browser/appcache/appcache_dispatcher_host.cc b/content/browser/appcache/appcache_dispatcher_host.cc index b68a73d..25e33cf 100644 --- a/content/browser/appcache/appcache_dispatcher_host.cc +++ b/content/browser/appcache/appcache_dispatcher_host.cc @@ -21,7 +21,6 @@ AppCacheDispatcherHost::AppCacheDispatcherHost( } void AppCacheDispatcherHost::OnChannelConnected(int32 peer_pid) { - BrowserMessageFilter::OnChannelConnected(peer_pid); if (appcache_service_.get()) { backend_impl_.Initialize( appcache_service_.get(), &frontend_proxy_, process_id_); diff --git a/content/browser/browser_child_process_host_impl.cc b/content/browser/browser_child_process_host_impl.cc index 5cc9532..8e6bde3 100644 --- a/content/browser/browser_child_process_host_impl.cc +++ b/content/browser/browser_child_process_host_impl.cc @@ -100,9 +100,9 @@ BrowserChildProcessHostImpl::BrowserChildProcessHostImpl( data_.id = ChildProcessHostImpl::GenerateChildProcessUniqueId(); child_process_host_.reset(ChildProcessHost::Create(this)); - child_process_host_->AddFilter(new TraceMessageFilter); - child_process_host_->AddFilter(new ProfilerMessageFilter(process_type)); - child_process_host_->AddFilter(new HistogramMessageFilter()); + AddFilter(new TraceMessageFilter); + AddFilter(new ProfilerMessageFilter(process_type)); + AddFilter(new HistogramMessageFilter); g_child_process_list.Get().push_back(this); GetContentClient()->browser()->BrowserChildProcessHostCreated(this); @@ -218,6 +218,10 @@ void BrowserChildProcessHostImpl::SetTerminateChildOnShutdown( child_process_->SetTerminateChildOnShutdown(terminate_on_shutdown); } +void BrowserChildProcessHostImpl::AddFilter(BrowserMessageFilter* filter) { + child_process_host_->AddFilter(filter->GetFilter()); +} + void BrowserChildProcessHostImpl::NotifyProcessInstanceCreated( const ChildProcessData& data) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); diff --git a/content/browser/browser_child_process_host_impl.h b/content/browser/browser_child_process_host_impl.h index 7540209..6345595 100644 --- a/content/browser/browser_child_process_host_impl.h +++ b/content/browser/browser_child_process_host_impl.h @@ -21,6 +21,7 @@ namespace content { class BrowserChildProcessHostIterator; class BrowserChildProcessObserver; +class BrowserMessageFilter; // Plugins/workers and other child processes that live on the IO thread use this // class. RenderProcessHostImpl is the main exception that doesn't use this @@ -70,6 +71,9 @@ class CONTENT_EXPORT BrowserChildProcessHostImpl // shutdown. Default is to always terminate. void SetTerminateChildOnShutdown(bool terminate_on_shutdown); + // Adds an IPC message filter. + void AddFilter(BrowserMessageFilter* filter); + // Called when an instance of a particular child is created in a page. static void NotifyProcessInstanceCreated(const ChildProcessData& data); diff --git a/content/browser/dom_storage/dom_storage_message_filter.cc b/content/browser/dom_storage/dom_storage_message_filter.cc index 52aabe1..7b62529 100644 --- a/content/browser/dom_storage/dom_storage_message_filter.cc +++ b/content/browser/dom_storage/dom_storage_message_filter.cc @@ -46,8 +46,6 @@ void DOMStorageMessageFilter::UninitializeInSequence() { } void DOMStorageMessageFilter::OnFilterAdded(IPC::Channel* channel) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - BrowserMessageFilter::OnFilterAdded(channel); context_->task_runner()->PostShutdownBlockingTask( FROM_HERE, DOMStorageTaskRunner::PRIMARY_SEQUENCE, @@ -55,8 +53,6 @@ void DOMStorageMessageFilter::OnFilterAdded(IPC::Channel* channel) { } void DOMStorageMessageFilter::OnFilterRemoved() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - BrowserMessageFilter::OnFilterRemoved(); context_->task_runner()->PostShutdownBlockingTask( FROM_HERE, DOMStorageTaskRunner::PRIMARY_SEQUENCE, diff --git a/content/browser/fileapi/fileapi_message_filter.cc b/content/browser/fileapi/fileapi_message_filter.cc index ecef755..7277a3b 100644 --- a/content/browser/fileapi/fileapi_message_filter.cc +++ b/content/browser/fileapi/fileapi_message_filter.cc @@ -107,7 +107,6 @@ FileAPIMessageFilter::FileAPIMessageFilter( void FileAPIMessageFilter::OnChannelConnected(int32 peer_pid) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - BrowserMessageFilter::OnChannelConnected(peer_pid); if (request_context_getter_.get()) { DCHECK(!request_context_); @@ -124,7 +123,6 @@ void FileAPIMessageFilter::OnChannelConnected(int32 peer_pid) { void FileAPIMessageFilter::OnChannelClosing() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - BrowserMessageFilter::OnChannelClosing(); // Unregister all the blob and stream URLs that are previously registered in // this process. diff --git a/content/browser/fileapi/fileapi_message_filter_unittest.cc b/content/browser/fileapi/fileapi_message_filter_unittest.cc index 90e422c..288ffa1 100644 --- a/content/browser/fileapi/fileapi_message_filter_unittest.cc +++ b/content/browser/fileapi/fileapi_message_filter_unittest.cc @@ -76,12 +76,10 @@ class FileAPIMessageFilterTest : public testing::Test { } // Tests via OnMessageReceived(const IPC::Message&). The channel proxy calls - // this method. Since OnMessageReceived is hidden on FileAPIMessageFilter, - // we need to cast it. + // this method. bool InvokeOnMessageReceived(const IPC::Message& message) { - IPC::ChannelProxy::MessageFilter* casted_filter = - static_cast<IPC::ChannelProxy::MessageFilter*>(filter_.get()); - return casted_filter->OnMessageReceived(message); + bool message_was_ok; + return filter_->OnMessageReceived(message, &message_was_ok); } base::MessageLoop message_loop_; @@ -108,13 +106,11 @@ TEST_F(FileAPIMessageFilterTest, CloseChannelWithInflightRequest) { // Complete initialization. message_loop_.RunUntilIdle(); - IPC::ChannelProxy::MessageFilter* casted_filter = - static_cast<IPC::ChannelProxy::MessageFilter*>(filter.get()); - int request_id = 0; const GURL kUrl("filesystem:http://example.com/temporary/foo"); FileSystemHostMsg_ReadMetadata read_metadata(request_id++, kUrl); - EXPECT_TRUE(casted_filter->OnMessageReceived(read_metadata)); + bool message_was_ok; + EXPECT_TRUE(filter->OnMessageReceived(read_metadata, &message_was_ok)); // Close the filter while it has inflight request. filter->OnChannelClosing(); @@ -144,13 +140,11 @@ TEST_F(FileAPIMessageFilterTest, MultipleFilters) { // Complete initialization. message_loop_.RunUntilIdle(); - IPC::ChannelProxy::MessageFilter* casted_filter = - static_cast<IPC::ChannelProxy::MessageFilter*>(filter1.get()); - int request_id = 0; const GURL kUrl("filesystem:http://example.com/temporary/foo"); FileSystemHostMsg_ReadMetadata read_metadata(request_id++, kUrl); - EXPECT_TRUE(casted_filter->OnMessageReceived(read_metadata)); + bool message_was_ok; + EXPECT_TRUE(filter1->OnMessageReceived(read_metadata, &message_was_ok)); // Close the other filter before the request for filter1 is processed. filter2->OnChannelClosing(); @@ -251,8 +245,7 @@ TEST_F(FileAPIMessageFilterTest, BuildStreamWithSharedMemory) { // OnAppendSharedMemoryToStream passes the peer process's handle to // SharedMemory's constructor. If it's incorrect, DuplicateHandle won't work // correctly. - static_cast<IPC::ChannelProxy::MessageFilter*>( - filter_.get())->OnChannelConnected(base::Process::Current().pid()); + filter_->set_peer_pid_for_testing(base::Process::Current().pid()); StreamHostMsg_StartBuilding start_message(kUrl, kFakeContentType); EXPECT_TRUE(InvokeOnMessageReceived(start_message)); diff --git a/content/browser/histogram_message_filter.cc b/content/browser/histogram_message_filter.cc index d46b96b..f44accdc 100644 --- a/content/browser/histogram_message_filter.cc +++ b/content/browser/histogram_message_filter.cc @@ -16,10 +16,6 @@ namespace content { HistogramMessageFilter::HistogramMessageFilter() {} -void HistogramMessageFilter::OnChannelConnected(int32 peer_pid) { - BrowserMessageFilter::OnChannelConnected(peer_pid); -} - bool HistogramMessageFilter::OnMessageReceived(const IPC::Message& message, bool* message_was_ok) { bool handled = true; diff --git a/content/browser/histogram_message_filter.h b/content/browser/histogram_message_filter.h index 2118d4a..64c13cb 100644 --- a/content/browser/histogram_message_filter.h +++ b/content/browser/histogram_message_filter.h @@ -19,9 +19,6 @@ class HistogramMessageFilter : public BrowserMessageFilter { HistogramMessageFilter(); // BrowserMessageFilter implementation. - virtual void OnChannelConnected(int32 peer_pid) OVERRIDE; - - // BrowserMessageFilter implementation. virtual bool OnMessageReceived(const IPC::Message& message, bool* message_was_ok) OVERRIDE; diff --git a/content/browser/indexed_db/indexed_db_dispatcher_host.cc b/content/browser/indexed_db/indexed_db_dispatcher_host.cc index 1cbc4b8..86277ec 100644 --- a/content/browser/indexed_db/indexed_db_dispatcher_host.cc +++ b/content/browser/indexed_db/indexed_db_dispatcher_host.cc @@ -44,8 +44,6 @@ IndexedDBDispatcherHost::IndexedDBDispatcherHost( IndexedDBDispatcherHost::~IndexedDBDispatcherHost() {} void IndexedDBDispatcherHost::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); - bool success = indexed_db_context_->TaskRunner()->PostTask( FROM_HERE, base::Bind(&IndexedDBDispatcherHost::ResetDispatcherHosts, this)); diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc index 449b515..75f3c7b 100644 --- a/content/browser/loader/resource_dispatcher_host_unittest.cc +++ b/content/browser/loader/resource_dispatcher_host_unittest.cc @@ -176,7 +176,7 @@ class ForwardingFilter : public ResourceMessageFilter { base::Unretained(this))), dest_(dest), resource_context_(resource_context) { - OnChannelConnected(base::GetCurrentProcId()); + set_peer_pid_for_testing(base::GetCurrentProcId()); } // ResourceMessageFilter override @@ -1804,9 +1804,6 @@ TEST_F(ResourceDispatcherHostTest, TransferNavigationWithProcessCrash) { // Make sure we don't hold onto the ResourceMessageFilter after it is deleted. GlobalRequestID first_global_request_id(first_child_id, request_id); - const ResourceRequestInfoImpl* info = ResourceRequestInfoImpl::ForRequest( - host_.GetURLRequest(first_global_request_id)); - EXPECT_FALSE(info->filter()); // This second filter is used to emulate a second process. scoped_refptr<ForwardingFilter> second_filter = new ForwardingFilter( diff --git a/content/browser/loader/resource_message_filter.cc b/content/browser/loader/resource_message_filter.cc index c55f2d1..c3f30ff 100644 --- a/content/browser/loader/resource_message_filter.cc +++ b/content/browser/loader/resource_message_filter.cc @@ -32,8 +32,6 @@ ResourceMessageFilter::~ResourceMessageFilter() { } void ResourceMessageFilter::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); - // Unhook us from all pending network requests so they don't get sent to a // deleted object. ResourceDispatcherHostImpl::Get()->CancelRequestsForProcess(child_id_); diff --git a/content/browser/plugin_process_host.cc b/content/browser/plugin_process_host.cc index 86d3134..e22b32f 100644 --- a/content/browser/plugin_process_host.cc +++ b/content/browser/plugin_process_host.cc @@ -265,7 +265,7 @@ bool PluginProcessHost::Init(const WebPluginInfo& info) { ResourceMessageFilter* resource_message_filter = new ResourceMessageFilter( process_->GetData().id, PROCESS_TYPE_PLUGIN, NULL, NULL, NULL, get_contexts_callback); - process_->GetHost()->AddFilter(resource_message_filter); + process_->AddFilter(resource_message_filter); return true; } @@ -275,10 +275,6 @@ void PluginProcessHost::ForceShutdown() { process_->ForceShutdown(); } -void PluginProcessHost::AddFilter(IPC::ChannelProxy::MessageFilter* filter) { - process_->GetHost()->AddFilter(filter); -} - bool PluginProcessHost::OnMessageReceived(const IPC::Message& msg) { bool handled = true; IPC_BEGIN_MESSAGE_MAP(PluginProcessHost, msg) diff --git a/content/browser/plugin_process_host.h b/content/browser/plugin_process_host.h index 75c667d..0b60062 100644 --- a/content/browser/plugin_process_host.h +++ b/content/browser/plugin_process_host.h @@ -120,9 +120,6 @@ class CONTENT_EXPORT PluginProcessHost : public BrowserChildProcessHostDelegate, void AddWindow(HWND window); #endif - // Adds an IPC message filter. A reference will be kept to the filter. - void AddFilter(IPC::ChannelProxy::MessageFilter* filter); - private: // Sends a message to the plugin process to request creation of a new channel // for the given mime type. diff --git a/content/browser/ppapi_plugin_process_host.cc b/content/browser/ppapi_plugin_process_host.cc index ecbbfd6..e42d5499 100644 --- a/content/browser/ppapi_plugin_process_host.cc +++ b/content/browser/ppapi_plugin_process_host.cc @@ -218,7 +218,7 @@ PpapiPluginProcessHost::PpapiPluginProcessHost( false)); filter_ = new PepperMessageFilter(); - process_->GetHost()->AddFilter(filter_.get()); + process_->AddFilter(filter_.get()); process_->GetHost()->AddFilter(host_impl_->message_filter().get()); GetContentClient()->browser()->DidCreatePpapiPlugin(host_impl_.get()); diff --git a/content/browser/profiler_message_filter.cc b/content/browser/profiler_message_filter.cc index cb30f25..adea8e7 100644 --- a/content/browser/profiler_message_filter.cc +++ b/content/browser/profiler_message_filter.cc @@ -16,8 +16,6 @@ ProfilerMessageFilter::ProfilerMessageFilter(int process_type) } void ProfilerMessageFilter::OnChannelConnected(int32 peer_pid) { - BrowserMessageFilter::OnChannelConnected(peer_pid); - tracked_objects::ThreadData::Status status = tracked_objects::ThreadData::status(); Send(new ChildProcessMsg_SetProfilerStatus(status)); diff --git a/content/browser/renderer_host/DEPS b/content/browser/renderer_host/DEPS index 0848d36..a0f0ecf 100644 --- a/content/browser/renderer_host/DEPS +++ b/content/browser/renderer_host/DEPS @@ -9,6 +9,7 @@ include_rules = [ # delegate interfaces. "-content/browser/web_contents", "-content/public/browser/web_contents.h", + "-content/public/browser/web_contents_delegate.h", "-content/public/browser/web_contents_view.h", ] diff --git a/content/browser/renderer_host/database_message_filter.cc b/content/browser/renderer_host/database_message_filter.cc index 38cc49a..d41be65 100644 --- a/content/browser/renderer_host/database_message_filter.cc +++ b/content/browser/renderer_host/database_message_filter.cc @@ -47,7 +47,6 @@ DatabaseMessageFilter::DatabaseMessageFilter( } void DatabaseMessageFilter::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); if (observer_added_) { observer_added_ = false; BrowserThread::PostTask( diff --git a/content/browser/renderer_host/media/audio_input_renderer_host.cc b/content/browser/renderer_host/media/audio_input_renderer_host.cc index 1c02109..a04a8cc 100644 --- a/content/browser/renderer_host/media/audio_input_renderer_host.cc +++ b/content/browser/renderer_host/media/audio_input_renderer_host.cc @@ -63,8 +63,6 @@ AudioInputRendererHost::~AudioInputRendererHost() { } void AudioInputRendererHost::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); - // Since the IPC channel is gone, close all requested audio streams. DeleteEntries(); } diff --git a/content/browser/renderer_host/media/audio_input_renderer_host.h b/content/browser/renderer_host/media/audio_input_renderer_host.h index 5df1fc6..122df13 100644 --- a/content/browser/renderer_host/media/audio_input_renderer_host.h +++ b/content/browser/renderer_host/media/audio_input_renderer_host.h @@ -79,6 +79,7 @@ class CONTENT_EXPORT AudioInputRendererHost private: // TODO(henrika): extend test suite (compare AudioRenderHost) friend class BrowserThread; + friend class TestAudioInputRendererHost; friend class base::DeleteHelper<AudioInputRendererHost>; struct AudioEntry; diff --git a/content/browser/renderer_host/media/audio_renderer_host.cc b/content/browser/renderer_host/media/audio_renderer_host.cc index c09dc6c..1181fbe 100644 --- a/content/browser/renderer_host/media/audio_renderer_host.cc +++ b/content/browser/renderer_host/media/audio_renderer_host.cc @@ -128,10 +128,6 @@ AudioRendererHost::~AudioRendererHost() { } void AudioRendererHost::OnChannelClosing() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - BrowserMessageFilter::OnChannelClosing(); - // Since the IPC channel is gone, close all requested audio streams. while (!audio_entries_.empty()) { // Note: OnCloseStream() removes the entries from audio_entries_. diff --git a/content/browser/renderer_host/media/audio_renderer_host.h b/content/browser/renderer_host/media/audio_renderer_host.h index 47dbee9..486d55f 100644 --- a/content/browser/renderer_host/media/audio_renderer_host.h +++ b/content/browser/renderer_host/media/audio_renderer_host.h @@ -83,6 +83,7 @@ class CONTENT_EXPORT AudioRendererHost : public BrowserMessageFilter { friend class BrowserThread; friend class base::DeleteHelper<AudioRendererHost>; friend class MockAudioRendererHost; + friend class TestAudioRendererHost; FRIEND_TEST_ALL_PREFIXES(AudioRendererHostTest, CreateMockStream); FRIEND_TEST_ALL_PREFIXES(AudioRendererHostTest, MockStreamDataConversation); diff --git a/content/browser/renderer_host/media/audio_renderer_host_unittest.cc b/content/browser/renderer_host/media/audio_renderer_host_unittest.cc index 26c0963..ac837c2 100644 --- a/content/browser/renderer_host/media/audio_renderer_host_unittest.cc +++ b/content/browser/renderer_host/media/audio_renderer_host_unittest.cc @@ -179,7 +179,7 @@ class AudioRendererHostTest : public testing::Test { media_stream_manager_.get()); // Simulate IPC channel connected. - host_->OnChannelConnected(base::GetCurrentProcId()); + host_->set_peer_pid_for_testing(base::GetCurrentProcId()); } virtual void TearDown() { diff --git a/content/browser/renderer_host/media/device_request_message_filter.cc b/content/browser/renderer_host/media/device_request_message_filter.cc index 1ebfa1a..56fdff4 100644 --- a/content/browser/renderer_host/media/device_request_message_filter.cc +++ b/content/browser/renderer_host/media/device_request_message_filter.cc @@ -146,8 +146,6 @@ bool DeviceRequestMessageFilter::OnMessageReceived(const IPC::Message& message, } void DeviceRequestMessageFilter::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); - // Since the IPC channel is gone, cancel outstanding device requests. for (DeviceRequestList::iterator it = requests_.begin(); it != requests_.end(); diff --git a/content/browser/renderer_host/media/media_stream_dispatcher_host.cc b/content/browser/renderer_host/media/media_stream_dispatcher_host.cc index ebc4d89..cb0af34 100644 --- a/content/browser/renderer_host/media/media_stream_dispatcher_host.cc +++ b/content/browser/renderer_host/media/media_stream_dispatcher_host.cc @@ -125,7 +125,6 @@ bool MediaStreamDispatcherHost::OnMessageReceived( } void MediaStreamDispatcherHost::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); DVLOG(1) << "MediaStreamDispatcherHost::OnChannelClosing"; // Since the IPC channel is gone, close all requesting/requested streams. diff --git a/content/browser/renderer_host/media/midi_host.cc b/content/browser/renderer_host/media/midi_host.cc index 0467404..934d5a2 100644 --- a/content/browser/renderer_host/media/midi_host.cc +++ b/content/browser/renderer_host/media/midi_host.cc @@ -46,12 +46,6 @@ MIDIHost::~MIDIHost() { midi_manager_->EndSession(this); } -void MIDIHost::OnChannelClosing() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - - BrowserMessageFilter::OnChannelClosing(); -} - void MIDIHost::OnDestruct() const { BrowserThread::DeleteOnIOThread::Destruct(this); } diff --git a/content/browser/renderer_host/media/midi_host.h b/content/browser/renderer_host/media/midi_host.h index 944325d..e3b9df1 100644 --- a/content/browser/renderer_host/media/midi_host.h +++ b/content/browser/renderer_host/media/midi_host.h @@ -28,7 +28,6 @@ class CONTENT_EXPORT MIDIHost MIDIHost(int renderer_process_id, media::MIDIManager* midi_manager); // BrowserMessageFilter implementation. - virtual void OnChannelClosing() OVERRIDE; virtual void OnDestruct() const OVERRIDE; virtual bool OnMessageReceived(const IPC::Message& message, bool* message_was_ok) OVERRIDE; diff --git a/content/browser/renderer_host/media/video_capture_host.cc b/content/browser/renderer_host/media/video_capture_host.cc index 7ed77ae..1c22199 100644 --- a/content/browser/renderer_host/media/video_capture_host.cc +++ b/content/browser/renderer_host/media/video_capture_host.cc @@ -20,8 +20,6 @@ VideoCaptureHost::VideoCaptureHost(MediaStreamManager* media_stream_manager) VideoCaptureHost::~VideoCaptureHost() {} void VideoCaptureHost::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); - // Since the IPC channel is gone, close all requested VideoCaptureDevices. for (EntryMap::iterator it = entries_.begin(); it != entries_.end(); it++) { const base::WeakPtr<VideoCaptureController>& controller = it->second; diff --git a/content/browser/renderer_host/p2p/socket_dispatcher_host.cc b/content/browser/renderer_host/p2p/socket_dispatcher_host.cc index d1a60b4..21463a1 100644 --- a/content/browser/renderer_host/p2p/socket_dispatcher_host.cc +++ b/content/browser/renderer_host/p2p/socket_dispatcher_host.cc @@ -94,8 +94,6 @@ P2PSocketDispatcherHost::P2PSocketDispatcherHost( } void P2PSocketDispatcherHost::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); - // Since the IPC channel is gone, close pending connections. STLDeleteContainerPairSecondPointers(sockets_.begin(), sockets_.end()); sockets_.clear(); diff --git a/content/browser/renderer_host/pepper/browser_ppapi_host_impl.cc b/content/browser/renderer_host/pepper/browser_ppapi_host_impl.cc index db3f8ef..8fca534 100644 --- a/content/browser/renderer_host/pepper/browser_ppapi_host_impl.cc +++ b/content/browser/renderer_host/pepper/browser_ppapi_host_impl.cc @@ -30,9 +30,9 @@ BrowserPpapiHost* BrowserPpapiHost::CreateExternalPluginProcess( scoped_refptr<PepperMessageFilter> pepper_message_filter( new PepperMessageFilter()); - channel->AddFilter(pepper_message_filter); - channel->AddFilter(browser_ppapi_host->message_filter().get()); - channel->AddFilter(new TraceMessageFilter()); + channel->AddFilter(pepper_message_filter->GetFilter()); + channel->AddFilter(browser_ppapi_host->message_filter()); + channel->AddFilter((new TraceMessageFilter())->GetFilter()); return browser_ppapi_host; } diff --git a/content/browser/renderer_host/render_message_filter.cc b/content/browser/renderer_host/render_message_filter.cc index 29e1e1f..e78ea69 100644 --- a/content/browser/renderer_host/render_message_filter.cc +++ b/content/browser/renderer_host/render_message_filter.cc @@ -321,7 +321,6 @@ RenderMessageFilter::~RenderMessageFilter() { } void RenderMessageFilter::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); #if defined(ENABLE_PLUGINS) for (std::set<OpenChannelToNpapiPluginCallback*>::iterator it = plugin_host_clients_.begin(); it != plugin_host_clients_.end(); ++it) { @@ -342,7 +341,6 @@ void RenderMessageFilter::OnChannelClosing() { } void RenderMessageFilter::OnChannelConnected(int32 peer_id) { - BrowserMessageFilter::OnChannelConnected(peer_id); base::ProcessHandle handle = PeerHandle(); #if defined(OS_MACOSX) process_metrics_.reset(base::ProcessMetrics::CreateProcessMetrics(handle, diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc index 6e6a92e..0a2dcb5 100644 --- a/content/browser/renderer_host/render_process_host_impl.cc +++ b/content/browser/renderer_host/render_process_host_impl.cc @@ -531,7 +531,7 @@ bool RenderProcessHostImpl::Init() { void RenderProcessHostImpl::CreateMessageFilters() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - channel_->AddFilter(new ResourceSchedulerFilter(GetID())); + AddFilter(new ResourceSchedulerFilter(GetID())); MediaInternals* media_internals = MediaInternals::GetInstance();; media::AudioManager* audio_manager = BrowserMainLoop::GetInstance()->audio_manager(); @@ -540,7 +540,7 @@ void RenderProcessHostImpl::CreateMessageFilters() { if (supports_browser_plugin_) { scoped_refptr<BrowserPluginMessageFilter> bp_message_filter( new BrowserPluginMessageFilter(GetID(), IsGuest())); - channel_->AddFilter(bp_message_filter.get()); + AddFilter(bp_message_filter.get()); } scoped_refptr<RenderMessageFilter> render_message_filter( @@ -558,7 +558,7 @@ void RenderProcessHostImpl::CreateMessageFilters() { audio_manager, media_internals, storage_partition_impl_->GetDOMStorageContext())); - channel_->AddFilter(render_message_filter.get()); + AddFilter(render_message_filter.get()); BrowserContext* browser_context = GetBrowserContext(); ResourceContext* resource_context = browser_context->GetResourceContext(); @@ -578,86 +578,86 @@ void RenderProcessHostImpl::CreateMessageFilters() { storage_partition_impl_->GetFileSystemContext(), get_contexts_callback); - channel_->AddFilter(resource_message_filter); + AddFilter(resource_message_filter); MediaStreamManager* media_stream_manager = BrowserMainLoop::GetInstance()->media_stream_manager(); - channel_->AddFilter(new AudioInputRendererHost( + AddFilter(new AudioInputRendererHost( audio_manager, media_stream_manager, BrowserMainLoop::GetInstance()->audio_mirroring_manager(), BrowserMainLoop::GetInstance()->user_input_monitor())); - channel_->AddFilter(new AudioRendererHost( + AddFilter(new AudioRendererHost( GetID(), audio_manager, BrowserMainLoop::GetInstance()->audio_mirroring_manager(), media_internals, media_stream_manager)); - channel_->AddFilter( + AddFilter( new MIDIHost(GetID(), BrowserMainLoop::GetInstance()->midi_manager())); - channel_->AddFilter(new MIDIDispatcherHost(GetID(), browser_context)); - channel_->AddFilter(new VideoCaptureHost(media_stream_manager)); - channel_->AddFilter(new AppCacheDispatcherHost( + AddFilter(new MIDIDispatcherHost(GetID(), browser_context)); + AddFilter(new VideoCaptureHost(media_stream_manager)); + AddFilter(new AppCacheDispatcherHost( storage_partition_impl_->GetAppCacheService(), GetID())); - channel_->AddFilter(new ClipboardMessageFilter); - channel_->AddFilter(new DOMStorageMessageFilter( + AddFilter(new ClipboardMessageFilter); + AddFilter(new DOMStorageMessageFilter( GetID(), storage_partition_impl_->GetDOMStorageContext())); - channel_->AddFilter(new IndexedDBDispatcherHost( + AddFilter(new IndexedDBDispatcherHost( GetID(), storage_partition_impl_->GetIndexedDBContext())); - channel_->AddFilter(new ServiceWorkerDispatcherHost( + AddFilter(new ServiceWorkerDispatcherHost( storage_partition_impl_->GetServiceWorkerContext())); if (IsGuest()) { if (!g_browser_plugin_geolocation_context.Get().get()) { g_browser_plugin_geolocation_context.Get() = new BrowserPluginGeolocationPermissionContext(); } - channel_->AddFilter(GeolocationDispatcherHost::New( + AddFilter(GeolocationDispatcherHost::New( GetID(), g_browser_plugin_geolocation_context.Get().get())); } else { - channel_->AddFilter(GeolocationDispatcherHost::New( + AddFilter(GeolocationDispatcherHost::New( GetID(), browser_context->GetGeolocationPermissionContext())); } gpu_message_filter_ = new GpuMessageFilter(GetID(), widget_helper_.get()); - channel_->AddFilter(gpu_message_filter_); + AddFilter(gpu_message_filter_); #if defined(ENABLE_WEBRTC) - channel_->AddFilter(new WebRTCIdentityServiceHost( + AddFilter(new WebRTCIdentityServiceHost( GetID(), storage_partition_impl_->GetWebRTCIdentityStore())); peer_connection_tracker_host_ = new PeerConnectionTrackerHost(GetID()); - channel_->AddFilter(peer_connection_tracker_host_.get()); - channel_->AddFilter(new MediaStreamDispatcherHost( + AddFilter(peer_connection_tracker_host_.get()); + AddFilter(new MediaStreamDispatcherHost( GetID(), media_stream_manager)); - channel_->AddFilter( + AddFilter( new DeviceRequestMessageFilter(resource_context, media_stream_manager)); #endif #if defined(ENABLE_PLUGINS) - channel_->AddFilter(new PepperRendererConnection(GetID())); + AddFilter(new PepperRendererConnection(GetID())); #endif #if defined(ENABLE_INPUT_SPEECH) - channel_->AddFilter(new InputTagSpeechDispatcherHost( + AddFilter(new InputTagSpeechDispatcherHost( IsGuest(), GetID(), storage_partition_impl_->GetURLRequestContext())); #endif - channel_->AddFilter(new SpeechRecognitionDispatcherHost( + AddFilter(new SpeechRecognitionDispatcherHost( GetID(), storage_partition_impl_->GetURLRequestContext())); - channel_->AddFilter(new FileAPIMessageFilter( + AddFilter(new FileAPIMessageFilter( GetID(), storage_partition_impl_->GetURLRequestContext(), storage_partition_impl_->GetFileSystemContext(), ChromeBlobStorageContext::GetFor(browser_context), StreamContext::GetFor(browser_context))); - channel_->AddFilter(new OrientationMessageFilter()); - channel_->AddFilter(new FileUtilitiesMessageFilter(GetID())); - channel_->AddFilter(new MimeRegistryMessageFilter()); - channel_->AddFilter(new DatabaseMessageFilter( + AddFilter(new OrientationMessageFilter()); + AddFilter(new FileUtilitiesMessageFilter(GetID())); + AddFilter(new MimeRegistryMessageFilter()); + AddFilter(new DatabaseMessageFilter( storage_partition_impl_->GetDatabaseTracker())); #if defined(OS_MACOSX) - channel_->AddFilter(new TextInputClientMessageFilter(GetID())); + AddFilter(new TextInputClientMessageFilter(GetID())); #elif defined(OS_WIN) channel_->AddFilter(new FontCacheDispatcher()); #elif defined(OS_ANDROID) browser_demuxer_android_ = new BrowserDemuxerAndroid(); - channel_->AddFilter(browser_demuxer_android_); + AddFilter(browser_demuxer_android_); #endif SocketStreamDispatcherHost::GetRequestContextCallback @@ -668,9 +668,9 @@ void RenderProcessHostImpl::CreateMessageFilters() { SocketStreamDispatcherHost* socket_stream_dispatcher_host = new SocketStreamDispatcherHost( GetID(), request_context_callback, resource_context); - channel_->AddFilter(socket_stream_dispatcher_host); + AddFilter(socket_stream_dispatcher_host); - channel_->AddFilter(new WorkerMessageFilter( + AddFilter(new WorkerMessageFilter( GetID(), resource_context, WorkerStoragePartition( @@ -685,30 +685,30 @@ void RenderProcessHostImpl::CreateMessageFilters() { base::Unretained(widget_helper_.get())))); #if defined(ENABLE_WEBRTC) - channel_->AddFilter(new P2PSocketDispatcherHost( + AddFilter(new P2PSocketDispatcherHost( resource_context, browser_context->GetRequestContextForRenderProcess(GetID()))); #endif - channel_->AddFilter(new TraceMessageFilter()); - channel_->AddFilter(new ResolveProxyMsgHelper( + AddFilter(new TraceMessageFilter()); + AddFilter(new ResolveProxyMsgHelper( browser_context->GetRequestContextForRenderProcess(GetID()))); - channel_->AddFilter(new QuotaDispatcherHost( + AddFilter(new QuotaDispatcherHost( GetID(), storage_partition_impl_->GetQuotaManager(), GetContentClient()->browser()->CreateQuotaPermissionContext())); - channel_->AddFilter(new GamepadBrowserMessageFilter()); - channel_->AddFilter(new DeviceMotionMessageFilter()); - channel_->AddFilter(new DeviceOrientationMessageFilter()); - channel_->AddFilter(new ProfilerMessageFilter(PROCESS_TYPE_RENDERER)); - channel_->AddFilter(new HistogramMessageFilter()); + AddFilter(new GamepadBrowserMessageFilter()); + AddFilter(new DeviceMotionMessageFilter()); + AddFilter(new DeviceOrientationMessageFilter()); + AddFilter(new ProfilerMessageFilter(PROCESS_TYPE_RENDERER)); + AddFilter(new HistogramMessageFilter()); #if defined(USE_TCMALLOC) && (defined(OS_LINUX) || defined(OS_ANDROID)) if (CommandLine::ForCurrentProcess()->HasSwitch( switches::kEnableMemoryBenchmarking)) - channel_->AddFilter(new MemoryBenchmarkMessageFilter()); + AddFilter(new MemoryBenchmarkMessageFilter()); #endif #if defined(OS_ANDROID) - channel_->AddFilter(new VibrationMessageFilter()); + AddFilter(new VibrationMessageFilter()); #endif } @@ -1377,6 +1377,10 @@ IPC::ChannelProxy* RenderProcessHostImpl::GetChannel() { return channel_.get(); } +void RenderProcessHostImpl::AddFilter(BrowserMessageFilter* filter) { + channel_->AddFilter(filter->GetFilter()); +} + bool RenderProcessHostImpl::FastShutdownForPageCount(size_t count) { if (static_cast<size_t>(GetActiveViewCount()) == count) return FastShutdownIfPossible(); diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h index 0e9ae1b..567b629b 100644 --- a/content/browser/renderer_host/render_process_host_impl.h +++ b/content/browser/renderer_host/render_process_host_impl.h @@ -107,6 +107,7 @@ class CONTENT_EXPORT RenderProcessHostImpl virtual void SetSuddenTerminationAllowed(bool enabled) OVERRIDE; virtual bool SuddenTerminationAllowed() const OVERRIDE; virtual IPC::ChannelProxy* GetChannel() OVERRIDE; + virtual void AddFilter(BrowserMessageFilter* filter) OVERRIDE; virtual bool FastShutdownForPageCount(size_t count) OVERRIDE; virtual bool FastShutdownStarted() const OVERRIDE; virtual base::TimeDelta GetChildProcessIdleTime() const OVERRIDE; diff --git a/content/browser/resolve_proxy_msg_helper_unittest.cc b/content/browser/resolve_proxy_msg_helper_unittest.cc index 628df86..91f0243 100644 --- a/content/browser/resolve_proxy_msg_helper_unittest.cc +++ b/content/browser/resolve_proxy_msg_helper_unittest.cc @@ -27,6 +27,25 @@ class MockProxyConfigService : public net::ProxyConfigService { } }; +class TestResolveProxyMsgHelper : public ResolveProxyMsgHelper { + public: + TestResolveProxyMsgHelper( + net::ProxyService* proxy_service, + IPC::Listener* listener) + : ResolveProxyMsgHelper(proxy_service), + listener_(listener) {} + virtual bool Send(IPC::Message* message) OVERRIDE { + listener_->OnMessageReceived(*message); + delete message; + return true; + } + + protected: + virtual ~TestResolveProxyMsgHelper() {} + + IPC::Listener* listener_; +}; + class ResolveProxyMsgHelperTest : public testing::Test, public IPC::Listener { public: struct PendingResult { @@ -43,11 +62,10 @@ class ResolveProxyMsgHelperTest : public testing::Test, public IPC::Listener { : resolver_(new net::MockAsyncProxyResolver), service_( new net::ProxyService(new MockProxyConfigService, resolver_, NULL)), - helper_(new ResolveProxyMsgHelper(service_.get())), + helper_(new TestResolveProxyMsgHelper(service_.get(), this)), message_loop_(base::MessageLoop::TYPE_IO), io_thread_(BrowserThread::IO, &message_loop_) { test_sink_.AddFilter(this); - helper_->OnFilterAdded(&test_sink_); } protected: diff --git a/content/browser/tracing/trace_message_filter.cc b/content/browser/tracing/trace_message_filter.cc index e75d786..b77c908 100644 --- a/content/browser/tracing/trace_message_filter.cc +++ b/content/browser/tracing/trace_message_filter.cc @@ -15,15 +15,7 @@ TraceMessageFilter::TraceMessageFilter() : is_awaiting_buffer_percent_full_ack_(false) { } -void TraceMessageFilter::OnFilterAdded(IPC::Channel* channel) { - // Always on IO thread (BrowserMessageFilter guarantee). - BrowserMessageFilter::OnFilterAdded(channel); -} - void TraceMessageFilter::OnChannelClosing() { - // Always on IO thread (BrowserMessageFilter guarantee). - BrowserMessageFilter::OnChannelClosing(); - if (has_child_) { if (is_awaiting_end_ack_) OnEndTracingAck(std::vector<std::string>()); diff --git a/content/browser/tracing/trace_message_filter.h b/content/browser/tracing/trace_message_filter.h index 41fd017..9f97184 100644 --- a/content/browser/tracing/trace_message_filter.h +++ b/content/browser/tracing/trace_message_filter.h @@ -20,9 +20,6 @@ class TraceMessageFilter : public BrowserMessageFilter { public: TraceMessageFilter(); - // BrowserMessageFilter override. - virtual void OnFilterAdded(IPC::Channel* channel) OVERRIDE; - // BrowserMessageFilter implementation. virtual void OnChannelClosing() OVERRIDE; virtual bool OnMessageReceived(const IPC::Message& message, diff --git a/content/browser/worker_host/worker_message_filter.cc b/content/browser/worker_host/worker_message_filter.cc index faedbdc..6631de3 100644 --- a/content/browser/worker_host/worker_message_filter.cc +++ b/content/browser/worker_host/worker_message_filter.cc @@ -30,8 +30,6 @@ WorkerMessageFilter::~WorkerMessageFilter() { } void WorkerMessageFilter::OnChannelClosing() { - BrowserMessageFilter::OnChannelClosing(); - MessagePortService::GetInstance()->OnWorkerMessageFilterClosing(this); WorkerServiceImpl::GetInstance()->OnWorkerMessageFilterClosing(this); } diff --git a/content/browser/worker_host/worker_process_host.cc b/content/browser/worker_host/worker_process_host.cc index a228f9c..7145af3 100644 --- a/content/browser/worker_host/worker_process_host.cc +++ b/content/browser/worker_host/worker_process_host.cc @@ -221,27 +221,26 @@ void WorkerProcessHost::CreateMessageFilters(int render_process_id) { blob_storage_context, partition_.filesystem_context(), get_contexts_callback); - process_->GetHost()->AddFilter(resource_message_filter); + process_->AddFilter(resource_message_filter); worker_message_filter_ = new WorkerMessageFilter( render_process_id, resource_context_, partition_, base::Bind(&WorkerServiceImpl::next_worker_route_id, base::Unretained(WorkerServiceImpl::GetInstance()))); - process_->GetHost()->AddFilter(worker_message_filter_.get()); - process_->GetHost()->AddFilter(new AppCacheDispatcherHost( + process_->AddFilter(worker_message_filter_.get()); + process_->AddFilter(new AppCacheDispatcherHost( partition_.appcache_service(), process_->GetData().id)); - process_->GetHost()->AddFilter(new FileAPIMessageFilter( + process_->AddFilter(new FileAPIMessageFilter( process_->GetData().id, url_request_context, partition_.filesystem_context(), blob_storage_context, stream_context)); - process_->GetHost()->AddFilter(new FileUtilitiesMessageFilter( + process_->AddFilter(new FileUtilitiesMessageFilter( process_->GetData().id)); - process_->GetHost()->AddFilter(new MimeRegistryMessageFilter()); - process_->GetHost()->AddFilter( - new DatabaseMessageFilter(partition_.database_tracker())); - process_->GetHost()->AddFilter(new QuotaDispatcherHost( + process_->AddFilter(new MimeRegistryMessageFilter()); + process_->AddFilter(new DatabaseMessageFilter(partition_.database_tracker())); + process_->AddFilter(new QuotaDispatcherHost( process_->GetData().id, partition_.quota_manager(), GetContentClient()->browser()->CreateQuotaPermissionContext())); @@ -257,10 +256,9 @@ void WorkerProcessHost::CreateMessageFilters(int render_process_id) { request_context_callback, resource_context_); socket_stream_dispatcher_host_ = socket_stream_dispatcher_host; - process_->GetHost()->AddFilter(socket_stream_dispatcher_host); - process_->GetHost()->AddFilter( - new WorkerDevToolsMessageFilter(process_->GetData().id)); - process_->GetHost()->AddFilter(new IndexedDBDispatcherHost( + process_->AddFilter(socket_stream_dispatcher_host); + process_->AddFilter(new WorkerDevToolsMessageFilter(process_->GetData().id)); + process_->AddFilter(new IndexedDBDispatcherHost( process_->GetData().id, partition_.indexed_db_context())); } diff --git a/content/public/browser/browser_message_filter.cc b/content/public/browser/browser_message_filter.cc index 53c0f87..bbc6b13 100644 --- a/content/public/browser/browser_message_filter.cc +++ b/content/public/browser/browser_message_filter.cc @@ -18,52 +18,87 @@ using content::BrowserMessageFilter; namespace content { -BrowserMessageFilter::BrowserMessageFilter() - : channel_(NULL), -#if defined(OS_WIN) - peer_handle_(base::kNullProcessHandle), -#endif - peer_pid_(base::kNullProcessId) { -} +class BrowserMessageFilter::Internal : public IPC::ChannelProxy::MessageFilter { + public: + explicit Internal(BrowserMessageFilter* filter) : filter_(filter) {} -void BrowserMessageFilter::OnFilterAdded(IPC::Channel* channel) { - channel_ = channel; -} + private: + virtual ~Internal() {} -void BrowserMessageFilter::OnChannelClosing() { - channel_ = NULL; -} + // IPC::ChannelProxy::MessageFilter implementation: + virtual void OnFilterAdded(IPC::Channel* channel) OVERRIDE { + filter_->channel_ = channel; + filter_->OnFilterAdded(channel); + } -void BrowserMessageFilter::OnChannelConnected(int32 peer_pid) { - peer_pid_ = peer_pid; -} + virtual void OnFilterRemoved() OVERRIDE { + filter_->OnFilterRemoved(); + } + + virtual void OnChannelClosing() OVERRIDE { + filter_->channel_ = NULL; + filter_->OnChannelClosing(); + } + + virtual void OnChannelConnected(int32 peer_pid) OVERRIDE { + filter_->peer_pid_ = peer_pid; + filter_->OnChannelConnected(peer_pid); + } -bool BrowserMessageFilter::OnMessageReceived(const IPC::Message& message) { - BrowserThread::ID thread = BrowserThread::IO; - OverrideThreadForMessage(message, &thread); - - if (thread == BrowserThread::IO) { - scoped_refptr<base::TaskRunner> runner = - OverrideTaskRunnerForMessage(message); - if (runner.get()) { - runner->PostTask( - FROM_HERE, - base::Bind(base::IgnoreResult(&BrowserMessageFilter::DispatchMessage), - this, - message)); + virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE { + BrowserThread::ID thread = BrowserThread::IO; + filter_->OverrideThreadForMessage(message, &thread); + + if (thread == BrowserThread::IO) { + scoped_refptr<base::TaskRunner> runner = + filter_->OverrideTaskRunnerForMessage(message); + if (runner.get()) { + runner->PostTask( + FROM_HERE, + base::Bind( + base::IgnoreResult(&Internal::DispatchMessage), this, message)); + return true; + } + return DispatchMessage(message); + } + + if (thread == BrowserThread::UI && + !BrowserMessageFilter::CheckCanDispatchOnUI(message, filter_)) { return true; } - return DispatchMessage(message); - } - if (thread == BrowserThread::UI && !CheckCanDispatchOnUI(message, this)) + BrowserThread::PostTask( + thread, FROM_HERE, + base::Bind( + base::IgnoreResult(&Internal::DispatchMessage), this, message)); return true; + } - BrowserThread::PostTask( - thread, FROM_HERE, - base::Bind(base::IgnoreResult(&BrowserMessageFilter::DispatchMessage), - this, message)); - return true; + // Dispatches a message to the derived class. + bool DispatchMessage(const IPC::Message& message) { + bool message_was_ok = true; + bool rv = filter_->OnMessageReceived(message, &message_was_ok); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) || rv) << + "Must handle messages that were dispatched to another thread!"; + if (!message_was_ok) { + content::RecordAction(UserMetricsAction("BadMessageTerminate_BMF")); + filter_->BadMessageReceived(); + } + + return rv; + } + + scoped_refptr<BrowserMessageFilter> filter_; + + DISALLOW_COPY_AND_ASSIGN(Internal); +}; + +BrowserMessageFilter::BrowserMessageFilter() + : internal_(NULL), channel_(NULL), +#if defined(OS_WIN) + peer_handle_(base::kNullProcessHandle), +#endif + peer_pid_(base::kNullProcessId) { } base::ProcessHandle BrowserMessageFilter::PeerHandle() { @@ -80,6 +115,11 @@ base::ProcessHandle BrowserMessageFilter::PeerHandle() { #endif } + +void BrowserMessageFilter::OnDestruct() const { + delete this; +} + bool BrowserMessageFilter::Send(IPC::Message* message) { if (message->is_sync()) { // We don't support sending synchronous messages from the browser. If we @@ -106,10 +146,6 @@ bool BrowserMessageFilter::Send(IPC::Message* message) { return false; } -void BrowserMessageFilter::OverrideThreadForMessage(const IPC::Message& message, - BrowserThread::ID* thread) { -} - base::TaskRunner* BrowserMessageFilter::OverrideTaskRunnerForMessage( const IPC::Message& message) { return NULL; @@ -152,17 +188,12 @@ BrowserMessageFilter::~BrowserMessageFilter() { #endif } -bool BrowserMessageFilter::DispatchMessage(const IPC::Message& message) { - bool message_was_ok = true; - bool rv = OnMessageReceived(message, &message_was_ok); - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) || rv) << - "Must handle messages that were dispatched to another thread!"; - if (!message_was_ok) { - content::RecordAction(UserMetricsAction("BadMessageTerminate_BMF")); - BadMessageReceived(); - } - - return rv; +IPC::ChannelProxy::MessageFilter* BrowserMessageFilter::GetFilter() { + // We create this on demand so that if a filter is used in a unit test but + // never attached to a channel, we don't leak Internal and this; + DCHECK(!internal_) << "Should only be called once."; + internal_ = new Internal(this); + return internal_; } } // namespace content diff --git a/content/public/browser/browser_message_filter.h b/content/public/browser/browser_message_filter.h index c997cc6..ad1f069 100644 --- a/content/public/browser/browser_message_filter.h +++ b/content/public/browser/browser_message_filter.h @@ -5,6 +5,7 @@ #ifndef CONTENT_PUBLIC_BROWSER_BROWSER_MESSAGE_FILTER_H_ #define CONTENT_PUBLIC_BROWSER_BROWSER_MESSAGE_FILTER_H_ +#include "base/memory/ref_counted.h" #include "base/process/process.h" #include "content/common/content_export.h" #include "content/public/browser/browser_thread.h" @@ -19,22 +20,28 @@ class TaskRunner; } namespace content { +struct BrowserMessageFilterTraits; // Base class for message filters in the browser process. You can receive and // send messages on any thread. class CONTENT_EXPORT BrowserMessageFilter - : public IPC::ChannelProxy::MessageFilter, + : public base::RefCountedThreadSafe< + BrowserMessageFilter, BrowserMessageFilterTraits>, public IPC::Sender { public: BrowserMessageFilter(); - // IPC::ChannelProxy::MessageFilter methods. If you override them, make sure - // to call them as well. These are always called on the IO thread. - virtual void OnFilterAdded(IPC::Channel* channel) OVERRIDE; - virtual void OnChannelClosing() OVERRIDE; - virtual void OnChannelConnected(int32 peer_pid) OVERRIDE; - // DON'T OVERRIDE THIS! Override the other version below. - virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; + // These match the corresponding IPC::ChannelProxy::MessageFilter methods and + // are always called on the IO thread. + virtual void OnFilterAdded(IPC::Channel* channel) {} + virtual void OnFilterRemoved() {} + virtual void OnChannelClosing() {} + virtual void OnChannelConnected(int32 peer_pid) {} + + // Called when the message filter is about to be deleted. This gives + // derived classes the option of controlling which thread they're deleted + // on etc. + virtual void OnDestruct() const; // IPC::Sender implementation. Can be called on any thread. Can't send sync // messages (since we don't want to block the browser on any other process). @@ -49,7 +56,7 @@ class CONTENT_EXPORT BrowserMessageFilter // browser thread, change |thread| to the id of the target thread virtual void OverrideThreadForMessage( const IPC::Message& message, - BrowserThread::ID* thread); + BrowserThread::ID* thread) {} // If you want the message to be dispatched via the SequencedWorkerPool, // return a non-null task runner which will target tasks accordingly. @@ -71,6 +78,10 @@ class CONTENT_EXPORT BrowserMessageFilter // Can be called on any thread, after OnChannelConnected is called. base::ProcessId peer_pid() const { return peer_pid_; } + void set_peer_pid_for_testing(base::ProcessId peer_pid) { + peer_pid_ = peer_pid; + } + // Checks that the given message can be dispatched on the UI thread, depending // on the platform. If not, returns false and an error ot the sender. static bool CheckCanDispatchOnUI(const IPC::Message& message, @@ -84,8 +95,24 @@ class CONTENT_EXPORT BrowserMessageFilter virtual ~BrowserMessageFilter(); private: - // Dispatches a message to the derived class. - bool DispatchMessage(const IPC::Message& message); + friend class base::RefCountedThreadSafe<BrowserMessageFilter, + BrowserMessageFilterTraits>; + + class Internal; + friend class BrowserChildProcessHostImpl; + friend class BrowserPpapiHost; + friend class RenderProcessHostImpl; + + // This is private because the only classes that need access to it are made + // friends above. This is only guaranteed to be valid on creation, after that + // this class could outlive the filter. + IPC::ChannelProxy::MessageFilter* GetFilter(); + + // This implements IPC::ChannelProxy::MessageFilter so that we can hide that + // from child classes. Internal keeps a reference to this class, which is why + // there's a weak pointer back. This class could outlive Internal based on + // what the child class does in its OnDestruct method. + Internal* internal_; IPC::Channel* channel_; base::ProcessId peer_pid_; @@ -96,6 +123,12 @@ class CONTENT_EXPORT BrowserMessageFilter #endif }; +struct BrowserMessageFilterTraits { + static void Destruct(const BrowserMessageFilter* filter) { + filter->OnDestruct(); + } +}; + } // namespace content #endif // CONTENT_PUBLIC_BROWSER_BROWSER_MESSAGE_FILTER_H_ diff --git a/content/public/browser/render_process_host.h b/content/public/browser/render_process_host.h index 35627c7..4d301c0 100644 --- a/content/public/browser/render_process_host.h +++ b/content/public/browser/render_process_host.h @@ -18,17 +18,15 @@ class GURL; struct ViewMsg_SwapOut_Params; -namespace content { -class BrowserContext; -class RenderWidgetHost; -class StoragePartition; -} - namespace base { class TimeDelta; } namespace content { +class BrowserContext; +class BrowserMessageFilter; +class RenderWidgetHost; +class StoragePartition; typedef base::Thread* (*RendererMainThreadFactoryFunction)( const std::string& id); @@ -160,6 +158,9 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender, // Returns the renderer channel. virtual IPC::ChannelProxy* GetChannel() = 0; + // Adds a message filter to the IPC channel. + virtual void AddFilter(BrowserMessageFilter* filter) = 0; + // Try to shutdown the associated render process as fast as possible virtual bool FastShutdownForPageCount(size_t count) = 0; diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc index 7375048..20f44cc 100644 --- a/content/public/test/mock_render_process_host.cc +++ b/content/public/test/mock_render_process_host.cc @@ -205,6 +205,9 @@ IPC::ChannelProxy* MockRenderProcessHost::GetChannel() { return NULL; } +void MockRenderProcessHost::AddFilter(BrowserMessageFilter* filter) { +} + int MockRenderProcessHost::GetActiveViewCount() { int num_active_views = 0; scoped_ptr<RenderWidgetHostIterator> widgets( diff --git a/content/public/test/mock_render_process_host.h b/content/public/test/mock_render_process_host.h index 700f63f..a11473a 100644 --- a/content/public/test/mock_render_process_host.h +++ b/content/public/test/mock_render_process_host.h @@ -67,6 +67,7 @@ class MockRenderProcessHost : public RenderProcessHost { virtual bool InSameStoragePartition( StoragePartition* partition) const OVERRIDE; virtual IPC::ChannelProxy* GetChannel() OVERRIDE; + virtual void AddFilter(BrowserMessageFilter* filter) OVERRIDE; virtual bool FastShutdownForPageCount(size_t count) OVERRIDE; virtual base::TimeDelta GetChildProcessIdleTime() const OVERRIDE; virtual void SurfaceUpdated(int32 surface_id) OVERRIDE; diff --git a/content/shell/browser/shell_content_browser_client.cc b/content/shell/browser/shell_content_browser_client.cc index 040fce8..0facfaec 100644 --- a/content/shell/browser/shell_content_browser_client.cc +++ b/content/shell/browser/shell_content_browser_client.cc @@ -79,7 +79,7 @@ void ShellContentBrowserClient::RenderProcessHostCreated( RenderProcessHost* host) { if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kDumpRenderTree)) return; - host->GetChannel()->AddFilter(new ShellMessageFilter( + host->AddFilter(new ShellMessageFilter( host->GetID(), BrowserContext::GetDefaultStoragePartition(browser_context()) ->GetDatabaseTracker(), diff --git a/content/test/webrtc_audio_device_test.cc b/content/test/webrtc_audio_device_test.cc index 9b0a819..605713a 100644 --- a/content/test/webrtc_audio_device_test.cc +++ b/content/test/webrtc_audio_device_test.cc @@ -77,6 +77,61 @@ class WebRTCMockRenderProcess : public RenderProcess { DISALLOW_COPY_AND_ASSIGN(WebRTCMockRenderProcess); }; +class TestAudioRendererHost : public AudioRendererHost { + public: + TestAudioRendererHost( + int render_process_id, + media::AudioManager* audio_manager, + AudioMirroringManager* mirroring_manager, + MediaInternals* media_internals, + MediaStreamManager* media_stream_manager, + IPC::Channel* channel) + : AudioRendererHost(render_process_id, audio_manager, mirroring_manager, + media_internals, media_stream_manager), + channel_(channel) {} + virtual bool Send(IPC::Message* message) OVERRIDE { + if (channel_) + return channel_->Send(message); + return false; + } + void ResetChannel() { + channel_ = NULL; + } + + protected: + virtual ~TestAudioRendererHost() {} + + private: + IPC::Channel* channel_; +}; + +class TestAudioInputRendererHost : public AudioInputRendererHost { + public: + TestAudioInputRendererHost( + media::AudioManager* audio_manager, + MediaStreamManager* media_stream_manager, + AudioMirroringManager* audio_mirroring_manager, + media::UserInputMonitor* user_input_monitor, + IPC::Channel* channel) + : AudioInputRendererHost(audio_manager, media_stream_manager, + audio_mirroring_manager, user_input_monitor), + channel_(channel) {} + virtual bool Send(IPC::Message* message) OVERRIDE { + if (channel_) + return channel_->Send(message); + return false; + } + void ResetChannel() { + channel_ = NULL; + } + + protected: + virtual ~TestAudioInputRendererHost() {} + + private: + IPC::Channel* channel_; +}; + // Utility scoped class to replace the global content client's renderer for the // duration of the test. class ReplaceContentClientRenderer { @@ -258,24 +313,23 @@ void MAYBE_WebRTCAudioDeviceTest::UninitializeIOThread() { void MAYBE_WebRTCAudioDeviceTest::CreateChannel(const char* name) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + channel_.reset(new IPC::Channel(name, IPC::Channel::MODE_SERVER, this)); + ASSERT_TRUE(channel_->Connect()); + static const int kRenderProcessId = 1; - audio_render_host_ = new AudioRendererHost( + audio_render_host_ = new TestAudioRendererHost( kRenderProcessId, audio_manager_.get(), mirroring_manager_.get(), - media_internals_.get(), media_stream_manager_.get()); - audio_render_host_->OnChannelConnected(base::GetCurrentProcId()); + media_internals_.get(), media_stream_manager_.get(), channel_.get()); + audio_render_host_->set_peer_pid_for_testing(base::GetCurrentProcId()); audio_input_renderer_host_ = - new AudioInputRendererHost(audio_manager_.get(), - media_stream_manager_.get(), - mirroring_manager_.get(), - NULL); - audio_input_renderer_host_->OnChannelConnected(base::GetCurrentProcId()); - - channel_.reset(new IPC::Channel(name, IPC::Channel::MODE_SERVER, this)); - ASSERT_TRUE(channel_->Connect()); - - audio_render_host_->OnFilterAdded(channel_.get()); - audio_input_renderer_host_->OnFilterAdded(channel_.get()); + new TestAudioInputRendererHost(audio_manager_.get(), + media_stream_manager_.get(), + mirroring_manager_.get(), + NULL, + channel_.get()); + audio_input_renderer_host_->set_peer_pid_for_testing( + base::GetCurrentProcId()); } void MAYBE_WebRTCAudioDeviceTest::DestroyChannel() { @@ -284,6 +338,8 @@ void MAYBE_WebRTCAudioDeviceTest::DestroyChannel() { audio_render_host_->OnFilterRemoved(); audio_input_renderer_host_->OnChannelClosing(); audio_input_renderer_host_->OnFilterRemoved(); + audio_render_host_->ResetChannel(); + audio_input_renderer_host_->ResetChannel(); channel_.reset(); audio_render_host_ = NULL; audio_input_renderer_host_ = NULL; diff --git a/content/test/webrtc_audio_device_test.h b/content/test/webrtc_audio_device_test.h index 927e1dc..b22d7d9 100644 --- a/content/test/webrtc_audio_device_test.h +++ b/content/test/webrtc_audio_device_test.h @@ -45,13 +45,13 @@ class ScopedCOMInitializer; namespace content { -class AudioInputRendererHost; class AudioMirroringManager; -class AudioRendererHost; class ContentRendererClient; class MediaStreamManager; class RenderThreadImpl; class ResourceContext; +class TestAudioInputRendererHost; +class TestAudioRendererHost; class TestBrowserThread; class WebRtcAudioRenderer; class WebRTCMockRenderProcess; @@ -174,8 +174,8 @@ class MAYBE_WebRTCAudioDeviceTest : public ::testing::Test, scoped_ptr<net::URLRequestContext> test_request_context_; scoped_ptr<ResourceContext> resource_context_; scoped_ptr<IPC::Channel> channel_; - scoped_refptr<AudioRendererHost> audio_render_host_; - scoped_refptr<AudioInputRendererHost> audio_input_renderer_host_; + scoped_refptr<TestAudioRendererHost> audio_render_host_; + scoped_refptr<TestAudioInputRendererHost> audio_input_renderer_host_; media::AudioHardwareConfig* audio_hardware_config_; // Weak reference. |