cc: Make separate interface for BeginFrame ipc from OutputSurface

content/renderer/render_thread_impl.cc

 // Copyright (c) 2012 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 "content/renderer/render_thread_impl.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <vector>
@@ skipping 55 matching lines @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/renderer_preferences.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/renderer/content_renderer_client.h"
 #include "content/public/renderer/render_process_observer.h"
 #include "content/public/renderer/render_view_visitor.h"
 #include "content/renderer/devtools/devtools_agent_filter.h"
 #include "content/renderer/dom_storage/dom_storage_dispatcher.h"
 #include "content/renderer/dom_storage/webstoragearea_impl.h"
 #include "content/renderer/dom_storage/webstoragenamespace_impl.h"
+#include "content/renderer/gpu/compositor_external_begin_frame_source.h"
 #include "content/renderer/gpu/compositor_output_surface.h"
 #include "content/renderer/gpu/gpu_benchmarking_extension.h"
 #include "content/renderer/input/input_event_filter.h"
 #include "content/renderer/input/input_handler_manager.h"
 #include "content/renderer/media/aec_dump_message_filter.h"
 #include "content/renderer/media/audio_input_message_filter.h"
 #include "content/renderer/media/audio_message_filter.h"
 #include "content/renderer/media/audio_renderer_mixer_manager.h"
 #include "content/renderer/media/media_stream_center.h"
 #include "content/renderer/media/midi_message_filter.h"
@@ skipping 552 matching lines @@
 
   // Shutdown the file thread if it's running.
   if (file_thread_)
     file_thread_->Stop();
 
   if (compositor_output_surface_filter_.get()) {
     RemoveFilter(compositor_output_surface_filter_.get());
     compositor_output_surface_filter_ = NULL;
   }
 
+  if (begin_frame_source_filter_.get()) {
+    RemoveFilter(begin_frame_source_filter_.get());
+    begin_frame_source_filter_ = NULL;
+  }
+
   media_thread_.reset();
 
   // AudioMessageFilter may be accessed on |media_thread_|, so shutdown after.
   RemoveFilter(audio_message_filter_.get());
   audio_message_filter_ = NULL;
 
   compositor_thread_.reset();
   input_handler_manager_.reset();
   if (input_event_filter_.get()) {
     RemoveFilter(input_event_filter_.get());
@@ skipping 238 matching lines @@
                                main_thread_compositor_task_runner_,
                                compositor_message_loop_proxy_);
       AddFilter(input_event_filter_.get());
       input_handler_manager_client = input_event_filter_.get();
     }
     input_handler_manager_.reset(
         new InputHandlerManager(compositor_message_loop_proxy_,
                                 input_handler_manager_client));
   }
 
   scoped_refptr<base::MessageLoopProxy> output_surface_loop;

[danakj, 2014/10/10 16:03:36]

nit: rename to compositor_impl_side_loop?

[simonhong, 2014/10/15 01:04:22]

Done.

   if (enable)
     output_surface_loop = compositor_message_loop_proxy_;
   else
     output_surface_loop = base::MessageLoopProxy::current();
 
   compositor_output_surface_filter_ =
       CompositorOutputSurface::CreateFilter(output_surface_loop.get());
   AddFilter(compositor_output_surface_filter_.get());
 
+  begin_frame_source_filter_ = CompositorExternalBeginFrameSource::CreateFilter(
+      output_surface_loop.get());
+  AddFilter(begin_frame_source_filter_.get());

[piman, 2014/10/10 20:39:46]

I would prefer if there was still a single filter. We found scaling problems
with filters, because every IPC message needs to go through every filter, so a
new filter adds overhead to *all* messages, including unrelated ones, see
crbug.com/340881

Since both CompositorOutputSurface's and CompositorExternalBeginFrameSource's
filters do essentially the same thing (find messages to handle on the compositor
thread and forward to that), we can make a single filter, quickly reject
irrelevant messages, then post onto the compositor thread and dispatch between
CompositorOutputSurface and CompositorExternalBeginFrameSource based on the type
(bonus points if we can fold InputEventFilter into the same logic).

[simonhong, 2014/10/15 01:04:22]

Thanks for pointing out.
CreateCompositorMessageFilter() is created to handle this in
content/renderer/gpu/compositor_message_filter.h

On 2014/10/10 20:39:46, piman (Very slow to review) wrote:

+
   RenderThreadImpl::RegisterSchemes();
 
   EnableBlinkPlatformLogChannels(
       command_line.GetSwitchValueASCII(switches::kBlinkPlatformLogChannels));
 
   SetRuntimeFeaturesDefaultsAndUpdateFromArgs(command_line);
 
   if (!media::IsMediaLibraryInitialized()) {
     WebRuntimeFeatures::enableWebAudio(false);
   }
@@ skipping 706 matching lines @@
   hidden_widget_count_--;
 
   if (!GetContentClient()->renderer()->RunIdleHandlerWhenWidgetsHidden()) {
     return;
   }
 
   ScheduleIdleHandler(kLongIdleHandlerDelayMs);
 }
 
 }  // namespace content