Refactor UniqueNameHelper to use an adapter pattern for code sharing.

content/renderer/render_frame_impl.cc

 // Copyright 2013 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/renderer/render_frame_impl.h"
 
+#include <algorithm>
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/auto_reset.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
 #include "base/debug/asan_invalid_access.h"
@@ skipping 883 matching lines @@
 }
 
 struct RenderFrameImpl::PendingFileChooser {
   PendingFileChooser(const FileChooserParams& p,
                      blink::WebFileChooserCompletion* c)
       : params(p), completion(c) {}
   FileChooserParams params;
   blink::WebFileChooserCompletion* completion;  // MAY BE NULL to skip callback.
 };
 
+const std::string& UniqueNameForWebFrame(blink::WebFrame* frame) {
+  return frame->IsWebLocalFrame()
+             ? RenderFrameImpl::FromWebFrame(frame)->unique_name()
+             : RenderFrameProxy::FromWebFrame(frame)->unique_name();
+}
+
+RenderFrameImpl::RenderFrameAdapter::RenderFrameAdapter(
+    RenderFrameImpl* render_frame)
+    : render_frame_(render_frame) {}
+
+RenderFrameImpl::RenderFrameAdapter::~RenderFrameAdapter() {}
+
+bool RenderFrameImpl::RenderFrameAdapter::IsMainFrame() const {
+  return render_frame_->IsMainFrame();
+}
+
+bool RenderFrameImpl::RenderFrameAdapter::IsCandidateUnique(
+    const std::string& name) const {
+  // This method is currently O(N), where N = number of frames in the tree.
+  DCHECK(!name.empty());
+
+  for (blink::WebFrame* frame = GetWebFrame()->Top(); frame;
+       frame = frame->TraverseNext()) {
+    if (frame == GetWebFrame())

[Łukasz Anforowicz, 2017/05/25 16:48:50]

I wonder if this check means that we can avoid the call to
|unique_name_.clear();| in UniqueNameHelper::UpdateName.

[dcheng, 2017/05/25 18:25:03]

This doesn't work very well as it turns out: since the PendingChildFrameAdapter
delegates to the parent's ChildFrameAdapter, it'll ignore the wrong frame.

I could add an extra parameter, but just doing the clear() seems simpler in this
case.

[Łukasz Anforowicz, 2017/05/25 18:33:18]

Acknowledged.

+      continue;
+    if (UniqueNameForWebFrame(frame) == name)
+      return false;
+  }
+
+  return true;
+}
+
+int RenderFrameImpl::RenderFrameAdapter::GetSiblingCount() const {
+  int sibling_count = 0;
+  for (blink::WebFrame* frame = GetWebFrame()->Parent()->FirstChild(); frame;
+       frame = frame->NextSibling()) {
+    if (frame == GetWebFrame())
+      continue;
+    ++sibling_count;
+  }
+  return sibling_count;
+}
+
+int RenderFrameImpl::RenderFrameAdapter::GetChildCount() const {
+  int child_count = 0;
+  for (blink::WebFrame* frame = GetWebFrame()->FirstChild(); frame;
+       frame = frame->NextSibling()) {
+    ++child_count;
+  }
+  return child_count;
+}
+
+std::vector<base::StringPiece>
+RenderFrameImpl::RenderFrameAdapter::CollectAncestorNames(
+    BeginPoint begin_point,
+    bool (*should_stop)(base::StringPiece)) const {
+  std::vector<base::StringPiece> result;
+  for (blink::WebFrame* frame = begin_point == BeginPoint::kParentFrame
+                                    ? GetWebFrame()->Parent()
+                                    : GetWebFrame();
+       frame; frame = frame->Parent()) {
+    result.push_back(UniqueNameForWebFrame(frame));
+    if (should_stop(result.back()))
+      break;
+  }
+  // Reverse, as the names should be ordered from root to leaf. Contrast with
+  // GetFramePosition(), which has the opposite ordering requirement…
+  std::reverse(result.begin(), result.end());

[Łukasz Anforowicz, 2017/05/25 16:48:50]

Will the other implementation (for exploded page state) also have to reverse? 
If yes, then maybe reversing should be done inside unique_name_helper.cc.

[dcheng, 2017/05/25 18:25:03]

I was going to do this in the followup, but doing it now seems fine as well.

+  return result;
+}
+
+std::vector<int> RenderFrameImpl::RenderFrameAdapter::GetFramePosition(
+    BeginPoint begin_point) const {
+  std::vector<int> result;
+  blink::WebFrame* parent = begin_point == BeginPoint::kParentFrame
+                                ? GetWebFrame()->Parent()
+                                : GetWebFrame();
+  blink::WebFrame* child =
+      begin_point == BeginPoint::kParentFrame ? GetWebFrame() : nullptr;
+  while (parent) {
+    int position_in_parent = 0;
+    blink::WebFrame* sibling = parent->FirstChild();
+    while (sibling != child) {
+      sibling = sibling->NextSibling();
+      ++position_in_parent;
+    }
+    result.push_back(position_in_parent);
+
+    child = parent;
+    parent = parent->Parent();
+  }
+  return result;
+}
+
+blink::WebLocalFrame* RenderFrameImpl::RenderFrameAdapter::GetWebFrame() const {
+  return render_frame_->frame_;
+}
+
 // static
 RenderFrameImpl* RenderFrameImpl::Create(RenderViewImpl* render_view,
                                          int32_t routing_id) {
   DCHECK(routing_id != MSG_ROUTING_NONE);
   CreateParams params(render_view, routing_id);
 
   if (g_create_render_frame_impl)
     return g_create_render_frame_impl(params);
   else
     return new RenderFrameImpl(params);
@@ skipping 179 matching lines @@
 
 blink::WebURL RenderFrameImpl::OverrideFlashEmbedWithHTML(
     const blink::WebURL& url) {
   return GetContentClient()->renderer()->OverrideFlashEmbedWithHTML(url);
 }
 
 // RenderFrameImpl ----------------------------------------------------------
 RenderFrameImpl::RenderFrameImpl(const CreateParams& params)
     : frame_(NULL),
       is_main_frame_(true),
-      unique_name_helper_(this),
+      render_frame_adapter_(this),
+      unique_name_helper_(&render_frame_adapter_),
       in_browser_initiated_detach_(false),
       in_frame_tree_(false),
       render_view_(params.render_view),
       routing_id_(params.routing_id),
       proxy_routing_id_(MSG_ROUTING_NONE),
 #if BUILDFLAG(ENABLE_PLUGINS)
       plugin_power_saver_helper_(nullptr),
       plugin_find_handler_(nullptr),
 #endif
       cookie_jar_(this),
@@ skipping 2013 matching lines @@
 }
 
 blink::WebLocalFrame* RenderFrameImpl::CreateChildFrame(
     blink::WebLocalFrame* parent,
     blink::WebTreeScopeType scope,
     const blink::WebString& name,
     const blink::WebString& fallback_name,
     blink::WebSandboxFlags sandbox_flags,
     const blink::WebParsedFeaturePolicy& container_policy,
     const blink::WebFrameOwnerProperties& frame_owner_properties) {
+  DCHECK_EQ(frame_, parent);

[Łukasz Anforowicz, 2017/05/25 16:48:50]

Does that mean that the |parent| parameter can be removed?

[dcheng, 2017/05/25 18:25:03]

Likely. But out of scope of this patch.

+
   // Synchronously notify the browser of a child frame creation to get the
   // routing_id for the RenderFrame.
   int child_routing_id = MSG_ROUTING_NONE;
   FrameHostMsg_CreateChildFrame_Params params;
   params.parent_routing_id = routing_id_;
   params.scope = scope;
   params.frame_name = name.Utf8();
   // The unique name generation logic was moved out of Blink, so for historical
   // reasons, unique name generation needs to take something called the
   // |fallback_name| into account. Normally, unique names are generated based on
   // the browing context name. For new frames, the initial browsing context name
   // comes from the name attribute of the browsing context container element.
   //
   // However, when the browsing context name is null, Blink instead uses the
   // "fallback name" to derive the unique name. The exact contents of the
   // "fallback name" are unspecified, but may contain the value of the
   // 'subresource attribute' of the browsing context container element.
   //
   // Note that Blink can't be changed to just pass |fallback_name| as |name| in
   // the case |name| is empty: |fallback_name| should never affect the actual
   // browsing context name, only unique name generation.
-  params.frame_unique_name = UniqueNameHelper::GenerateNameForNewChildFrame(
-      parent,
+  params.frame_unique_name = unique_name_helper_.GenerateNameForNewChildFrame(
       params.frame_name.empty() ? fallback_name.Utf8() : params.frame_name);
   params.sandbox_flags = sandbox_flags;
   params.container_policy = FeaturePolicyHeaderFromWeb(container_policy);
   params.frame_owner_properties =
       ConvertWebFrameOwnerPropertiesToFrameOwnerProperties(
           frame_owner_properties);
   Send(new FrameHostMsg_CreateChildFrame(params, &child_routing_id));
 
   // Allocation of routing id failed, so we can't create a child frame. This can
   // happen if the synchronous IPC message above has failed.  This can
@@ skipping 3898 matching lines @@
       policy(info.default_policy),
       replaces_current_history_item(info.replaces_current_history_item),
       history_navigation_in_new_child_frame(
           info.is_history_navigation_in_new_child_frame),
       client_redirect(info.is_client_redirect),
       cache_disabled(info.is_cache_disabled),
       form(info.form),
       source_location(info.source_location) {}
 
 }  // namespace content