Use a map to store WebFrame in WebFramesManagerImpl

Change the implementation of the WebFramesManagerImpl to use
a map instead of a vector.
WebFrames are uniquely identified by frameID, so having a map
frameID -> WebFrame will ensure that there uniqueness and make
lookup easier and faster (log(N) instead of N).

Bug: 851636
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: If3c76acd8b1bd05a57821c87b571cffa7b8d3607
Reviewed-on: https://chromium-review.googlesource.com/1142781
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585469}

ios/web/public/web_state/web_frames_manager.h

@@ -5,8 +5,8 @@
 #ifndef IOS_WEB_PUBLIC_WEB_STATE_WEB_FRAMES_MANAGER_H_
 #define IOS_WEB_PUBLIC_WEB_STATE_WEB_FRAMES_MANAGER_H_
 
+#include <set>
 #include <string>
-#include <vector>
 
 #include "base/macros.h"
 #import "ios/web/public/web_state/web_state_user_data.h"
@@ -25,7 +25,7 @@ class WebFramesManager : public web::WebStateUserData<WebFramesManager> {
   // Returns a list of all the web frames associated with WebState.
   // NOTE: Due to the asynchronous nature of renderer, this list may be
   // outdated.
-  virtual const std::vector<WebFrame*>& GetAllWebFrames() = 0;
+  virtual std::set<WebFrame*> GetAllWebFrames() = 0;
   // Returns the web frame for the main frame associated with WebState or null
   // if unknown.
   // NOTE: Due to the asynchronous nature of JavaScript to native messsaging,

ios/web/web_state/web_frames_manager_impl.h

@@ -31,17 +31,15 @@ class WebFramesManagerImpl : public WebFramesManager {
   void RegisterExistingFrames();
 
   // WebFramesManager overrides
-  const std::vector<WebFrame*>& GetAllWebFrames() override;
+  std::set<WebFrame*> GetAllWebFrames() override;
   WebFrame* GetMainWebFrame() override;
   WebFrame* GetFrameWithId(const std::string& frame_id) override;
 
  private:
   friend class web::WebStateUserData<WebFramesManagerImpl>;
 
-  // List of all web frames associated with WebState.
-  std::vector<std::unique_ptr<WebFrame>> web_frames_;
   // List of pointers to all web frames associated with WebState.
-  std::vector<WebFrame*> web_frame_ptrs_;
+  std::map<std::string, std::unique_ptr<WebFrame>> web_frames_;
 
   // Reference to the current main web frame.
   WebFrame* main_web_frame_ = nullptr;

ios/web/web_state/web_frames_manager_impl.mm

@@ -54,39 +54,35 @@ void WebFramesManagerImpl::AddFrame(std::unique_ptr<WebFrame> frame) {
   if (frame->IsMainFrame()) {
     main_web_frame_ = frame.get();
   }
-  web_frame_ptrs_.push_back(frame.get());
-  web_frames_.push_back(std::move(frame));
+  DCHECK(web_frames_.count(frame->GetFrameId()) == 0);
+  web_frames_[frame->GetFrameId()] = std::move(frame);
 }
 
 void WebFramesManagerImpl::RemoveFrameWithId(const std::string& frame_id) {
   if (main_web_frame_ && main_web_frame_->GetFrameId() == frame_id) {
     main_web_frame_ = nullptr;
   }
-
-  auto web_frame_ptrs_it = std::find_if(
-      web_frame_ptrs_.begin(), web_frame_ptrs_.end(), FrameIdMatcher(frame_id));
-  if (web_frame_ptrs_it != web_frame_ptrs_.end()) {
-    web_frame_ptrs_.erase(web_frame_ptrs_it);
-    web_frames_.erase(web_frames_.begin() +
-                      (web_frame_ptrs_it - web_frame_ptrs_.begin()));
-  }
+  web_frames_.erase(frame_id);
 }
 
 void WebFramesManagerImpl::RemoveAllWebFrames() {
   main_web_frame_ = nullptr;
   web_frames_.clear();
-  web_frame_ptrs_.clear();
 }
 
 WebFrame* WebFramesManagerImpl::GetFrameWithId(const std::string& frame_id) {
-  auto web_frame_ptrs_it = std::find_if(
-      web_frame_ptrs_.begin(), web_frame_ptrs_.end(), FrameIdMatcher(frame_id));
-  return web_frame_ptrs_it == web_frame_ptrs_.end() ? nullptr
-                                                    : *web_frame_ptrs_it;
+  DCHECK(!frame_id.empty());
+  auto web_frames_it = web_frames_.find(frame_id);
+  return web_frames_it == web_frames_.end() ? nullptr
+                                            : web_frames_it->second.get();
 }
 
-const std::vector<WebFrame*>& WebFramesManagerImpl::GetAllWebFrames() {
-  return web_frame_ptrs_;
+std::set<WebFrame*> WebFramesManagerImpl::GetAllWebFrames() {
+  std::set<WebFrame*> frames;
+  for (const auto& it : web_frames_) {
+    frames.insert(it.second.get());
+  }
+  return frames;
 }
 
 WebFrame* WebFramesManagerImpl::GetMainWebFrame() {

ios/web/web_state/web_frames_manager_impl_unittest.mm

@@ -22,7 +22,7 @@ namespace {
 const char kFrameKey[] = "d1uJzdvOFIUT5kEpK4o+x5JCaSlYT/a45ISU7S9EzTo=";
 
 // Returns true if |web_frame| is contained in |frames|.
-bool ContainsWebFrame(std::vector<web::WebFrame*> frames,
+bool ContainsWebFrame(std::set<web::WebFrame*> frames,
                       web::WebFrame* web_frame) {
   return frames.end() != std::find(frames.begin(), frames.end(), web_frame);
 }