Move ViewMsg_Stop from RenderViewHost to RenderFrameHost.

This CL is a reland of https://codereview.chromium.org/479403004/ without the part enabling the test. The test is failing due to several bugs, some of which are fixed in this CL and some others lingering. The goal is to reland these fixes without enabling the test and fixing the rest of the issues in followup changes.

BUG=304341
R=avi@chromium.org, creis@chromium.org

Review URL: https://codereview.chromium.org/509063004

Cr-Commit-Position: refs/heads/master@{#292486}

15 files changed, 39 insertions, 62 deletions

diff --git a/chrome/browser/ui/cocoa/applescript/tab_applescript.mm b/chrome/browser/ui/cocoa/applescript/tab_applescript.mm
index 17ee0e4..3dec7e5 100644
--- a/chrome/browser/ui/cocoa/applescript/tab_applescript.mm
+++ b/chrome/browser/ui/cocoa/applescript/tab_applescript.mm
@@ -208,15 +208,7 @@ void ResumeAppleEventAndSendReply(NSAppleEventManagerSuspensionID suspension_id,
 }
 
 - (void)handlesStopScriptCommand:(NSScriptCommand*)command {
-  RenderViewHost* view = webContents_->GetRenderViewHost();
-  if (!view) {
-    // We tolerate Stop being called even before a view has been created.
-    // So just log a warning instead of a NOTREACHED().
-    DLOG(WARNING) << "Stop: no view for handle ";
-    return;
-  }
-
-  view->Stop();
+  webContents_->Stop();
 }
 
 - (void)handlesPrintScriptCommand:(NSScriptCommand*)command {
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 54ba0cf..c9c7c8c 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -594,7 +594,7 @@ void RenderFrameHostImpl::OnNavigate(const IPC::Message& msg) {
 
   // If we're waiting for a cross-site beforeunload ack from this renderer and
   // we receive a Navigate message from the main frame, then the renderer was
-  // navigating already and sent it before hearing the ViewMsg_Stop message.
+  // navigating already and sent it before hearing the FrameMsg_Stop message.
   // We do not want to cancel the pending navigation in this case, since the
   // old page will soon be stopped.  Instead, treat this as a beforeunload ack
   // to allow the pending navigation to continue.
@@ -1120,6 +1120,10 @@ void RenderFrameHostImpl::NavigateToURL(const GURL& url) {
   Navigate(params);
 }
 
+void RenderFrameHostImpl::Stop() {
+  Send(new FrameMsg_Stop(routing_id_));
+}
+
 void RenderFrameHostImpl::DispatchBeforeUnload(bool for_cross_site_transition) {
   TRACE_EVENT_ASYNC_BEGIN0(
       "navigation", "RenderFrameHostImpl::BeforeUnload", this);
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
index 97e1d02..aebe492 100644
--- a/content/browser/frame_host/render_frame_host_impl.h
+++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -192,6 +192,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // Load the specified URL; this is a shortcut for Navigate().
   void NavigateToURL(const GURL& url);
 
+  // Stop the load in progress.
+  void Stop();
+
   // Returns whether navigation messages are currently suspended for this
   // RenderFrameHost. Only true during a cross-site navigation, while waiting
   // for the onbeforeunload handler.
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 0e2612a..b983d2c 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -246,13 +246,13 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate(
 }
 
 void RenderFrameHostManager::Stop() {
-  render_frame_host_->render_view_host()->Stop();
+  render_frame_host_->Stop();
 
   // If we are cross-navigating, we should stop the pending renderers.  This
   // will lead to a DidFailProvisionalLoad, which will properly destroy them.
   if (cross_navigation_pending_) {
-    pending_render_frame_host_->render_view_host()->Send(new ViewMsg_Stop(
-        pending_render_frame_host_->render_view_host()->GetRoutingID()));
+    pending_render_frame_host_->Send(new FrameMsg_Stop(
+        pending_render_frame_host_->GetRoutingID()));
   }
 }
 
@@ -1503,9 +1503,8 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
       // Also make sure the old render view stops, in case a load is in
       // progress.  (We don't want to do this for transfers, since it will
       // interrupt the transfer with an unexpected DidStopLoading.)
-      render_frame_host_->render_view_host()->Send(new ViewMsg_Stop(
-          render_frame_host_->render_view_host()->GetRoutingID()));
-
+      render_frame_host_->Send(new FrameMsg_Stop(
+          render_frame_host_->GetRoutingID()));
       pending_render_frame_host_->SetNavigationsSuspended(true,
                                                           base::TimeTicks());
     }
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 479e680..fd62be0 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -1925,10 +1925,6 @@ void RenderWidgetHostImpl::StartUserGesture() {
   OnUserGesture();
 }
 
-void RenderWidgetHostImpl::Stop() {
-  Send(new ViewMsg_Stop(GetRoutingID()));
-}
-
 void RenderWidgetHostImpl::SetBackgroundOpaque(bool opaque) {
   Send(new ViewMsg_SetBackgroundOpaque(GetRoutingID(), opaque));
 }
diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h
index 56cac76..6f31db2 100644
--- a/content/browser/renderer_host/render_widget_host_impl.h
+++ b/content/browser/renderer_host/render_widget_host_impl.h
@@ -155,7 +155,6 @@ class CONTENT_EXPORT RenderWidgetHostImpl
   virtual void ResizeRectChanged(const gfx::Rect& new_rect) OVERRIDE;
   virtual void RestartHangMonitorTimeout() OVERRIDE;
   virtual void SetIgnoreInputEvents(bool ignore_input_events) OVERRIDE;
-  virtual void Stop() OVERRIDE;
   virtual void WasResized() OVERRIDE;
   virtual void AddKeyPressEventCallback(
       const KeyPressEventCallback& callback) OVERRIDE;
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 7d15e8a..e2ef0e5 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -1158,7 +1158,7 @@ TEST_F(WebContentsImplTest, CrossSiteNotPreemptedDuringBeforeUnload) {
   EXPECT_TRUE(orig_rfh->GetRenderViewHost()->is_waiting_for_beforeunload_ack());
 
   // Suppose the first navigation tries to commit now, with a
-  // ViewMsg_Stop in flight.  This should not cancel the pending navigation,
+  // FrameMsg_Stop in flight.  This should not cancel the pending navigation,
   // but it should act as if the beforeunload ack arrived.
   orig_rfh->SendNavigate(1, GURL("chrome://blah"));
   EXPECT_TRUE(contents()->cross_navigation_pending());
diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h
index d9aca27..d77d9e6 100644
--- a/content/common/frame_messages.h
+++ b/content/common/frame_messages.h
@@ -401,6 +401,9 @@ IPC_MESSAGE_ROUTED0(FrameMsg_BeforeUnload)
 IPC_MESSAGE_ROUTED1(FrameMsg_SwapOut,
                     int /* proxy_routing_id */)
 
+// Instructs the frame to stop the load in progress, if any.
+IPC_MESSAGE_ROUTED0(FrameMsg_Stop)
+
 // Request for the renderer to insert CSS into the frame.
 IPC_MESSAGE_ROUTED1(FrameMsg_CSSInsertRequest,
                     std::string  /* css */)
diff --git a/content/common/view_messages.h b/content/common/view_messages.h
index 11a02a3..fe4140d 100644
--- a/content/common/view_messages.h
+++ b/content/common/view_messages.h
@@ -603,8 +603,6 @@ IPC_MESSAGE_ROUTED2(ViewMsg_ShowContextMenu,
                     ui::MenuSourceType,
                     gfx::Point /* location where menu should be shown */)
 
-IPC_MESSAGE_ROUTED0(ViewMsg_Stop)
-
 // Sent when the user wants to search for a word on the page (find in page).
 IPC_MESSAGE_ROUTED3(ViewMsg_Find,
                     int /* request_id */,
diff --git a/content/public/browser/render_widget_host.h b/content/public/browser/render_widget_host.h
index a87e3fa..4dd8f57 100644
--- a/content/public/browser/render_widget_host.h
+++ b/content/public/browser/render_widget_host.h
@@ -229,9 +229,6 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender {
 
   virtual void SetIgnoreInputEvents(bool ignore_input_events) = 0;
 
-  // Stops loading the page.
-  virtual void Stop() = 0;
-
   // Called to notify the RenderWidget that it has been resized.
   virtual void WasResized() = 0;
 
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 28a872e..b1aafa1 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -752,6 +752,7 @@ bool RenderFrameImpl::OnMessageReceived(const IPC::Message& msg) {
     IPC_MESSAGE_HANDLER(FrameMsg_Navigate, OnNavigate)
     IPC_MESSAGE_HANDLER(FrameMsg_BeforeUnload, OnBeforeUnload)
     IPC_MESSAGE_HANDLER(FrameMsg_SwapOut, OnSwapOut)
+    IPC_MESSAGE_HANDLER(FrameMsg_Stop, OnStop)
     IPC_MESSAGE_HANDLER(FrameMsg_ContextMenuClosed, OnContextMenuClosed)
     IPC_MESSAGE_HANDLER(FrameMsg_CustomContextMenuAction,
                         OnCustomContextMenuAction)
@@ -1016,6 +1017,7 @@ void RenderFrameImpl::OnSwapOut(int proxy_routing_id) {
   RenderFrameProxy* proxy = NULL;
   bool is_site_per_process =
       CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess);
+  bool is_main_frame = !frame_->parent();
 
   // Only run unload if we're not swapped out yet, but send the ack either way.
   if (!is_swapped_out_ || !render_view_->is_swapped_out_) {
@@ -1034,12 +1036,12 @@ void RenderFrameImpl::OnSwapOut(int proxy_routing_id) {
     // Synchronously run the unload handler before sending the ACK.
     // TODO(creis): Call dispatchUnloadEvent unconditionally here to support
     // unload on subframes as well.
-    if (!frame_->parent())
+    if (is_main_frame)
       frame_->dispatchUnloadEvent();
 
     // Swap out and stop sending any IPC messages that are not ACKs.
     // TODO(nasko): Do we need RenderFrameImpl::is_swapped_out_ anymore?
-    if (!frame_->parent())
+    if (is_main_frame)
       render_view_->SetSwappedOut(true);
     is_swapped_out_ = true;
 
@@ -1049,27 +1051,24 @@ void RenderFrameImpl::OnSwapOut(int proxy_routing_id) {
     // TODO(creis): Should we be stopping all frames here and using
     // StopAltErrorPageFetcher with RenderView::OnStop, or just stopping this
     // frame?
-    if (!frame_->parent())
-      render_view_->OnStop();
-    else
-      frame_->stopLoading();
+    OnStop();
 
     // Let subframes know that the frame is now rendered remotely, for the
     // purposes of compositing and input events.
-    if (frame_->parent())
+    if (!is_main_frame)
       frame_->setIsRemote(true);
 
     // Replace the page with a blank dummy URL. The unload handler will not be
     // run a second time, thanks to a check in FrameLoader::stopLoading.
     // TODO(creis): Need to add a better way to do this that avoids running the
     // beforeunload handler. For now, we just run it a second time silently.
-    if (!is_site_per_process || frame_->parent() == NULL)
+    if (!is_site_per_process || is_main_frame)
       render_view_->NavigateToSwappedOutURL(frame_);
 
     // Let WebKit know that this view is hidden so it can drop resources and
     // stop compositing.
     // TODO(creis): Support this for subframes as well.
-    if (!frame_->parent()) {
+    if (is_main_frame) {
       render_view_->webview()->setVisibilityState(
           blink::WebPageVisibilityStateHidden, false);
     }
@@ -1077,7 +1076,7 @@ void RenderFrameImpl::OnSwapOut(int proxy_routing_id) {
 
   // It is now safe to show modal dialogs again.
   // TODO(creis): Deal with modal dialogs from subframes.
-  if (!frame_->parent())
+  if (is_main_frame)
     render_view_->suppress_dialogs_until_swap_out_ = false;
 
   Send(new FrameHostMsg_SwapOut_ACK(routing_id_));
@@ -1085,7 +1084,7 @@ void RenderFrameImpl::OnSwapOut(int proxy_routing_id) {
   // Now that all of the cleanup is complete and the browser side is notified,
   // start using the RenderFrameProxy, if one is created.
   if (proxy) {
-    if (frame_->parent()) {
+    if (!is_main_frame) {
       frame_->swap(proxy->web_frame());
       if (is_site_per_process) {
         // TODO(nasko): delete the frame here, since we've replaced it with a
@@ -1097,7 +1096,7 @@ void RenderFrameImpl::OnSwapOut(int proxy_routing_id) {
   }
 
   // Safe to exit if no one else is using the process.
-  if (!frame_->parent())
+  if (is_main_frame)
     render_view_->WasSwappedOut();
 }
 
@@ -3173,6 +3172,11 @@ void RenderFrameImpl::RemoveObserver(RenderFrameObserver* observer) {
 }
 
 void RenderFrameImpl::OnStop() {
+  DCHECK(frame_);
+  frame_->stopLoading();
+  if (!frame_->parent())
+    FOR_EACH_OBSERVER(RenderViewObserver, render_view_->observers_, OnStop());
+
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_, OnStop());
 }
 
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h
index 9fc0541..08d5b33 100644
--- a/content/renderer/render_frame_impl.h
+++ b/content/renderer/render_frame_impl.h
@@ -147,9 +147,6 @@ class CONTENT_EXPORT RenderFrameImpl
   // spot.
   void Initialize();
 
-  // Notification from RenderView.
-  virtual void OnStop();
-
   // Notifications from RenderWidget.
   void WasHidden();
   void WasShown();
@@ -497,6 +494,7 @@ class CONTENT_EXPORT RenderFrameImpl
   // content/common/*_messages.h for the message that the function is handling.
   void OnBeforeUnload();
   void OnSwapOut(int proxy_routing_id);
+  void OnStop();
   void OnShowContextMenu(const gfx::Point& location);
   void OnContextMenuClosed(const CustomContextMenuContext& custom_context);
   void OnCustomContextMenuAction(const CustomContextMenuContext& custom_context,
diff --git a/content/renderer/render_frame_proxy.cc b/content/renderer/render_frame_proxy.cc
index 2a8a389..c358944 100644
--- a/content/renderer/render_frame_proxy.cc
+++ b/content/renderer/render_frame_proxy.cc
@@ -39,16 +39,10 @@ RenderFrameProxy* RenderFrameProxy::CreateProxyToReplaceFrame(
   scoped_ptr<RenderFrameProxy> proxy(
       new RenderFrameProxy(routing_id, frame_to_replace->GetRoutingID()));
 
-  blink::WebRemoteFrame* web_frame = NULL;
-  if (frame_to_replace->GetWebFrame()->parent() &&
-      frame_to_replace->GetWebFrame()->parent()->isWebRemoteFrame()) {
-    blink::WebRemoteFrame* parent_web_frame =
-        frame_to_replace->GetWebFrame()->parent()->toWebRemoteFrame();
-    web_frame = parent_web_frame->createRemoteChild("", proxy.get());
-  } else {
-    web_frame = blink::WebRemoteFrame::create(proxy.get());
-  }
-
+  // When a RenderFrame is replaced by a RenderProxy, the WebRemoteFrame should
+  // always come from WebRemoteFrame::create and a call to WebFrame::swap must
+  // follow later.
+  blink::WebRemoteFrame* web_frame = blink::WebRemoteFrame::create(proxy.get());
   proxy->Init(web_frame, frame_to_replace->render_view());
   return proxy.release();
 }
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 743d8db..6c9a045 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1303,7 +1303,6 @@ bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) {
                         OnScrollFocusedEditableNodeIntoRect)
     IPC_MESSAGE_HANDLER(InputMsg_SetEditCommandsForNextKeyEvent,
                         OnSetEditCommandsForNextKeyEvent)
-    IPC_MESSAGE_HANDLER(ViewMsg_Stop, OnStop)
     IPC_MESSAGE_HANDLER(ViewMsg_CopyImageAt, OnCopyImageAt)
     IPC_MESSAGE_HANDLER(ViewMsg_SaveImageAt, OnSaveImageAt)
     IPC_MESSAGE_HANDLER(ViewMsg_Find, OnFind)
@@ -1436,14 +1435,6 @@ bool RenderViewImpl::IsBackForwardToStaleEntry(
   return false;
 }
 
-// Stop loading the current page.
-void RenderViewImpl::OnStop() {
-  if (webview())
-    webview()->mainFrame()->stopLoading();
-  FOR_EACH_OBSERVER(RenderViewObserver, observers_, OnStop());
-  main_render_frame_->OnStop();
-}
-
 void RenderViewImpl::OnCopyImageAt(int x, int y) {
   webview()->copyImageAt(WebPoint(x, y));
 }
diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h
index 08a4f74..cd6ba6d 100644
--- a/content/renderer/render_view_impl.h
+++ b/content/renderer/render_view_impl.h
@@ -715,7 +715,6 @@ class CONTENT_EXPORT RenderViewImpl
   void OnSetWebUIProperty(const std::string& name, const std::string& value);
   void OnSetZoomLevelForLoadingURL(const GURL& url, double zoom_level);
   void OnSetZoomLevelForView(bool uses_temporary_zoom_level, double level);
-  void OnStop();
   void OnStopFinding(StopFindAction action);
   void OnSuppressDialogsUntilSwapOut();
   void OnThemeChanged();