Revert "Move OpenURL from RenderView to RenderFrame."

This reverts commit e0dd068. Suspected of causing layouttest failures. Specifically, some are timing out, and some are no longer printing messages like: Policy delegate: attempt to load http://127.0.0.1:8000/misc/resources/iframe-policy-2.html with navigation type 'link clicked' e.g. http/tests/misc/policy-delegate-called-twice.html https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux/27954/layout-test-results/results.html TBR=nasko@chromium.org BUG= Review URL: https://codereview.chromium.org/181063024 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@254636 0039d316-1c4b-4281-b951-d872f2087c98
cbchan · Mar 4, 2014 · a218763 · a218763
1 parent 9f40590
commit a218763
Show file tree

Hide file tree

Showing 22 changed files with 479 additions and 530 deletions.
diff --git a/content/browser/frame_host/navigator.h b/content/browser/frame_host/navigator.h
@@ -8,7 +8,6 @@
 #include "base/memory/ref_counted.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/navigation_controller.h"
-#include "ui/base/window_open_disposition.h"
 
 class GURL;
 struct FrameHostMsg_DidCommitProvisionalLoad_Params;
@@ -89,31 +88,6 @@ class CONTENT_EXPORT Navigator : public base::RefCounted<Navigator> {
 
   virtual base::TimeTicks GetCurrentLoadStart();
 
-  // The RenderFrameHostImpl has received a request to open a URL with the
-  // specified |disposition|.
-  virtual void RequestOpenURL(RenderFrameHostImpl* render_frame_host,
-                              const GURL& url,
-                              const Referrer& referrer,
-                              WindowOpenDisposition disposition,
-                              int64 source_frame_id,
-                              bool should_replace_current_entry,
-                              bool user_gesture) {}
-
-  // The RenderFrameHostImpl wants to transfer the request to a new renderer.
-  // |redirect_chain| contains any redirect URLs (excluding |url|) that happened
-  // before the transfer.
-  virtual void RequestTransferURL(
-      RenderFrameHostImpl* render_frame_host,
-      const GURL& url,
-      const std::vector<GURL>& redirect_chain,
-      const Referrer& referrer,
-      PageTransition page_transition,
-      WindowOpenDisposition disposition,
-      int64 source_frame_id,
-      const GlobalRequestID& transferred_global_request_id,
-      bool should_replace_current_entry,
-      bool user_gesture) {}
-
  protected:
   friend class base::RefCounted<Navigator>;
   virtual ~Navigator() {}

diff --git a/content/browser/frame_host/navigator_delegate.h b/content/browser/frame_host/navigator_delegate.h
@@ -9,7 +9,6 @@
 #include "content/public/browser/invalidate_type.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/common/page_transition_types.h"
-#include "ui/base/window_open_disposition.h"
 
 class GURL;
 struct FrameHostMsg_DidCommitProvisionalLoad_Params;
@@ -19,7 +18,6 @@ namespace content {
 
 class RenderFrameHostImpl;
 struct LoadCommittedDetails;
-struct OpenURLParams;
 
 // A delegate API used by Navigator to notify its embedder of navigation
 // related events.
@@ -91,10 +89,6 @@ class CONTENT_EXPORT NavigatorDelegate {
       RenderFrameHostImpl* render_frame_host,
       const GURL& url,
       NavigationController::ReloadType reload_type) {}
-
-  // Opens a URL with the given parameters. See PageNavigator::OpenURL, which
-  // this forwards to.
-  virtual void RequestOpenURL(const OpenURLParams& params) {}
 };
 
 }  // namspace content

diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc
@@ -14,16 +14,13 @@
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
-#include "content/browser/webui/web_ui_impl.h"
 #include "content/common/frame_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/global_request_id.h"
 #include "content/public/browser/invalidate_type.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_details.h"
-#include "content/public/browser/page_navigator.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/content_client.h"
@@ -114,13 +111,6 @@ void MakeNavigateParams(const NavigationEntryImpl& entry,
   params->frame_to_navigate = entry.GetFrameToNavigate();
 }
 
-RenderFrameHostManager* GetRenderManager(RenderFrameHostImpl* rfh) {
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess))
-    return rfh->frame_tree_node()->render_manager();
-
-  return rfh->frame_tree_node()->frame_tree()->root()->render_manager();
-}
-
 }  // namespace
 
 
@@ -538,97 +528,4 @@ bool NavigatorImpl::ShouldAssignSiteForURL(const GURL& url) {
   return GetContentClient()->browser()->ShouldAssignSiteForURL(url);
 }
 
