Migrate components/data_reduction_proxy to Mojo interfaces.

chrome/browser/chrome_content_browser_client.cc

Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 0072a6711749da34bc6832158dc1785287ffdf1a..ba913e5b245c00b74309cefe21a029ae00ccf193 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -118,7 +118,7 @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_types.h"
-#include "components/data_reduction_proxy/content/browser/data_reduction_proxy_message_filter.h"
+#include "components/data_reduction_proxy/content/browser/data_reduction_proxy_host_impl.h"
 #include "components/dom_distiller/core/dom_distiller_switches.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/error_page/common/error_page_switches.h"
@@ -989,10 +989,6 @@ void ChromeContentBrowserClient::RenderProcessWillLaunch(
 #if defined(OS_ANDROID)
   host->AddFilter(new cdm::CdmMessageFilterAndroid());
 #endif
-  DataReductionProxyChromeSettings* data_reduction_proxy_settings =
-      DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile);
-  host->AddFilter(new data_reduction_proxy::DataReductionProxyMessageFilter(
-      data_reduction_proxy_settings));
 
   host->Send(new ChromeViewMsg_SetIsIncognitoProcess(
       profile->IsOffTheRecord()));
@@ -2720,6 +2716,23 @@ void ChromeContentBrowserClient::RegisterRenderProcessMojoServices(
     content::RenderProcessHost* render_process_host) {
   registry->AddService(
       base::Bind(&startup_metric_utils::StartupMetricHostImpl::Create));
+
+  Profile* profile =
+      Profile::FromBrowserContext(render_process_host->GetBrowserContext());
+  DataReductionProxyChromeSettings* data_reduction_proxy_settings =
+      DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile);

[Anand Mistry (off Chromium), 2016/06/24 07:12:28]

I'm personally of the opinion that as much as possible, we should be lazily
creating impls. However, others have differing views. I'll ask sam to comment on
this.

[Sam McNally, 2016/06/24 07:24:48]

While it would be nicer to create them lazily, it's simpler to create them ahead
of time. If we create something to abstract away this complexity, then I think
we can recommend lazy creation.

+  auto* data_reduction_proxy_host =
+      new data_reduction_proxy::DataReductionProxyHostImpl(
+          data_reduction_proxy_settings
+              ? data_reduction_proxy_settings->Config()
+              : nullptr,
+          render_process_host);
+  // The data_reduction_proxy_host is a render process host observer, so here we

[Anand Mistry (off Chromium), 2016/06/24 07:12:28]

I'm convinced this is technically correct. However, the reason why is subtle
which makes me not like this approach. Basically, you're relying on PostTask
ordering between the deletion in the RPH destruction observer and the bind in
ServiceRegistryImpl. You're guaranteed that the service registry post will
always happen before the destruction (because RPH destruction kills the service
registry), and therefore any call to DRPHI::BindRequest will always happen
before the DRPHI is deleted.

Another approach is to always have the DataReductionProxyHostImpl live and
service requests on the UI thread. That way, you can delete when the RPH
observer fires and avoid any lifetime complexity. To service the
IsDataReductionProxy() method, you'll need to post to the IO thread and post
back to the UI thread to respond, but you can use a WeakPtr to ensure lifetime
and this would make lifetime reasoning much simpler.

[Sam McNally, 2016/06/24 07:24:48]

This feels like a lot of unnecessary thread bouncing, and may not work for all
MessageFilter conversions. I would prefer we find a way to do this nicely for
interface implementations that don't live on the UI thread. That said, exposing
this complexity in each converted MessageFilter does sound like an invitation
for confusion and bugs further down the road.

[leonhsl(Using Gerrit), 2016/06/24 09:59:21]

En,, if let DataReductionProxyHostImpl live on UI thread, this maybe cause UI
thread get blocked sometimes when IO thread is busy. As IsDataReductionProxy()
is a sync call.

[leonhsl(Using Gerrit), 2016/06/24 14:46:11]

Sorry for my misunderstanding, you could just ignore my nonsense words.. I'll
try to consider about the nicer way mentioned by Sam.

+  // can use base::Unretained safely.
+  registry->AddService(
+      base::Bind(&data_reduction_proxy::DataReductionProxyHostImpl::BindRequest,
+                 base::Unretained(data_reduction_proxy_host)),
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
 }
 
 void ChromeContentBrowserClient::RegisterFrameMojoShellInterfaces(