Move unique name generation and tracking into //content.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 4af9b671725620814b3f5d10384a881f4c68d83a..e75feb12a7275ef16dfe06dd81481e149238b170 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -978,9 +978,10 @@ void RenderFrameImpl::CreateFrame(
     render_frame =
         RenderFrameImpl::Create(parent_proxy->render_view(), routing_id);
     render_frame->InitializeBlameContext(FromRoutingID(parent_routing_id));
+    render_frame->unique_name_helper_.set_propagated_name(
+        replicated_state.unique_name);
     web_frame = parent_web_frame->createLocalChild(
         replicated_state.scope, WebString::fromUTF8(replicated_state.name),
-        WebString::fromUTF8(replicated_state.unique_name),
         replicated_state.sandbox_flags, render_frame,
         render_frame->blink_interface_provider_.get(),
         render_frame->blink_interface_registry_.get(),
@@ -1079,6 +1080,7 @@ blink::WebURL RenderFrameImpl::overrideFlashEmbedWithHTML(
 RenderFrameImpl::RenderFrameImpl(const CreateParams& params)
     : frame_(NULL),
       is_main_frame_(true),
+      unique_name_helper_(this),
       in_browser_initiated_detach_(false),
       in_frame_tree_(false),
       render_view_(params.render_view),
@@ -3019,7 +3021,7 @@ blink::WebLocalFrame* RenderFrameImpl::createChildFrame(
     blink::WebLocalFrame* parent,
     blink::WebTreeScopeType scope,
     const blink::WebString& name,
-    const blink::WebString& unique_name,
+    const blink::WebString& fallback_name,
     blink::WebSandboxFlags sandbox_flags,
     const blink::WebFrameOwnerProperties& frame_owner_properties) {
   // Synchronously notify the browser of a child frame creation to get the
@@ -3029,7 +3031,9 @@ blink::WebLocalFrame* RenderFrameImpl::createChildFrame(
   params.parent_routing_id = routing_id_;
   params.scope = scope;
   params.frame_name = name.utf8();
-  params.frame_unique_name = unique_name.utf8();
+  params.frame_unique_name = UniqueNameHelper::GenerateNameForNewChildFrame(
+      parent,
+      params.frame_name.empty() ? fallback_name.utf8() : params.frame_name);
   params.sandbox_flags = sandbox_flags;
   params.frame_owner_properties =
       ConvertWebFrameOwnerPropertiesToFrameOwnerProperties(
@@ -3054,6 +3058,8 @@ blink::WebLocalFrame* RenderFrameImpl::createChildFrame(
   // Create the RenderFrame and WebLocalFrame, linking the two.
   RenderFrameImpl* child_render_frame =
       RenderFrameImpl::Create(render_view_, child_routing_id);
+  child_render_frame->unique_name_helper_.set_propagated_name(
+      params.frame_unique_name);
   child_render_frame->InitializeBlameContext(this);
   blink::WebLocalFrame* web_frame = WebLocalFrame::create(
       scope, child_render_frame,
@@ -3164,10 +3170,11 @@ void RenderFrameImpl::willCommitProvisionalLoad(blink::WebLocalFrame* frame) {
     observer.WillCommitProvisionalLoad();
 }
 
-void RenderFrameImpl::didChangeName(const blink::WebString& name,
-                                    const blink::WebString& unique_name) {
-  Send(new FrameHostMsg_DidChangeName(
-      routing_id_, name.utf8(), unique_name.utf8()));
+void RenderFrameImpl::didChangeName(const blink::WebString& name) {
+  if (current_history_item_.isNull())
+    unique_name_helper_.UpdateName(name.utf8());
+  Send(new FrameHostMsg_DidChangeName(routing_id_, name.utf8(),
+                                      unique_name_helper_.value()));
 
   if (!committed_first_load_)
     name_changed_before_first_commit_ = true;
@@ -3620,6 +3627,8 @@ void RenderFrameImpl::didCommitProvisionalLoad(
   // Update the current history item for this frame (both in default Chrome and
   // subframe FrameNavigationEntry modes).

[Charlie Reis, 2017/03/02 23:59:17]

Oops-- while you're here, can you kill this parenthetical phrase?  I should have
removed it when turning on subframe FNEs by default.

[dcheng, 2017/03/03 10:22:08]

Done.

   current_history_item_ = item;
+  current_history_item_.setTarget(
+      blink::WebString::fromUTF8(unique_name_helper_.value()));
 
   InternalDocumentStateData* internal_data =
       InternalDocumentStateData::FromDocumentState(document_state);
@@ -3684,7 +3693,7 @@ void RenderFrameImpl::didCommitProvisionalLoad(
   // new navigation.
   navigation_state->set_request_committed(true);
 
-  SendDidCommitProvisionalLoad(frame, commit_type, item);
+  SendDidCommitProvisionalLoad(frame, commit_type, current_history_item_);

[Charlie Reis, 2017/03/02 23:59:17]

I suppose this is ok.  It's a bit risky if anyone else adds a use of |item|
without realizing they should use |current_history_item_| instead, but I don't
see a great alternative to suggest.

[dcheng, 2017/03/03 10:22:08]

I considered removing it as a function param and just having
SendDidCommitProvisionalLoad() use the member directly. WDYT?

[Charlie Reis, 2017/03/20 20:11:38]

Yes, I definitely support that.

It's a bit orthogonal to my concern (i.e., that someone might use |item| in this
method without realizing it has diverged from current_history_item_, but that's
not something I'm too concerned about.  At most, I suppose we could put a "//
Don't use |item| after this point" comment when we assign it to
current_history_item_.

[dcheng, 2017/03/21 22:03:41]

Done.

 
   // Check whether we have new encoding name.
   UpdateEncoding(frame, frame->view()->pageEncoding().utf8());
@@ -5034,6 +5043,10 @@ bool RenderFrameImpl::SwapIn() {
   RenderFrameProxy* proxy = RenderFrameProxy::FromRoutingID(proxy_routing_id_);
   CHECK(proxy);
 
+  unique_name_helper_.set_propagated_name(proxy->unique_name());
+
+  // Note: Calling swap() will detach and delete |proxy|, so do not reference it
+  // after this.
   int proxy_routing_id = proxy_routing_id_;
   if (!proxy->web_frame()->swap(frame_))
     return false;
@@ -5290,13 +5303,13 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
     // just staying at the initial about:blank document.
     bool should_ask_browser = false;
     RenderFrameImpl* parent = RenderFrameImpl::FromWebFrame(frame_->parent());
-    const auto& iter = parent->history_subframe_unique_names_.find(
-        frame_->uniqueName().utf8());
+    auto iter = parent->history_subframe_unique_names_.find(
+        unique_name_helper_.value());
     if (iter != parent->history_subframe_unique_names_.end()) {
       bool history_item_is_about_blank = iter->second;
       should_ask_browser =
           !history_item_is_about_blank || url != url::kAboutBlankURL;
-      parent->history_subframe_unique_names_.erase(frame_->uniqueName().utf8());
+      parent->history_subframe_unique_names_.erase(iter);
     }
 
     if (should_ask_browser) {