-void NavigatorImpl::RequestOpenURL(
-    RenderFrameHostImpl* render_frame_host,
-    const GURL& url,
-    const Referrer& referrer,
-    WindowOpenDisposition disposition,
-    int64 source_frame_id,
-    bool should_replace_current_entry,
-    bool user_gesture) {
-  SiteInstance* current_site_instance =
-      GetRenderManager(render_frame_host)->current_frame_host()->
-          GetSiteInstance();
-  // If this came from a swapped out RenderViewHost, we only allow the request
-  // if we are still in the same BrowsingInstance.
-  if (render_frame_host->render_view_host()->IsSwappedOut() &&
-      !render_frame_host->GetSiteInstance()->IsRelatedSiteInstance(
-          current_site_instance)) {
-    return;
-  }
-
-  // Delegate to RequestTransferURL because this is just the generic
-  // case where |old_request_id| is empty.
-  // TODO(creis): Pass the redirect_chain into this method to support client
-  // redirects.  http://crbug.com/311721.
-  std::vector<GURL> redirect_chain;
-  RequestTransferURL(
-      render_frame_host, url, redirect_chain, referrer, PAGE_TRANSITION_LINK,
-      disposition, source_frame_id, GlobalRequestID(),
-      should_replace_current_entry, user_gesture);
-}
-
-void NavigatorImpl::RequestTransferURL(
-    RenderFrameHostImpl* render_frame_host,
-    const GURL& url,
-    const std::vector<GURL>& redirect_chain,
-    const Referrer& referrer,
-    PageTransition page_transition,
-    WindowOpenDisposition disposition,
-    int64 source_frame_id,
-    const GlobalRequestID& transferred_global_request_id,
-    bool should_replace_current_entry,
-    bool user_gesture) {
-  GURL dest_url(url);
-  SiteInstance* current_site_instance =
-      GetRenderManager(render_frame_host)->current_frame_host()->
-          GetSiteInstance();
-  if (!GetContentClient()->browser()->ShouldAllowOpenURL(
-          current_site_instance, url)) {
-    dest_url = GURL(kAboutBlankURL);
-  }
-
-  // Look up the FrameTreeNode ID corresponding to source_frame_id.
-  int64 frame_tree_node_id = -1;
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess) &&
-      source_frame_id != -1) {
-    FrameTreeNode* source_node =
-        render_frame_host->frame_tree_node()->frame_tree()->FindByRoutingID(
-            source_frame_id, transferred_global_request_id.child_id);
-    if (source_node)
-      frame_tree_node_id = source_node->frame_tree_node_id();
-  }
-  OpenURLParams params(
-      dest_url, referrer, source_frame_id, frame_tree_node_id, disposition,
-      page_transition, true /* is_renderer_initiated */);
-  if (redirect_chain.size() > 0)
-    params.redirect_chain = redirect_chain;
-  params.transferred_global_request_id = transferred_global_request_id;
-  params.should_replace_current_entry = should_replace_current_entry;
-  params.user_gesture = user_gesture;
-
-  if (GetRenderManager(render_frame_host)->web_ui()) {
-    // Web UI pages sometimes want to override the page transition type for
-    // link clicks (e.g., so the new tab page can specify AUTO_BOOKMARK for
-    // automatically generated suggestions).  We don't override other types
-    // like TYPED because they have different implications (e.g., autocomplete).
-    if (PageTransitionCoreTypeIs(params.transition, PAGE_TRANSITION_LINK))
-      params.transition =
-          GetRenderManager(render_frame_host)->web_ui()->
-              GetLinkTransitionType();
-
-    // Note also that we hide the referrer for Web UI pages. We don't really
-    // want web sites to see a referrer of "chrome://blah" (and some
-    // chrome: URLs might have search terms or other stuff we don't want to
-    // send to the site), so we send no referrer.
-    params.referrer = Referrer();
-
-    // Navigations in Web UI pages count as browser-initiated navigations.
-    params.is_renderer_initiated = false;
-  }
-
-  if (delegate_)
-    delegate_->RequestOpenURL(params);
-}
-
 }  // namespace content
diff --git a/content/browser/frame_host/navigator_impl.h b/content/browser/frame_host/navigator_impl.h
@@ -51,24 +51,6 @@ class CONTENT_EXPORT NavigatorImpl : public Navigator {
       RenderFrameHostImpl* render_frame_host,
       NavigationController::ReloadType reload_type) OVERRIDE;
   virtual base::TimeTicks GetCurrentLoadStart() OVERRIDE;
