Refactor UniqueNameHelper to use an adapter pattern for code sharing.

content/renderer/render_frame_impl.h

 // 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.
 
 #ifndef CONTENT_RENDERER_RENDER_FRAME_IMPL_H_
 #define CONTENT_RENDERER_RENDER_FRAME_IMPL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 19 matching lines @@
 #include "base/unguessable_token.h"
 #include "build/build_config.h"
 #include "content/common/accessibility_mode.h"
 #include "content/common/associated_interface_registry_impl.h"
 #include "content/common/download/mhtml_save_status.h"
 #include "content/common/features.h"
 #include "content/common/frame.mojom.h"
 #include "content/common/frame_message_enums.h"
 #include "content/common/host_zoom.mojom.h"
 #include "content/common/renderer.mojom.h"
+#include "content/common/unique_name_helper.h"
 #include "content/common/url_loader_factory.mojom.h"
 #include "content/public/common/console_message_level.h"
 #include "content/public/common/javascript_dialog_type.h"
 #include "content/public/common/previews_state.h"
 #include "content/public/common/referrer.h"
 #include "content/public/common/request_context_type.h"
 #include "content/public/common/stop_find_action.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/renderer/frame_blame_context.h"
 #include "content/renderer/mojo/blink_interface_provider_impl.h"
 #include "content/renderer/renderer_webcookiejar_impl.h"
-#include "content/renderer/unique_name_helper.h"
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_platform_file.h"
 #include "media/base/routing_token_callback.h"
 #include "media/blink/webmediaplayer_delegate.h"
 #include "media/blink/webmediaplayer_params.h"
 #include "media/mojo/features.h"
 #include "media/mojo/interfaces/remoting.mojom.h"
 #include "mojo/public/cpp/bindings/associated_binding.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
@@ skipping 1098 matching lines @@
   // constructor until BindToWebFrame is called, and it is null after
   // frameDetached is called until destruction (which is asynchronous in the
   // case of the main frame, but not subframes).
   blink::WebLocalFrame* frame_;
 
   // Boolean value indicating whether this RenderFrameImpl object is for the
   // main frame or not. It remains accurate during destruction, even when
   // |frame_| has been invalidated.
   bool is_main_frame_;
 
+  class RenderFrameAdapter : public UniqueNameHelper::FrameAdapter {
+   public:
+    explicit RenderFrameAdapter(RenderFrameImpl* render_frame);
+    ~RenderFrameAdapter() override;
+
+    // FrameAdapter overrides:
+    bool IsMainFrame() const override;
+    bool IsCandidateUnique(const std::string& name) const override;
+    int GetSiblingCount() const override;
+    int GetChildCount() const override;
+    std::vector<base::StringPiece> CollectAncestorNames(
+        BeginPoint begin_point,
+        bool (*should_stop)(base::StringPiece)) const override;
+    std::vector<int> GetFramePosition(BeginPoint begin_point) const override;
+
+   private:
+    blink::WebLocalFrame* GetWebFrame() const;
+
+    RenderFrameImpl* render_frame_;
+  };
+  // TODO(dcheng): This name is not good.
+  RenderFrameAdapter render_frame_adapter_;

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

Optional: Have you considered hiding the RenderFrameAdapter class in an
anonymous namespace inside render_frame_impl.cc?  (and changing
RenderFrameAdapter type below into
std::unique_ptr<UniqueNameHelper::FrameAdapter>).

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

I considered that. While it hides the subclass definition, it introduces an
unnecessary heap indirection. Internal Google style is to avoid using forward
declares just to avoid including a full definition.

One option would be to move it to a separate file, but then I really need to
find a better name =)

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

FWIW, the current name is not that bad IMO.  If you want to change it, then
maybe UniqueNameFrameAdapter (since the "Render" part is implied by having the
class nested within RenderFrame)?

[dcheng, 2017/05/25 18:40:41]

Done.

   UniqueNameHelper unique_name_helper_;
 
   // When a frame is detached in response to a message from the browser process,
   // this RenderFrame should not be sending notifications back to it. This
   // boolean is used to indicate this case.
   bool in_browser_initiated_detach_;
 
   // Indicates whether the frame has been inserted into the frame tree yet or
   // not.
   //
@@ skipping 298 matching lines @@
   std::vector<media::RoutingTokenCallback> pending_routing_token_callbacks_;
 
   base::WeakPtrFactory<RenderFrameImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderFrameImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_RENDER_FRAME_IMPL_H_