Convert GetSearchProviderInstallState to Mojo

chrome/browser/search_engines/search_provider_install_state_impl.cc

Index: chrome/browser/search_engines/search_provider_install_state_impl.cc
diff --git a/chrome/browser/search_engines/search_provider_install_state_message_filter.cc b/chrome/browser/search_engines/search_provider_install_state_impl.cc
similarity index 40%
rename from chrome/browser/search_engines/search_provider_install_state_message_filter.cc
rename to chrome/browser/search_engines/search_provider_install_state_impl.cc
index ccd32f4dcb5690316e8b538772a645ea67829686..27db381357c87fe921047f1c94ec504292bf3bb6 100644
--- a/chrome/browser/search_engines/search_provider_install_state_message_filter.cc
+++ b/chrome/browser/search_engines/search_provider_install_state_impl.cc
@@ -2,26 +2,54 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/search_engines/search_provider_install_state_message_filter.h"
+#include "chrome/browser/search_engines/search_provider_install_state_impl.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/supports_user_data.h"
 #include "chrome/browser/google/google_url_tracker_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
-#include "chrome/common/render_messages.h"
+#include "chrome/common/search_provider.mojom.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/common/service_registry.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 
-SearchProviderInstallStateMessageFilter::
-SearchProviderInstallStateMessageFilter(
+// Adapter used to store the impl in user data and to make sure it's deleted on
+// the right thread.
+class SearchProviderInstallStateImplAdapter

[dcheng, 2016/07/04 06:46:29]

Please add some more comments describing how this all works together. A lot of
things are underspecified during teardown, so it's not obvious that this is
safe: for example, is it possible there are still pending requests to create
this interface when we've already queued the impl to be destroyed on the IO
thread? I would suggest:
- mentioning that this particular class is just an adapter which is created /
destroyed on the UI thread and has its lifetime associated with the RPH
- the actual impl must be constructed on the ui thread but must otherwise be
used (and destroyed on the io thread)
- other stuff?

[tibell, 2016/07/15 00:47:08]

This class is now gone. All ownership/registration is now done in the RPH.

+    : public base::SupportsUserData::Data {
+ public:
+  explicit SearchProviderInstallStateImplAdapter(
+      std::unique_ptr<SearchProviderInstallStateImpl,

[dcheng, 2016/07/04 06:46:29]

I'd recommend just constructing SearchProviderInstallStateImpl directly in the
ctor to avoid passing this verbose name around. It'll also avoid the need to
explicitly spell out the typename on line 72.

[tibell, 2016/07/15 00:47:08]

Acknowledged.

+                      content::BrowserThread::DeleteOnIOThread> impl);
+  ~SearchProviderInstallStateImplAdapter() override = default;
+
+  SearchProviderInstallStateImpl* get() { return impl_.get(); }
+
+ private:
+  std::unique_ptr<SearchProviderInstallStateImpl,
+                  content::BrowserThread::DeleteOnIOThread>
+      impl_;
+
+  DISALLOW_COPY_AND_ASSIGN(SearchProviderInstallStateImplAdapter);
+};
+
+SearchProviderInstallStateImplAdapter::SearchProviderInstallStateImplAdapter(
+    std::unique_ptr<SearchProviderInstallStateImpl,
+                    content::BrowserThread::DeleteOnIOThread> impl)
+    : impl_(std::move(impl)) {}
+
+// Key used to attach the filter to the RenderProcessHost.
+static const char kRenderProcessHostKey[] = "kRenderProcessHostKey";

[dcheng, 2016/07/04 06:46:29]

The key should be specific to this SupportsUserData type (i.e. it should mention
'search provider')

[tibell, 2016/07/15 00:47:08]

No longer used.

+
+SearchProviderInstallStateImpl::SearchProviderInstallStateImpl(
     int render_process_id,
     Profile* profile)
-    : BrowserMessageFilter(ChromeMsgStart),
-      provider_data_(TemplateURLServiceFactory::GetForProfile(profile),
+    : provider_data_(TemplateURLServiceFactory::GetForProfile(profile),
                      UIThreadSearchTermsData(profile).GoogleBaseURLValue(),
                      GoogleURLTrackerFactory::GetForProfile(profile),
                      content::RenderProcessHost::FromID(render_process_id)),
@@ -32,80 +60,78 @@ SearchProviderInstallStateMessageFilter(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
-bool SearchProviderInstallStateMessageFilter::OnMessageReceived(
-    const IPC::Message& message) {
+SearchProviderInstallStateImpl::~SearchProviderInstallStateImpl() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(SearchProviderInstallStateMessageFilter, message)
-    IPC_MESSAGE_HANDLER_DELAY_REPLY(
-        ChromeViewHostMsg_GetSearchProviderInstallState,
-        OnGetSearchProviderInstallState)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
 }
 
-SearchProviderInstallStateMessageFilter::
-~SearchProviderInstallStateMessageFilter() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+void SearchProviderInstallStateImpl::InstallService(
+    content::RenderProcessHost* host) {
+  int id = host->GetID();
+  Profile* profile = Profile::FromBrowserContext(host->GetBrowserContext());
+  auto* adapter = new SearchProviderInstallStateImplAdapter(
+      std::unique_ptr<SearchProviderInstallStateImpl,
+                      content::BrowserThread::DeleteOnIOThread>(
+          new SearchProviderInstallStateImpl(id, profile)));
+  host->GetServiceRegistry()
+      ->AddService<chrome::mojom::SearchProviderInstallState>(
+          base::Bind(&SearchProviderInstallStateImpl::Bind,
+                     base::Unretained(adapter->get())),
+          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
+  host->SetUserData(kRenderProcessHostKey, adapter);
 }
 
-search_provider::InstallState
-SearchProviderInstallStateMessageFilter::GetSearchProviderInstallState(
+void SearchProviderInstallStateImpl::Bind(
+    chrome::mojom::SearchProviderInstallStateRequest request) {
+  binding_set_.AddBinding(this, std::move(request));
+}
+
+chrome::mojom::InstallState
+SearchProviderInstallStateImpl::GetSearchProviderInstallState(
     const GURL& page_location,
     const GURL& requested_host) {
   GURL requested_origin = requested_host.GetOrigin();
 
   // Do the security check before any others to avoid information leaks.
   if (page_location.GetOrigin() != requested_origin)
-    return search_provider::DENIED;
+    return chrome::mojom::InstallState::DENIED;
 
   // In incognito mode, no search information is exposed. (This check must be
   // done after the security check or else a web site can detect that the
   // user is in incognito mode just by doing a cross origin request.)
   if (is_off_the_record_)
-      return search_provider::NOT_INSTALLED;
+    return chrome::mojom::InstallState::NOT_INSTALLED;
 
   switch (provider_data_.GetInstallState(requested_origin)) {
     case SearchProviderInstallData::NOT_INSTALLED:
-      return search_provider::NOT_INSTALLED;
+      return chrome::mojom::InstallState::NOT_INSTALLED;
 
     case SearchProviderInstallData::INSTALLED_BUT_NOT_DEFAULT:
-      return search_provider::INSTALLED_BUT_NOT_DEFAULT;
+      return chrome::mojom::InstallState::INSTALLED_BUT_NOT_DEFAULT;
 
     case SearchProviderInstallData::INSTALLED_AS_DEFAULT:
-      return search_provider::INSTALLED_AS_DEFAULT;
+      return chrome::mojom::InstallState::INSTALLED_AS_DEFAULT;
   }
 
   NOTREACHED();
-  return search_provider::NOT_INSTALLED;
+  return chrome::mojom::InstallState::NOT_INSTALLED;
 }
 
-void
-SearchProviderInstallStateMessageFilter::OnGetSearchProviderInstallState(
+void SearchProviderInstallStateImpl::GetInstallState(
     const GURL& page_location,
     const GURL& requested_host,
-    IPC::Message* reply_msg) {
-  provider_data_.CallWhenLoaded(
-      base::Bind(
-          &SearchProviderInstallStateMessageFilter::
-          ReplyWithProviderInstallState,
-          weak_factory_.GetWeakPtr(),
-          page_location,
-          requested_host,
-          reply_msg));
+    const GetInstallStateCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  provider_data_.CallWhenLoaded(base::Bind(
+      &SearchProviderInstallStateImpl::ReplyWithProviderInstallState,
+      weak_factory_.GetWeakPtr(), page_location, requested_host, callback));
 }
 
-void SearchProviderInstallStateMessageFilter::ReplyWithProviderInstallState(
+void SearchProviderInstallStateImpl::ReplyWithProviderInstallState(
     const GURL& page_location,
     const GURL& requested_host,
-    IPC::Message* reply_msg) {
-  DCHECK(reply_msg);
-  search_provider::InstallState install_state =
+    const GetInstallStateCallback& callback) {
+  chrome::mojom::InstallState install_state =
       GetSearchProviderInstallState(page_location, requested_host);
 
-  ChromeViewHostMsg_GetSearchProviderInstallState::WriteReplyParams(
-      reply_msg,
-      install_state);
-  Send(reply_msg);
+  callback.Run(install_state);
 }