-  virtual void RequestOpenURL(RenderFrameHostImpl* render_frame_host,
-                              const GURL& url,
-                              const Referrer& referrer,
-                              WindowOpenDisposition disposition,
-                              int64 source_frame_id,
-                              bool should_replace_current_entry,
-                              bool user_gesture) OVERRIDE;
-  virtual void RequestTransferURL(
-      RenderFrameHostImpl* render_frame_host,
-      const GURL& url,
-      const std::vector<GURL>& redirect_chain,
-      const Referrer& referrer,
-      PageTransition page_transition,
-      WindowOpenDisposition disposition,
-      int64 source_frame_id,
-      const GlobalRequestID& transferred_global_request_id,
-      bool should_replace_current_entry,
-      bool user_gesture) OVERRIDE;
 
  private:
   virtual ~NavigatorImpl() {}

diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
@@ -162,7 +162,6 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
                                 OnNavigate(msg))
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidStartLoading, OnDidStartLoading)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidStopLoading, OnDidStopLoading)
-    IPC_MESSAGE_HANDLER(FrameHostMsg_OpenURL, OnOpenURL)
     IPC_MESSAGE_HANDLER(FrameHostMsg_SwapOut_ACK, OnSwapOutACK)
     IPC_MESSAGE_HANDLER(FrameHostMsg_ContextMenu, OnContextMenu)
   IPC_END_MESSAGE_MAP_EX()
@@ -193,16 +192,6 @@ void RenderFrameHostImpl::OnDetach() {
   frame_tree_->RemoveFrame(frame_tree_node_);
 }
 
