SearchBox: Hook up Mojo interfaces in DidCommitProvisionalLoad

SearchBox may execute JS in response to IPCs, but running JS in the
given frame is illegal before the provisional navigation has committed.

Bug: 765101
Change-Id: I7fc01989177b768667b5d64c2198b2da3ec0adb9
Reviewed-on: https://chromium-review.googlesource.com/723079Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509728}

chrome/renderer/searchbox/searchbox.cc

@@ -224,13 +224,12 @@ SearchBox::SearchBox(content::RenderFrame* render_frame)
       most_visited_items_cache_(kMaxInstantMostVisitedItemCacheSize),
       binding_(this),
       weak_ptr_factory_(this) {
-  // Connect to the embedded search interface in the browser.
-  chrome::mojom::EmbeddedSearchConnectorAssociatedPtr connector;
-  render_frame->GetRemoteAssociatedInterfaces()->GetInterface(&connector);
-  chrome::mojom::EmbeddedSearchClientAssociatedPtrInfo embedded_search_client;
-  binding_.Bind(mojo::MakeRequest(&embedded_search_client));
-  connector->Connect(mojo::MakeRequest(&embedded_search_service_),
-                     std::move(embedded_search_client));
+  // Note: This class may execute JS in |render_frame| in response to IPCs (via
+  // the SearchBoxExtension::Dispatch* methods). However, for cross-process
+  // navigations, a "provisional frame" is created at first, and it's illegal
+  // to execute any JS in it before it's actually swapped in, i.e.m before the
+  // navigation has committed. So we only hook up the Mojo interfaces in
+  // RenderFrameObserver::DidCommitProvisionalLoad. See crbug.com/765101.
 }
 
 SearchBox::~SearchBox() = default;
@@ -410,9 +409,18 @@ GURL SearchBox::GetURLForMostVisitedItem(InstantRestrictedID item_id) const {
   return GetMostVisitedItemWithID(item_id, &item) ? item.url : GURL();
 }
 
-void SearchBox::Bind(
-    chrome::mojom::EmbeddedSearchClientAssociatedRequest request) {
-  binding_.Bind(std::move(request));
+void SearchBox::DidCommitProvisionalLoad(bool is_new_navigation,
+                                         bool is_same_document_navigation) {
+  if (binding_.is_bound())
+    return;
+
+  // Connect to the embedded search interface in the browser.
+  chrome::mojom::EmbeddedSearchConnectorAssociatedPtr connector;
+  render_frame()->GetRemoteAssociatedInterfaces()->GetInterface(&connector);
+  chrome::mojom::EmbeddedSearchClientAssociatedPtrInfo embedded_search_client;
+  binding_.Bind(mojo::MakeRequest(&embedded_search_client));
+  connector->Connect(mojo::MakeRequest(&embedded_search_service_),
+                     std::move(embedded_search_client));
 }
 
 void SearchBox::OnDestruct() {

chrome/renderer/searchbox/searchbox.h

@@ -123,6 +123,8 @@ class SearchBox : public content::RenderFrameObserver,
 
  private:
   // Overridden from content::RenderFrameObserver:
+  void DidCommitProvisionalLoad(bool is_new_navigation,
+                                bool is_same_document_navigation) override;
   void OnDestruct() override;
 
   // Overridden from chrome::mojom::EmbeddedSearchClient:
@@ -141,8 +143,6 @@ class SearchBox : public content::RenderFrameObserver,
   // Returns the URL of the Most Visited item specified by the |item_id|.
   GURL GetURLForMostVisitedItem(InstantRestrictedID item_id) const;
 
-  void Bind(chrome::mojom::EmbeddedSearchClientAssociatedRequest request);
-
   int page_seq_no_;
   bool is_focused_;
   bool is_input_in_progress_;