NTP: simplify mojo connection setup

chrome/browser/ui/search/search_ipc_router.cc

Index: chrome/browser/ui/search/search_ipc_router.cc
diff --git a/chrome/browser/ui/search/search_ipc_router.cc b/chrome/browser/ui/search/search_ipc_router.cc
index f3b2e238dee25672b6225e899667d6d04b8377e4..4e64158204dd8fd9197a453dfd6d483e6b3107bf 100644
--- a/chrome/browser/ui/search/search_ipc_router.cc
+++ b/chrome/browser/ui/search/search_ipc_router.cc
@@ -30,56 +30,55 @@ bool IsRenderedInInstantProcess(content::WebContents* web_contents) {
 }
 
 class SearchBoxClientFactoryImpl
-    : public SearchIPCRouter::SearchBoxClientFactory {
+    : public SearchIPCRouter::SearchBoxClientFactory,
+      public chrome::mojom::NTPConnector {
  public:
-  // The |web_contents| must outlive this object.
-  explicit SearchBoxClientFactoryImpl(content::WebContents* web_contents)
+  // |web_contents| and |binding| must outlive this object.
+  SearchBoxClientFactoryImpl(
+      content::WebContents* web_contents,
+      mojo::AssociatedBinding<chrome::mojom::Instant>* binding)
       : web_contents_(web_contents),
-        last_connected_rfh_(
-            std::make_pair(content::ChildProcessHost::kInvalidUniqueID,
-                           MSG_ROUTING_NONE)) {}
-  chrome::mojom::SearchBox* GetSearchBox() override;
+        binding_(binding),
+        bindings_(web_contents, this) {
+    DCHECK(web_contents);
+    DCHECK(binding);
+    // Before we are connected to a frame we throw away all messages.
+    mojo::GetIsolatedProxy(&search_box_);
+  }
+
+  chrome::mojom::SearchBox* GetSearchBox() override {
+    return search_box_.get();
+  }
 
  private:
-  void OnConnectionError();
+  void Connect(chrome::mojom::InstantAssociatedRequest request,
+               chrome::mojom::SearchBoxAssociatedPtrInfo search_box) override;
 
   content::WebContents* web_contents_;
+
+  // An interface used to push updates to the frame that connected to us. Before
+  // we've been connected to a frame, messages sent on this interface goes into

[Marc Treib, 2017/02/08 10:00:36]

nit: s/goes/go/

[tibell, 2017/02/09 03:35:28]

Done.

+  // the void.
   chrome::mojom::SearchBoxAssociatedPtr search_box_;
 
-  // The proccess ID and routing ID of the last connected main frame. May be
-  // (ChildProcessHost::kInvalidUniqueID, MSG_ROUTING_NONE) if we haven't
-  // connected yet.
-  std::pair<int, int> last_connected_rfh_;
+  // Used to bind incomming interface requests to the implementation, which

[dcheng, 2017/02/08 09:01:39]

Nit: incomming -> incoming

[tibell, 2017/02/09 03:35:28]

Done.

+  // lives in SearchIPCRouter.
+  mojo::AssociatedBinding<chrome::mojom::Instant>* binding_;
+
+  // Binding used to listen to connection requests.
+  content::WebContentsFrameBindingSet<chrome::mojom::NTPConnector> bindings_;
 
   DISALLOW_COPY_AND_ASSIGN(SearchBoxClientFactoryImpl);
 };
 
-chrome::mojom::SearchBox* SearchBoxClientFactoryImpl::GetSearchBox() {
-  content::RenderFrameHost* frame = web_contents_->GetMainFrame();
-  auto id = std::make_pair(frame->GetProcess()->GetID(), frame->GetRoutingID());
-  // Avoid reconnecting repeatedly to the same RenderFrameHost for performance
-  // reasons.
-  if (id != last_connected_rfh_) {
-    if (IsRenderedInInstantProcess(web_contents_)) {
-      frame->GetRemoteAssociatedInterfaces()->GetInterface(&search_box_);
-      search_box_.set_connection_error_handler(
-          base::Bind(&SearchBoxClientFactoryImpl::OnConnectionError,
-                     base::Unretained(this)));
-    } else {
-      // Renderer is not an instant process. We'll create a connection that
-      // drops all messages.
-      mojo::GetIsolatedProxy(&search_box_);
-    }
-    last_connected_rfh_ = id;
+void SearchBoxClientFactoryImpl::Connect(
+    chrome::mojom::InstantAssociatedRequest request,
+    chrome::mojom::SearchBoxAssociatedPtrInfo search_box) {
+  if (!IsRenderedInInstantProcess(web_contents_)) {
+    return;
   }
-  return search_box_.get();
-}
-
-void SearchBoxClientFactoryImpl::OnConnectionError() {
-  search_box_.reset();
-  last_connected_rfh_ = std::make_pair(
-      content::ChildProcessHost::kInvalidUniqueID,
-      MSG_ROUTING_NONE);
+  binding_->Bind(std::move(request));
+  search_box_.Bind(std::move(search_box));
 }
 
 }  // namespace
@@ -92,14 +91,15 @@ SearchIPCRouter::SearchIPCRouter(content::WebContents* web_contents,
       policy_(std::move(policy)),
       commit_counter_(0),
       is_active_tab_(false),
-      bindings_(web_contents, this),
-      search_box_client_factory_(new SearchBoxClientFactoryImpl(web_contents)) {
+      binding_(this),
+      search_box_client_factory_(
+          new SearchBoxClientFactoryImpl(web_contents, &binding_)) {
   DCHECK(web_contents);
   DCHECK(delegate);
   DCHECK(policy_.get());
 }
 
-SearchIPCRouter::~SearchIPCRouter() {}
+SearchIPCRouter::~SearchIPCRouter() = default;
 
 void SearchIPCRouter::OnNavigationEntryCommitted() {
   ++commit_counter_;