-void RenderFrameHostImpl::OnOpenURL(
-    const FrameHostMsg_OpenURL_Params& params) {
-  GURL validated_url(params.url);
-  GetProcess()->FilterURL(false, &validated_url);
-
-  frame_tree_node_->navigator()->RequestOpenURL(
-      this, validated_url, params.referrer, params.disposition, params.frame_id,
-      params.should_replace_current_entry, params.user_gesture);
-}
-
 void RenderFrameHostImpl::OnDidStartProvisionalLoadForFrame(
     int parent_routing_id,
     bool is_main_frame,

diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
@@ -15,7 +15,6 @@
 
 class GURL;
 struct FrameHostMsg_DidFailProvisionalLoadWithError_Params;
-struct FrameHostMsg_OpenURL_Params;
 struct FrameMsg_Navigate_Params;
 
 namespace base {
@@ -132,7 +131,6 @@ class CONTENT_EXPORT RenderFrameHostImpl : public RenderFrameHost {
 
   // IPC Message handlers.
   void OnDetach();
-  void OnOpenURL(const FrameHostMsg_OpenURL_Params& params);
   void OnDidStartProvisionalLoadForFrame(int parent_routing_id,
                                          bool main_frame,
                                          const GURL& url);

diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
@@ -16,7 +16,6 @@
 #include "content/browser/frame_host/interstitial_page_impl.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
-#include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_factory.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/cross_site_transferring_request.h"
@@ -298,14 +297,10 @@ void RenderFrameHostManager::SwappedOut(RenderViewHost* render_view_host) {
     GURL transfer_url = pending_nav_params_->transfer_url_chain.back();
     pending_nav_params_->transfer_url_chain.pop_back();
 
-    RenderFrameHostImpl* render_frame_host =
-        static_cast<RenderFrameHostImpl*>(render_view_host->GetMainFrame());
-
     // We don't know whether the original request had |user_action| set to true.
     // However, since we force the navigation to be in the current tab, it
     // doesn't matter.
-    render_frame_host->frame_tree_node()->navigator()->RequestTransferURL(
-        render_frame_host,
+    render_view_host->GetDelegate()->RequestTransferURL(
         transfer_url,
         pending_nav_params_->transfer_url_chain,
         pending_nav_params_->referrer,
@@ -357,8 +352,8 @@ void RenderFrameHostManager::SwappedOutFrame(
     // We don't know whether the original request had |user_action| set to true.
     // However, since we force the navigation to be in the current tab, it
     // doesn't matter.
-    render_frame_host->frame_tree_node()->navigator()->RequestTransferURL(
-        render_frame_host,
+    // TODO(creis): Move RequestTransferURL to RenderFrameHost's navigator.
+    render_frame_host->render_view_host()->GetDelegate()->RequestTransferURL(
         transfer_url,
         pending_nav_params_->transfer_url_chain,
         pending_nav_params_->referrer,

diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h
@@ -204,6 +204,29 @@ class CONTENT_EXPORT RenderViewHostDelegate {
       RenderViewHost* render_view_host,
       int32 page_id) {}
 
+  // The page wants to open a URL with the specified disposition.
+  virtual void RequestOpenURL(RenderViewHost* rvh,
+                              const GURL& url,
+                              const Referrer& referrer,
+                              WindowOpenDisposition disposition,
+                              int64 source_frame_id,
+                              bool is_redirect,
+                              bool user_gesture) {}
+
+  // The page wants to transfer the request to a new renderer.
+  // |redirect_chain| contains any redirect URLs (excluding |url|) that happened
+  // before the transfer.
+  virtual void RequestTransferURL(
+      const GURL& url,
+      const std::vector<GURL>& redirect_chain,
+      const Referrer& referrer,
+      PageTransition page_transition,
+      WindowOpenDisposition disposition,
+      int64 source_frame_id,
+      const GlobalRequestID& old_request_id,
+      bool is_redirect,
+      bool user_gesture) {}
+
   // The page wants to close the active view in this tab.
   virtual void RouteCloseEvent(RenderViewHost* rvh) {}
 

diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
@@ -1188,6 +1188,7 @@ bool RenderViewHostImpl::OnMessageReceived(const IPC::Message& msg) {
     IPC_MESSAGE_HANDLER(ViewHostMsg_DocumentOnLoadCompletedInMainFrame,
                         OnDocumentOnLoadCompletedInMainFrame)
     IPC_MESSAGE_HANDLER(ViewHostMsg_ToggleFullscreen, OnToggleFullscreen)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_OpenURL, OnOpenURL)
     IPC_MESSAGE_HANDLER(ViewHostMsg_DidContentsPreferredSizeChange,
                         OnDidContentsPreferredSizeChange)
     IPC_MESSAGE_HANDLER(ViewHostMsg_DidChangeScrollOffset,
@@ -1424,6 +1425,16 @@ void RenderViewHostImpl::OnToggleFullscreen(bool enter_fullscreen) {
   WasResized();
 }
 
+void RenderViewHostImpl::OnOpenURL(
+    const ViewHostMsg_OpenURL_Params& params) {
+  GURL validated_url(params.url);
+  GetProcess()->FilterURL(false, &validated_url);
+
+  delegate_->RequestOpenURL(
+      this, validated_url, params.referrer, params.disposition, params.frame_id,
+      params.should_replace_current_entry, params.user_gesture);
+}
+
 void RenderViewHostImpl::OnDidContentsPreferredSizeChange(
     const gfx::Size& new_size) {
   delegate_->UpdatePreferredSize(new_size);

diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
@@ -37,6 +37,7 @@ struct AccessibilityHostMsg_EventParams;
 struct AccessibilityHostMsg_LocationChangeParams;
 struct MediaPlayerAction;
 struct ViewHostMsg_CreateWindow_Params;
+struct ViewHostMsg_OpenURL_Params;
 struct ViewHostMsg_SelectionBounds_Params;
 struct ViewHostMsg_ShowPopup_Params;
 struct FrameMsg_Navigate_Params;
@@ -565,6 +566,7 @@ class CONTENT_EXPORT RenderViewHostImpl
   void OnDocumentAvailableInMainFrame();
   void OnDocumentOnLoadCompletedInMainFrame(int32 page_id);
   void OnToggleFullscreen(bool enter_fullscreen);
+  void OnOpenURL(const ViewHostMsg_OpenURL_Params& params);
   void OnDidContentsPreferredSizeChange(const gfx::Size& new_size);
   void OnDidChangeScrollOffset();
   void OnDidChangeScrollbarsForMainFrame(bool has_horizontal_scrollbar,

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
@@ -6,7 +6,6 @@
 #include "base/containers/hash_tables.h"
 #include "content/browser/dom_storage/dom_storage_context_wrapper.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
-#include "content/browser/frame_host/navigator.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ -57,10 +56,10 @@ RenderViewHostImpl* PrepareToDuplicateHosts(Shell* shell,
   // Now, simulate a link click coming from the renderer.
   GURL extension_url("https://bar.com/files/simple_page.html");
   WebContentsImpl* wc = static_cast<WebContentsImpl*>(shell->web_contents());
-  wc->GetFrameTree()->root()->navigator()->RequestOpenURL(
-      wc->GetFrameTree()->root()->current_frame_host(), extension_url,
+  wc->RequestOpenURL(
+      shell->web_contents()->GetRenderViewHost(), extension_url,
       Referrer(), CURRENT_TAB,
-      wc->GetFrameTree()->root()->current_frame_host()->GetRoutingID(),
+      wc->GetFrameTree()->GetMainFrame()->GetRoutingID(),
       false, true);
 
   // Since the navigation above requires a cross-process swap, there will be a