ChannelMojo: Handle when ChannelMojo outlives ChannelMojoHost

content/browser/renderer_host/render_process_host_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_RENDERER_HOST_BROWSER_RENDER_PROCESS_HOST_IMPL_H_
 #define CONTENT_BROWSER_RENDERER_HOST_BROWSER_RENDER_PROCESS_HOST_IMPL_H_
 
 #include <map>
 #include <queue>
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/process/process.h"
 #include "base/timer/timer.h"
 #include "content/browser/child_process_launcher.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/power_monitor_message_broadcaster.h"
 #include "content/common/content_export.h"
 #include "content/common/mojo/service_registry_impl.h"
 #include "content/public/browser/gpu_data_manager_observer.h"
 #include "content/public/browser/render_process_host.h"
 #include "ipc/ipc_channel_proxy.h"
 #include "ipc/ipc_platform_file.h"
+#include "ipc/mojo/ipc_channel_mojo_host.h"
 #include "mojo/public/cpp/bindings/interface_ptr.h"
 #include "ui/gfx/gpu_memory_buffer.h"
 
 namespace base {
 class CommandLine;
 class MessageLoop;
 }
 
 namespace gfx {
 class Size;
 }
 
-namespace IPC {
-class ChannelMojoHost;
-}
-
 namespace content {
 class AudioRendererHost;
 class BrowserCdmManager;
 class BrowserDemuxerAndroid;
 class GpuMessageFilter;
 class MessagePortMessageFilter;
 class MojoApplicationHost;
 #if defined(ENABLE_WEBRTC)
 class P2PSocketDispatcherHost;
 #endif
@@ skipping 208 matching lines @@
   // Activates Mojo for this process. Does nothing if Mojo is already activated.
   void EnsureMojoActivated();
 
  protected:
   // A proxy for our IPC::Channel that lives on the IO thread (see
   // browser_process.h)
   scoped_ptr<IPC::ChannelProxy> channel_;
 
   // A host object ChannelMojo needs. The lifetime is bound to
   // the RenderProcessHostImpl, not the channel.
-  scoped_ptr<IPC::ChannelMojoHost> channel_mojo_host_;
+  scoped_ptr<IPC::ChannelMojoHost, IPC::ChannelMojoHostDeleter>

[viettrungluu, 2014/09/25 19:38:34]

In general, this seems fragile/awkward to me.

It seems to me that part of the purpose of ChannelMojoHost should be to hide
details about ChannelMojo, and so ChannelMojoHost should just live on the main
thread (or any thread, really).

It suggests to me that instead of ChannelMojo talking directly to
ChannelMojoHost, it should talk to some private object owned by the
ChannelMojoHost (and whose lifecycle you can modify appropriately).

The cleanest way of doing something like this would be to have ChannelMojo have
some ChannelMojo::Delegate interface that it talks to (and which has certain
lifetime requirements), and for the ChannelMojoHost to have some private object
that implements this.

[Hajime Morrita, 2014/09/25 22:00:03]

Thanks for the suggestion! This totally makes sense.

+      channel_mojo_host_;
 
   // True if fast shutdown has been performed on this RPH.
   bool fast_shutdown_started_;
 
   // True if we've posted a DeleteTask and will be deleted soon.
   bool deleting_soon_;
 
 #ifndef NDEBUG
   // True if this object has deleted itself.
   bool is_self_deleted_;
@@ skipping 176 matching lines @@
   base::TimeTicks survive_for_worker_start_time_;
 
   base::WeakPtrFactory<RenderProcessHostImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderProcessHostImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_BROWSER_RENDER_PROCESS_HOST_IMPL_H_