Fix setting the top-level FrameTreeNode name for named windows.

content/browser/web_contents/web_contents_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
@@ skipping 1186 matching lines @@
   should_normally_be_visible_ = !params.initially_hidden;
 
   // Either both routing ids can be given, or neither can be.
   DCHECK((params.routing_id == MSG_ROUTING_NONE &&
               params.main_frame_routing_id == MSG_ROUTING_NONE) ||
          (params.routing_id != MSG_ROUTING_NONE &&
               params.main_frame_routing_id != MSG_ROUTING_NONE));
   GetRenderManager()->Init(
       params.browser_context, params.site_instance, params.routing_id,
       params.main_frame_routing_id);
+  frame_tree_.root()->SetFrameName(params.main_frame_name);

[Charlie Reis, 2015/05/12 23:45:24]

Could anything in RFHM::Init ever depend on the name?  Just wondering if we need
to move this above it (or if there's a reason to wait until here).

[nasko, 2015/05/13 00:07:27]

SetFrameName actually accesses render_manager_ and sends notifications to all
proxies in it. It sounds incorrect to access an object that hasn't been
initialized yet. Alex and I tried to reason through whether we need the name
during Init and it doesn't seem we do, so decided to leave it this way.

 
   WebContentsViewDelegate* delegate =
       GetContentClient()->browser()->GetWebContentsViewDelegate(this);
 
   if (browser_plugin_guest_) {
     scoped_ptr<WebContentsView> platform_view(CreateWebContentsView(
         this, delegate, &render_view_host_delegate_view_));
 
     WebContentsViewGuest* rv = new WebContentsViewGuest(
         this, browser_plugin_guest_.get(), platform_view.Pass(),
@@ skipping 358 matching lines @@
     GetRenderViewHost()->GetProcess()->ResumeRequestsForView(
         main_frame_route_id);
     return;
   }
 
   // Create the new web contents. This will automatically create the new
   // WebContentsView. In the future, we may want to create the view separately.
   CreateParams create_params(GetBrowserContext(), site_instance.get());
   create_params.routing_id = route_id;
   create_params.main_frame_routing_id = main_frame_route_id;
+  create_params.main_frame_name = base::UTF16ToUTF8(params.frame_name);

[Charlie Reis, 2015/05/12 23:45:24]

Why do FrameTreeNode and FrameHostMsg_DidChangeName have a std::string when
ViewHostMsg_CreateWindow_Params has a string16?  Seems like these should have
the same type.

A quick skim shows that WebViewGuest is the one of two places that consume the
string16, and it converts it to a std::string. 
BackgroundContentsService::CreateBackgroundContents is the other, and that
(through some plumbing) passes it to DictionaryValue::SetString, which supports
both string16 and std::string.

Maybe in a separate cleanup CL we can standardize on std::string?

[nasko, 2015/05/13 00:07:27]

Yeah, a separate CL will be fine. I tried doing it on this one, but it did add a
bunch of unrelated changes, so decided to not upload that version.

   create_params.opener = this;
   create_params.opener_suppressed = params.opener_suppressed;
   if (params.disposition == NEW_BACKGROUND_TAB)
     create_params.initially_hidden = true;
   create_params.renderer_initiated_creation = true;
 
   WebContentsImpl* new_contents = NULL;
   if (!is_guest) {
     create_params.context = view_->GetNativeView();
     create_params.initial_size = GetContainerBounds().size();
@@ skipping 2839 matching lines @@
   node->render_manager()->ResumeResponseDeferredAtStart();
 }
 
 void WebContentsImpl::SetForceDisableOverscrollContent(bool force_disable) {
   force_disable_overscroll_content_ = force_disable;
   if (view_)
     view_->SetOverscrollControllerEnabled(CanOverscrollContent());
 }
 
 }  // namespace content