Move frame_to_navigate logic to the browser process.

Layout tests can choose a frame by name to navigate, using
testRunner.queueLoad(url, name).  This frame should be
found in the browser process rather than RenderFrameImpl.

This is useful cleanup in general, but also makes it possible
for layout tests to target OOPIFs.

TBR=dcheng,sky
BUG=477150
TEST=Layout tests continue to pass

Review URL: https://codereview.chromium.org/1104603002

Cr-Commit-Position: refs/heads/master@{#326928}

components/sessions/content/content_serialized_navigation_builder.cc

@@ -34,9 +34,6 @@ ContentSerializedNavigationBuilder::FromNavigationEntry(
   navigation.is_overriding_user_agent_ = entry.GetIsOverridingUserAgent();
   navigation.timestamp_ = entry.GetTimestamp();
   navigation.is_restored_ = entry.IsRestored();
-  // If you want to navigate a named frame in Chrome, you will first need to
-  // add support for persisting it. It is currently only used for layout tests.
-  CHECK(entry.GetFrameToNavigate().empty());
   entry.GetExtraData(kSearchTermsKey, &navigation.search_terms_);
   if (entry.GetFavicon().valid)
     navigation.favicon_url_ = entry.GetFavicon().url;

content/browser/frame_host/frame_tree.cc

@@ -23,7 +23,7 @@ namespace content {
 namespace {
 
 // Used with FrameTree::ForEach() to search for the FrameTreeNode
-// corresponding to |frame_tree_node_id| whithin a specific FrameTree.
+// corresponding to |frame_tree_node_id| within a specific FrameTree.
 bool FrameTreeNodeForId(int64 frame_tree_node_id,
                         FrameTreeNode** out_node,
                         FrameTreeNode* node) {
@@ -35,6 +35,19 @@ bool FrameTreeNodeForId(int64 frame_tree_node_id,
   return true;
 }
 
+// Used with FrameTree::ForEach() to search for the FrameTreeNode with the given
+// |name| within a specific FrameTree.
+bool FrameTreeNodeForName(const std::string& name,
+                          FrameTreeNode** out_node,
+                          FrameTreeNode* node) {
+  if (node->frame_name() == name) {
+    *out_node = node;
+    // Terminate iteration once the node has been found.
+    return false;
+  }
+  return true;
+}
+
 bool CreateProxyForSiteInstance(const scoped_refptr<SiteInstance>& instance,
                                 FrameTreeNode* node) {
   // If a new frame is created in the current SiteInstance, other frames in
@@ -104,7 +117,7 @@ FrameTree::~FrameTree() {
 }
 
 FrameTreeNode* FrameTree::FindByID(int64 frame_tree_node_id) {
-  FrameTreeNode* node = NULL;
+  FrameTreeNode* node = nullptr;
   ForEach(base::Bind(&FrameTreeNodeForId, frame_tree_node_id, &node));
   return node;
 }
@@ -126,12 +139,21 @@ FrameTreeNode* FrameTree::FindByRoutingID(int process_id, int routing_id) {
       return result;
   }
 
-  return NULL;
+  return nullptr;
+}
+
+FrameTreeNode* FrameTree::FindByName(const std::string& name) {
+  if (name.empty())
+    return root_.get();
+
+  FrameTreeNode* node = nullptr;
+  ForEach(base::Bind(&FrameTreeNodeForName, name, &node));
+  return node;
 }
 
 void FrameTree::ForEach(
     const base::Callback<bool(FrameTreeNode*)>& on_node) const {
-  ForEach(on_node, NULL);
+  ForEach(on_node, nullptr);
 }
 
 void FrameTree::ForEach(
@@ -272,7 +294,7 @@ RenderViewHostImpl* FrameTree::GetRenderViewHost(SiteInstance* site_instance) {
       render_view_host_map_.find(site_instance->GetId());
   // TODO(creis): Mirror the frame tree so this check can't fail.
   if (iter == render_view_host_map_.end())
-    return NULL;
+    return nullptr;
   return iter->second;
 }
 

content/browser/frame_host/frame_tree.h

@@ -59,6 +59,12 @@ class CONTENT_EXPORT FrameTree {
   // Returns the FrameTreeNode with the given renderer-specific |routing_id|.
   FrameTreeNode* FindByRoutingID(int process_id, int routing_id);
 
+  // Returns the first frame in this tree with the given |name|, or the main
+  // frame if |name| is empty.
+  // Note that this does NOT support pseudo-names like _self, _top, and _blank,
+  // nor searching other FrameTrees (unlike blink::WebView::findFrameByName).
+  FrameTreeNode* FindByName(const std::string& name);
+
   // Executes |on_node| on each node in the frame tree.  If |on_node| returns
   // false, terminates the iteration immediately. Returning false is useful
   // if |on_node| is just doing a search over the tree.  The iteration proceeds

content/browser/frame_host/frame_tree_unittest.cc

@@ -201,6 +201,49 @@ TEST_F(FrameTreeTest, DISABLED_Shape) {
             GetTreeState(frame_tree));
 }
 
+// Ensure frames can be found by frame_tree_node_id, routing ID, or name.
+TEST_F(FrameTreeTest, FindFrames) {
+  // Add a few child frames to the main frame.
+  FrameTree* frame_tree = contents()->GetFrameTree();
+  FrameTreeNode* root = frame_tree->root();
+  main_test_rfh()->OnCreateChildFrame(22, "child0", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(23, "child1", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(24, std::string(), SandboxFlags::NONE);
+  FrameTreeNode* child0 = root->child_at(0);
+  FrameTreeNode* child1 = root->child_at(1);
+  FrameTreeNode* child2 = root->child_at(2);
+
+  // Add one grandchild frame.
+  child1->current_frame_host()->OnCreateChildFrame(33, "grandchild",
+                                                   SandboxFlags::NONE);
+  FrameTreeNode* grandchild = child1->child_at(0);
+
+  // Ensure they can be found by FTN id.
+  EXPECT_EQ(root, frame_tree->FindByID(root->frame_tree_node_id()));
+  EXPECT_EQ(child0, frame_tree->FindByID(child0->frame_tree_node_id()));
+  EXPECT_EQ(child1, frame_tree->FindByID(child1->frame_tree_node_id()));
+  EXPECT_EQ(child2, frame_tree->FindByID(child2->frame_tree_node_id()));
+  EXPECT_EQ(grandchild, frame_tree->FindByID(grandchild->frame_tree_node_id()));
+  EXPECT_EQ(nullptr, frame_tree->FindByID(-1));
+
+  // Ensure they can be found by routing id.
+  int process_id = main_test_rfh()->GetProcess()->GetID();
+  EXPECT_EQ(root, frame_tree->FindByRoutingID(process_id,
+                                              main_test_rfh()->GetRoutingID()));
+  EXPECT_EQ(child0, frame_tree->FindByRoutingID(process_id, 22));
+  EXPECT_EQ(child1, frame_tree->FindByRoutingID(process_id, 23));
+  EXPECT_EQ(child2, frame_tree->FindByRoutingID(process_id, 24));
+  EXPECT_EQ(grandchild, frame_tree->FindByRoutingID(process_id, 33));
+  EXPECT_EQ(nullptr, frame_tree->FindByRoutingID(process_id, 37));
+
+  // Ensure they can be found by name, if they have one.
+  EXPECT_EQ(root, frame_tree->FindByName(std::string()));
+  EXPECT_EQ(child0, frame_tree->FindByName("child0"));
+  EXPECT_EQ(child1, frame_tree->FindByName("child1"));
+  EXPECT_EQ(grandchild, frame_tree->FindByName("grandchild"));
+  EXPECT_EQ(nullptr, frame_tree->FindByName("no such frame"));
+}
+
 // Do some simple manipulations of the frame tree, making sure that
 // WebContentsObservers see a consistent view of the tree as we go.
 TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {

content/browser/frame_host/navigation_controller_delegate.h

@@ -15,6 +15,7 @@ namespace content {
 struct LoadCommittedDetails;
 struct LoadNotificationDetails;
 struct NativeWebKeyboardEvent;
+class FrameTree;
 class InterstitialPage;
 class InterstitialPageImpl;
 class RenderFrameHost;
@@ -46,6 +47,7 @@ class NavigationControllerDelegate {
 
   // Methods from WebContentsImpl that NavigationControllerImpl needs to
   // call.
+  virtual FrameTree* GetFrameTree() = 0;
   virtual void NotifyBeforeFormRepostWarningShow() = 0;
   virtual void NotifyNavigationEntryCommitted(
       const LoadCommittedDetails& load_details) = 0;

content/browser/frame_host/navigation_controller_impl.cc

@@ -731,6 +731,13 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
           params.is_renderer_initiated,
           params.extra_headers,
           browser_context_));
+  if (!params.frame_name.empty()) {
+    // This is only used for navigating subframes in tests.
+    FrameTreeNode* named_frame =
+        delegate_->GetFrameTree()->FindByName(params.frame_name);
+    if (named_frame)
+      entry->set_frame_tree_node_id(named_frame->frame_tree_node_id());
+  }
   if (params.frame_tree_node_id != -1)
     entry->set_frame_tree_node_id(params.frame_tree_node_id);
   entry->set_source_site_instance(
@@ -745,7 +752,6 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
   entry->SetIsOverridingUserAgent(override);
   entry->set_transferred_global_request_id(
       params.transferred_global_request_id);
-  entry->SetFrameToNavigate(params.frame_name);
 
 #if defined(OS_ANDROID)
   if (params.intent_received_timestamp > 0) {

content/browser/frame_host/navigation_entry_impl.cc

@@ -313,14 +313,6 @@ bool NavigationEntryImpl::GetCanLoadLocalResources() const {
   return can_load_local_resources_;
 }
 
-void NavigationEntryImpl::SetFrameToNavigate(const std::string& frame_name) {
-  frame_to_navigate_ = frame_name;
-}
-
-const std::string& NavigationEntryImpl::GetFrameToNavigate() const {
-  return frame_to_navigate_;
-}
-
 void NavigationEntryImpl::SetExtraData(const std::string& key,
                                        const base::string16& data) {
   extra_data_[key] = data;
@@ -378,7 +370,6 @@ NavigationEntryImpl* NavigationEntryImpl::Clone() const {
   // ResetForCommit: should_replace_entry_
   copy->redirect_chain_ = redirect_chain_;
   // ResetForCommit: should_clear_history_list_
-  copy->frame_to_navigate_ = frame_to_navigate_;
   // ResetForCommit: frame_tree_node_id_
   // ResetForCommit: intent_received_timestamp_
   copy->extra_data_ = extra_data_;
@@ -444,10 +435,9 @@ RequestNavigationParams NavigationEntryImpl::ConstructRequestNavigationParams(
   }
   return RequestNavigationParams(
       GetIsOverridingUserAgent(), navigation_start, redirects,
-      GetCanLoadLocalResources(), GetFrameToNavigate(), base::Time::Now(),
-      GetPageState(), GetPageID(), pending_offset_to_send,
-      current_offset_to_send, current_length_to_send,
-      should_clear_history_list());
+      GetCanLoadLocalResources(), base::Time::Now(), GetPageState(),
+      GetPageID(), pending_offset_to_send, current_offset_to_send,
+      current_length_to_send, should_clear_history_list());
 }
 
 void NavigationEntryImpl::ResetForCommit() {

content/browser/frame_host/navigation_entry_impl.h

@@ -104,8 +104,6 @@ class CONTENT_EXPORT NavigationEntryImpl
   base::Time GetTimestamp() const override;
   void SetCanLoadLocalResources(bool allow) override;
   bool GetCanLoadLocalResources() const override;
-  void SetFrameToNavigate(const std::string& frame_name) override;
-  const std::string& GetFrameToNavigate() const override;
   void SetExtraData(const std::string& key,
                     const base::string16& data) override;
   bool GetExtraData(const std::string& key,
@@ -415,10 +413,6 @@ class CONTENT_EXPORT NavigationEntryImpl
   // value is not needed after the entry commits and is not persisted.
   bool can_load_local_resources_;
 
-  // If not empty, the name of the frame to navigate. This field is not
-  // persisted, because it is currently only used in tests.
-  std::string frame_to_navigate_;
-
   // If not -1, this indicates which FrameTreeNode to navigate.  This field is
   // not persisted because it is experimental and only used when the
   // --site-per-process flag is passed.  It is cleared in |ResetForCommit|

content/browser/frame_host/navigation_entry_impl_unittest.cc

@@ -207,10 +207,6 @@ TEST_F(NavigationEntryTest, NavigationEntryAccessors) {
   entry2_->SetBrowserInitiatedPostData(post_data.get());
   EXPECT_EQ(post_data->front(),
       entry2_->GetBrowserInitiatedPostData()->front());
-
- // Frame to navigate.
-  EXPECT_TRUE(entry1_->GetFrameToNavigate().empty());
-  EXPECT_TRUE(entry2_->GetFrameToNavigate().empty());
 }
 
 // Test basic Clone behavior.

content/common/frame_messages.h

@@ -270,7 +270,6 @@ IPC_STRUCT_TRAITS_BEGIN(content::RequestNavigationParams)
   IPC_STRUCT_TRAITS_MEMBER(browser_navigation_start)
   IPC_STRUCT_TRAITS_MEMBER(redirects)
   IPC_STRUCT_TRAITS_MEMBER(can_load_local_resources)
-  IPC_STRUCT_TRAITS_MEMBER(frame_to_navigate)
   IPC_STRUCT_TRAITS_MEMBER(request_time)
   IPC_STRUCT_TRAITS_MEMBER(page_state)
   IPC_STRUCT_TRAITS_MEMBER(page_id)

content/common/navigation_params.cc

@@ -95,7 +95,6 @@ RequestNavigationParams::RequestNavigationParams(
     base::TimeTicks navigation_start,
     const std::vector<GURL>& redirects,
     bool can_load_local_resources,
-    const std::string& frame_to_navigate,
     base::Time request_time,
     const PageState& page_state,
     int32 page_id,
@@ -107,7 +106,6 @@ RequestNavigationParams::RequestNavigationParams(
       browser_navigation_start(navigation_start),
       redirects(redirects),
       can_load_local_resources(can_load_local_resources),
-      frame_to_navigate(frame_to_navigate),
       request_time(request_time),
       page_state(page_state),
       page_id(page_id),

content/common/navigation_params.h

@@ -166,7 +166,6 @@ struct CONTENT_EXPORT RequestNavigationParams {
                           base::TimeTicks navigation_start,
                           const std::vector<GURL>& redirects,
                           bool can_load_local_resources,
-                          const std::string& frame_to_navigate,
                           base::Time request_time,
                           const PageState& page_state,
                           int32 page_id,
@@ -190,9 +189,6 @@ struct CONTENT_EXPORT RequestNavigationParams {
   // resources.
   bool can_load_local_resources;
 
-  // If not empty, which frame to navigate.
-  std::string frame_to_navigate;
-
   // The time the request was created. This is used by the old performance
   // infrastructure to set up DocumentState associated with the RenderView.
   // TODO(ppi): make it go away.

content/public/browser/navigation_entry.h

@@ -183,12 +183,6 @@ class NavigationEntry {
   virtual void SetCanLoadLocalResources(bool allow) = 0;
   virtual bool GetCanLoadLocalResources() const = 0;
 
-  // Used to specify which frame to navigate. If empty, the main frame is
-  // navigated. This is currently not persisted in session restore, because it
-  // is currently only used in tests.
-  virtual void SetFrameToNavigate(const std::string& frame_name) = 0;
-  virtual const std::string& GetFrameToNavigate() const = 0;
-
   // Set extra data on this NavigationEntry according to the specified |key|.
   // This data is not persisted by default.
   virtual void SetExtraData(const std::string& key,

content/renderer/render_frame_impl.cc

@@ -1091,15 +1091,6 @@ void RenderFrameImpl::OnNavigate(
 
   GetContentClient()->SetActiveURL(common_params.url);
 
-  WebFrame* frame = frame_;
-  if (!request_params.frame_to_navigate.empty()) {
-    // TODO(nasko): Move this lookup to the browser process.
-    frame = render_view_->webview()->findFrameByName(
-        WebString::fromUTF8(request_params.frame_to_navigate));
-    CHECK(frame) << "Invalid frame name passed: "
-                 << request_params.frame_to_navigate;
-  }
-
   // If this frame isn't in the same process as its parent, it will naively
   // assume that this is the first navigation in the iframe, but this may not
   // actually be the case. The PageTransition differentiates between the first
@@ -1109,7 +1100,6 @@ void RenderFrameImpl::OnNavigate(
                                    ui::PAGE_TRANSITION_MANUAL_SUBFRAME)) {
     CHECK(frame_->parent());
     if (frame_->parent()->isWebRemoteFrame()) {
-      CHECK_EQ(frame, frame_);
       frame_->setCommittedFirstRealLoad();
     }
   }
@@ -1136,9 +1126,9 @@ void RenderFrameImpl::OnNavigate(
                          FrameMsg_Navigate_Type::RELOAD_IGNORING_CACHE);
 
     if (reload_original_url)
-      frame->reloadWithOverrideURL(common_params.url, true);
+      frame_->reloadWithOverrideURL(common_params.url, true);
     else
-      frame->reload(ignore_cache);
+      frame_->reload(ignore_cache);
   } else if (is_history_navigation) {
     // We must know the page ID of the page we are navigating back to.
     DCHECK_NE(request_params.page_id, -1);
@@ -1154,12 +1144,12 @@ void RenderFrameImpl::OnNavigate(
           entry.Pass(), navigation_params.Pass(), cache_policy);
     }
   } else if (!common_params.base_url_for_data_url.is_empty()) {
-    LoadDataURL(common_params, frame);
+    LoadDataURL(common_params, frame_);
   } else {
     // Navigate to the given URL.
     WebURLRequest request = CreateURLRequestForNavigation(
         common_params, scoped_ptr<StreamOverrideParameters>(),
-        frame->isViewSourceModeEnabled());
+        frame_->isViewSourceModeEnabled());
 
     if (!start_params.extra_headers.empty()) {
       for (net::HttpUtil::HeadersIterator i(start_params.extra_headers.begin(),
@@ -1193,9 +1183,9 @@ void RenderFrameImpl::OnNavigate(
     // Record this before starting the load, we need a lower bound of this time
     // to sanitize the navigationStart override set below.
     base::TimeTicks renderer_navigation_start = base::TimeTicks::Now();
-    frame->loadRequest(request);
+    frame_->loadRequest(request);
 
-    UpdateFrameNavigationTiming(frame, request_params.browser_navigation_start,
+    UpdateFrameNavigationTiming(frame_, request_params.browser_navigation_start,
                                 renderer_navigation_start);
   }