Make window.open() IPCs be frame-based

content/browser/frame_host/render_frame_host_impl.cc

 // 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.
 
 #include "content/browser/frame_host/render_frame_host_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 46 matching lines @@
 #include "content/browser/websockets/websocket_manager.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/common/accessibility_messages.h"
 #include "content/common/associated_interface_provider_impl.h"
 #include "content/common/associated_interfaces.mojom.h"
 #include "content/common/frame_messages.h"
 #include "content/common/frame_owner_properties.h"
 #include "content/common/input_messages.h"
 #include "content/common/inter_process_time_ticks_converter.h"
 #include "content/common/navigation_params.h"
+#include "content/common/render_message_filter.mojom.h"
 #include "content/common/renderer.mojom.h"
 #include "content/common/site_isolation_policy.h"
 #include "content/common/swapped_out_messages.h"
+#include "content/common/view_messages.h"

[alexmos, 2016/12/13 18:41:40]

Why is this needed?

[ncarter (slow), 2016/12/15 00:33:16]

Removed. Good catch.

 #include "content/public/browser/ax_event_notification_details.h"
 #include "content/public/browser/browser_accessibility_state.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_plugin_guest_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/permission_manager.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host_view.h"
@@ skipping 669 matching lines @@
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidStopLoading, OnDidStopLoading)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidChangeLoadProgress,
                         OnDidChangeLoadProgress)
     IPC_MESSAGE_HANDLER(FrameHostMsg_SerializeAsMHTMLResponse,
                         OnSerializeAsMHTMLResponse)
     IPC_MESSAGE_HANDLER(FrameHostMsg_SelectionChanged, OnSelectionChanged)
 #if defined(USE_EXTERNAL_POPUP_MENU)
     IPC_MESSAGE_HANDLER(FrameHostMsg_ShowPopup, OnShowPopup)
     IPC_MESSAGE_HANDLER(FrameHostMsg_HidePopup, OnHidePopup)
 #endif
+    IPC_MESSAGE_HANDLER(FrameHostMsg_ShowCreatedWindow, OnShowCreatedWindow)
   IPC_END_MESSAGE_MAP()
 
   // No further actions here, since we may have been deleted.
   return handled;
 }
 
 void RenderFrameHostImpl::OnAssociatedInterfaceRequest(
     const std::string& interface_name,
     mojo::ScopedInterfaceEndpointHandle handle) {
   delegate_->OnAssociatedInterfaceRequest(
@@ skipping 235 matching lines @@
   // frame and it is delivered after this host is swapped out.
   // Ignore such messages, as we know this RenderFrameHost is going away.
   if (!is_active() || frame_tree_node_->current_frame_host() != this)
     return;
 
   frame_tree_->AddFrame(frame_tree_node_, GetProcess()->GetID(), new_routing_id,
                         scope, frame_name, frame_unique_name, sandbox_flags,
                         frame_owner_properties);
 }
 
+void RenderFrameHostImpl::OnCreateNewWindow(
+    int32_t render_view_route_id,
+    int32_t main_frame_route_id,
+    int32_t main_frame_widget_route_id,
+    const mojom::CreateNewWindowParams& params,
+    SessionStorageNamespace* session_storage_namespace) {
+  mojom::CreateNewWindowParamsPtr validated_params(params.Clone());
+  GetProcess()->FilterURL(false, &validated_params->target_url);
+
+  // TODO(nick): |opener_url|, |opener_security_origin|, and
+  // |opener_top_level_frame_url| should not be parameters; we can just use
+  // last_committed_url(), etc. Of these, |opener_top_level_frame_url| is
+  // particularly egregious, since an oopif isn't expected to know its top URL.

[alexmos, 2016/12/13 18:41:40]

This cleanup seems important.  Is it worth filing a bug so we don't forget to
deal with it, or maybe we already have one?  (There's 466297 that I used for
opener_top_level_frame_url, but it has a different focus.)

Also, we never do FilterURL on the opener_top_level_frame_url, do we?  (IIRC,
it's used for content settings lookups on the IO thread, so don't know how much
of a problem that is.  But the IO thread work that precedes this is why it's not
going to be easy to just switch these to last_committed_url(), etc.)

[ncarter (slow), 2016/12/15 00:33:16]

I've filed a bug. The usage on the IO thread is what makes this tricky.

+  GetProcess()->FilterURL(false, &validated_params->opener_url);
+  GetProcess()->FilterURL(true, &validated_params->opener_security_origin);
+
+  // Ignore creation when sent from a frame that's not current.
+  if (frame_tree_node()->current_frame_host() == this) {

[alexmos, 2016/12/13 18:41:40]

Can this be shortened to just is_active()?  (That covers pending delete, and we
can't ever get here from a pending RFH, can we?)

[ncarter (slow), 2016/12/15 00:33:16]

Good idea. Done.

+    delegate_->CreateNewWindow(GetSiteInstance(), render_view_route_id,
+                               main_frame_route_id, main_frame_widget_route_id,
+                               *validated_params, session_storage_namespace);
+  }
+
+  // Our caller (RenderWidgetHelper::OnCreateNewWindowOnUI) will send
+  // ViewMsg_Close if the above step did not adopt |main_frame_route_id|.
+}
+
 void RenderFrameHostImpl::OnDetach() {
   frame_tree_->RemoveFrame(frame_tree_node_);
 }
 
 void RenderFrameHostImpl::OnFrameFocused() {
   delegate_->SetFocusedFrame(frame_tree_node_, GetSiteInstance());
 }
 
 void RenderFrameHostImpl::OnOpenURL(const FrameHostMsg_OpenURL_Params& params) {
   GURL validated_url(params.url);
@@ skipping 1147 matching lines @@
 }
 
 void RenderFrameHostImpl::OnHidePopup() {
   RenderViewHostDelegateView* view =
       render_view_host_->delegate_->GetDelegateView();
   if (view)
     view->HidePopupMenu();
 }
 #endif
 
+void RenderFrameHostImpl::OnShowCreatedWindow(int pending_widget_routing_id,
+                                              WindowOpenDisposition disposition,
+                                              const gfx::Rect& initial_rect,
+                                              bool user_gesture) {
+  delegate_->ShowCreatedWindow(GetProcess()->GetID(), pending_widget_routing_id,
+                               disposition, initial_rect, user_gesture);
+}
+
 void RenderFrameHostImpl::RegisterMojoInterfaces() {
   device::GeolocationServiceContext* geolocation_service_context =
       delegate_ ? delegate_->GetGeolocationServiceContext() : NULL;
   if (geolocation_service_context) {
     // TODO(creis): Bind process ID here so that GeolocationServiceImpl
     // can perform permissions checks once site isolation is complete.
     // crbug.com/426384
     // NOTE: At shutdown, there is no guaranteed ordering between destruction of
     // this object and destruction of any GeolocationServicesImpls created via
     // the below service registry, the reason being that the destruction of the
@@ skipping 1131 matching lines @@
   // pending_nav_entry_id. If the previous handle was a prematurely aborted
   // navigation loaded via LoadDataWithBaseURL, propagate the entry id.
   return NavigationHandleImpl::Create(
       params.url, frame_tree_node_, is_renderer_initiated,
       params.was_within_same_page, params.is_srcdoc, base::TimeTicks::Now(),
       entry_id_for_data_nav, params.gesture,
       false);  // started_from_context_menu
 }
 
 }  // namespace content