Add DCHECKS when adding or removing WebFrames

Also clear frames on navigation as it is needed for the DCHECK to pass.

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

ios/web/web_state/ui/crw_web_controller.mm

@@ -2069,6 +2069,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
   [_webView stopLoading];
   [_pendingNavigationInfo setCancelled:YES];
   _certVerificationErrors->Clear();
+  web::WebFramesManagerImpl::FromWebState(self.webState)->RemoveAllWebFrames();
   [self loadCancelled];
 }
 
@@ -4609,6 +4610,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
     }
   }
 
+  web::WebFramesManagerImpl::FromWebState(self.webState)->RemoveAllWebFrames();
   // This must be reset at the end, since code above may need information about
   // the pending load.
   _pendingNavigationInfo = nil;
@@ -4700,6 +4702,8 @@ registerLoadRequestForURL:(const GURL&)requestURL
     }
   }
 
+  web::WebFramesManagerImpl::FromWebState(self.webState)->RemoveAllWebFrames();
+
   // This point should closely approximate the document object change, so reset
   // the list of injected scripts to those that are automatically injected.
   // Do not inject window ID if this is a placeholder URL: window ID is not
@@ -4939,6 +4943,8 @@ registerLoadRequestForURL:(const GURL&)requestURL
 
   [self handleLoadError:WKWebViewErrorWithSource(error, NAVIGATION)
           forNavigation:navigation];
+
+  web::WebFramesManagerImpl::FromWebState(self.webState)->RemoveAllWebFrames();
   _certVerificationErrors->Clear();
   [self forgetNullWKNavigation:navigation];
 }
@@ -4990,6 +4996,7 @@ registerLoadRequestForURL:(const GURL&)requestURL
   [self didReceiveWebViewNavigationDelegateCallback];
 
   _certVerificationErrors->Clear();
+  web::WebFramesManagerImpl::FromWebState(self.webState)->RemoveAllWebFrames();
   [self webViewWebProcessDidCrash];
 }
 

ios/web/web_state/web_frames_manager_impl.h

@@ -20,9 +20,14 @@ class WebFramesManagerImpl : public WebFramesManager {
   static WebFramesManagerImpl* FromWebState(WebState* web_state);
 
   // Adds |frame| to the list of web frames associated with WebState.
+  // The frame must not be already in the frame manager (the frame manager must
+  // not have a frame with the same frame ID). If |frame| is a main frame, the
+  // frame manager must not have a main frame already.
   void AddFrame(std::unique_ptr<WebFrame> frame);
   // Removes the web frame with |frame_id|, if one exists, from the list of
   // associated web frames.
+  // If the frame manager does not contain a frame with this ID, operation is a
+  // no-op.
   void RemoveFrameWithId(const std::string& frame_id);
   // Removes all web frames from the list of associated web frames.
   void RemoveAllWebFrames();

ios/web/web_state/web_frames_manager_impl.mm

@@ -51,7 +51,10 @@ WebFramesManagerImpl::WebFramesManagerImpl(web::WebState* web_state)
     : web_state_(web_state) {}
 
 void WebFramesManagerImpl::AddFrame(std::unique_ptr<WebFrame> frame) {
+  DCHECK(frame);
+  DCHECK(!frame->GetFrameId().empty());
   if (frame->IsMainFrame()) {
+    DCHECK(!main_web_frame_);
     main_web_frame_ = frame.get();
   }
   DCHECK(web_frames_.count(frame->GetFrameId()) == 0);
@@ -59,6 +62,11 @@ void WebFramesManagerImpl::AddFrame(std::unique_ptr<WebFrame> frame) {
 }
 
 void WebFramesManagerImpl::RemoveFrameWithId(const std::string& frame_id) {
+  DCHECK(!frame_id.empty());
+  // If the removed frame is a main frame, it should be the current one.
+  DCHECK(web_frames_.count(frame_id) == 0 ||
+         !web_frames_[frame_id]->IsMainFrame() ||
+         main_web_frame_ == web_frames_[frame_id].get());
   if (main_web_frame_ && main_web_frame_->GetFrameId() == frame_id) {
     main_web_frame_ = nullptr;
   }