Prevent drag-and-drop events from firing over cross-site, same-page frames.

This is done by storing the source RenderViewHost and RenderProcessHost for each dragstart. Then, for other drag events (like dragover), if the target frame for that event is in the same RenderViewHost, but different RenderProcessHost, than the source, then the event is not fired.

Note that this patch will not affect behavior on Mac, and a subsequent CL will enforce the same thing for Mac.

BUG=666858

Review-Url: https://codereview.chromium.org/2568893002
Cr-Commit-Position: refs/heads/master@{#438609}

chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc

@@ -1158,6 +1158,10 @@ void DragAndDropBrowserTest::CrossSiteDrag_Step3(
                     "dragend"}));
 }
 
+// TODO(paulmeyer): Should test the case of navigation happening in the middle
+// of a drag operation, and cross-site drags should be allowed across a
+// navigation.
+
 INSTANTIATE_TEST_CASE_P(
     SameSiteSubframe, DragAndDropBrowserTest, ::testing::Values(false));
 

content/browser/web_contents/web_contents_view_aura.cc

@@ -41,6 +41,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_view_delegate.h"
 #include "content/public/browser/web_drag_dest_delegate.h"
+#include "content/public/common/child_process_host.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/drop_data.h"
@@ -76,6 +77,7 @@
 #include "ui/touch_selection/touch_selection_controller.h"
 
 namespace content {
+
 WebContentsView* CreateWebContentsView(
     WebContentsImpl* web_contents,
     WebContentsViewDelegate* delegate,
@@ -405,6 +407,10 @@ int ConvertAuraEventFlagsToWebInputEventModifiers(int aura_event_flags) {
   return web_input_event_modifiers;
 }
 
+GlobalRoutingID GetRenderViewHostID(RenderViewHost* rvh) {
+  return GlobalRoutingID(rvh->GetProcess()->GetID(), rvh->GetRoutingID());
+}
+
 }  // namespace
 
 class WebContentsViewAura::WindowObserver
@@ -520,7 +526,10 @@ WebContentsViewAura::WebContentsViewAura(WebContentsImpl* web_contents,
       delegate_(delegate),
       current_drag_op_(blink::WebDragOperationNone),
       drag_dest_delegate_(nullptr),
-      current_rvh_for_drag_(nullptr),
+      current_rvh_for_drag_(ChildProcessHost::kInvalidUniqueID,
+                            MSG_ROUTING_NONE),
+      drag_start_process_id_(ChildProcessHost::kInvalidUniqueID),
+      drag_start_view_id_(ChildProcessHost::kInvalidUniqueID, MSG_ROUTING_NONE),
       current_overscroll_gesture_(OVERSCROLL_NONE),
       completed_overscroll_gesture_(OVERSCROLL_NONE),
       navigation_overlay_(nullptr),
@@ -557,6 +566,10 @@ void WebContentsViewAura::SizeChangedCommon(const gfx::Size& size) {
 
 void WebContentsViewAura::EndDrag(RenderWidgetHost* source_rwh,
                                   blink::WebDragOperationsMask ops) {
+  drag_start_process_id_ = ChildProcessHost::kInvalidUniqueID;
+  drag_start_view_id_ = GlobalRoutingID(ChildProcessHost::kInvalidUniqueID,
+                                        MSG_ROUTING_NONE);
+
   if (!web_contents_)
     return;
 
@@ -568,8 +581,8 @@ void WebContentsViewAura::EndDrag(RenderWidgetHost* source_rwh,
   if (screen_position_client)
     screen_position_client->ConvertPointFromScreen(window, &client_loc);
 
-  // TODO(paulmeyer): In the OOPIF case, should |client_loc| be converted to
-  // the coordinates local to |drag_start_rwh_|? See crbug.com/647249.
+  // TODO(paulmeyer): In the OOPIF case, should |client_loc| be converted to the
+  // coordinates local to |source_rwh|? See crbug.com/647249.
   web_contents_->DragSourceEndedAt(client_loc.x(), client_loc.y(),
                                    screen_loc.x(), screen_loc.y(), ops,
                                    source_rwh);
@@ -634,6 +647,13 @@ gfx::NativeView WebContentsViewAura::GetRenderWidgetHostViewParent() const {
   return window_.get();
 }
 
+bool WebContentsViewAura::IsValidDragTarget(
+    RenderWidgetHostImpl* target_rwh) const {
+  return target_rwh->GetProcess()->GetID() == drag_start_process_id_ ||
+      GetRenderViewHostID(web_contents_->GetRenderViewHost()) !=
+      drag_start_view_id_;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // WebContentsViewAura, WebContentsView implementation:
 
@@ -912,6 +932,9 @@ void WebContentsViewAura::StartDragging(
   base::WeakPtr<RenderWidgetHostImpl> source_rwh_weak_ptr =
       source_rwh->GetWeakPtr();
 
+  drag_start_process_id_ = source_rwh->GetProcess()->GetID();
+  drag_start_view_id_ = GetRenderViewHostID(web_contents_->GetRenderViewHost());
+
   ui::TouchSelectionController* selection_controller = GetSelectionController();
   if (selection_controller)
     selection_controller->HideAndDisallowShowingAutomatically();
@@ -1135,12 +1158,17 @@ void WebContentsViewAura::OnMouseEvent(ui::MouseEvent* event) {
 
 void WebContentsViewAura::OnDragEntered(const ui::DropTargetEvent& event) {
   gfx::Point transformed_pt;
-  current_rwh_for_drag_ =
+  RenderWidgetHostImpl* target_rwh =
       web_contents_->GetInputEventRouter()->GetRenderWidgetHostAtPoint(
           web_contents_->GetRenderViewHost()->GetWidget()->GetView(),
-          event.location(), &transformed_pt)->GetWeakPtr();
-  current_rvh_for_drag_ = web_contents_->GetRenderViewHost();
+          event.location(), &transformed_pt);
+
+  if (!IsValidDragTarget(target_rwh))
+    return;
 
+  current_rwh_for_drag_ = target_rwh->GetWeakPtr();
+  current_rvh_for_drag_ =
+      GetRenderViewHostID(web_contents_->GetRenderViewHost());
   current_drop_data_.reset(new DropData());
   PrepareDropData(current_drop_data_.get(), event.data());
   current_rwh_for_drag_->FilterDropData(current_drop_data_.get());
@@ -1176,6 +1204,9 @@ int WebContentsViewAura::OnDragUpdated(const ui::DropTargetEvent& event) {
           web_contents_->GetRenderViewHost()->GetWidget()->GetView(),
           event.location(), &transformed_pt);
 
+  if (!IsValidDragTarget(target_rwh))
+    return ui::DragDropTypes::DRAG_NONE;
+
   if (target_rwh != current_rwh_for_drag_.get()) {
     if (current_rwh_for_drag_)
       current_rwh_for_drag_->DragTargetDragLeave();
@@ -1198,7 +1229,8 @@ int WebContentsViewAura::OnDragUpdated(const ui::DropTargetEvent& event) {
 }
 
 void WebContentsViewAura::OnDragExited() {
-  if (current_rvh_for_drag_ != web_contents_->GetRenderViewHost() ||
+  if (current_rvh_for_drag_ !=
+      GetRenderViewHostID(web_contents_->GetRenderViewHost()) ||
       !current_drop_data_) {
     return;
   }
@@ -1221,6 +1253,9 @@ int WebContentsViewAura::OnPerformDrop(const ui::DropTargetEvent& event) {
           web_contents_->GetRenderViewHost()->GetWidget()->GetView(),
           event.location(), &transformed_pt);
 
+  if (!IsValidDragTarget(target_rwh))
+    return ui::DragDropTypes::DRAG_NONE;
+
   if (target_rwh != current_rwh_for_drag_.get()) {
     if (current_rwh_for_drag_)
       current_rwh_for_drag_->DragTargetDragLeave();

content/browser/web_contents/web_contents_view_aura.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "content/browser/loader/global_routing_id.h"
 #include "content/browser/renderer_host/overscroll_controller_delegate.h"
 #include "content/browser/renderer_host/render_view_host_delegate_view.h"
 #include "content/browser/web_contents/web_contents_view.h"
@@ -87,6 +88,11 @@ class CONTENT_EXPORT WebContentsViewAura
   // Returns GetNativeView unless overridden for testing.
   gfx::NativeView GetRenderWidgetHostViewParent() const;
 
+  // Returns whether |target_rwh| is a valid RenderWidgetHost to be dragging
+  // over. This enforces that same-page, cross-site drags are not allowed. See
+  // crbug.com/666858.
+  bool IsValidDragTarget(RenderWidgetHostImpl* target_rwh) const;
+
   // Overridden from WebContentsView:
   gfx::NativeView GetNativeView() const override;
   gfx::NativeView GetContentNativeView() const override;
@@ -198,10 +204,24 @@ class CONTENT_EXPORT WebContentsViewAura
   // during a drag, we need to re-send the DragEnter message.
   base::WeakPtr<RenderWidgetHostImpl> current_rwh_for_drag_;
 
-  // We also keep track of the RenderViewHost we're dragging over to avoid
-  // sending the drag exited message after leaving the current
-  // view. |current_rvh_for_drag_| should not be dereferenced.
-  void* current_rvh_for_drag_;
+  // We also keep track of the ID of the RenderViewHost we're dragging over to
+  // avoid sending the drag exited message after leaving the current view.
+  GlobalRoutingID current_rvh_for_drag_;
+
+  // We track the IDs of the source RenderProcessHost and RenderViewHost from
+  // which the current drag originated. These are used to ensure that drag
+  // events do not fire over a cross-site frame (with respect to the source
+  // frame) in the same page (see crbug.com/666858). Specifically, the
+  // RenderViewHost is used to check the "same page" property, while the
+  // RenderProcessHost is used to check the "cross-site" property. Note that the
+  // reason the RenderProcessHost is tracked instead of the RenderWidgetHost is
+  // so that we still allow drags between non-contiguous same-site frames (such
+  // frames will have the same process, but different widgets). Note also that
+  // the RenderViewHost may not be in the same process as the RenderProcessHost,
+  // since the view corresponds to the page, while the process is specific to
+  // the frame from which the drag started.
+  int drag_start_process_id_;
+  GlobalRoutingID drag_start_view_id_;
 
   // The overscroll gesture currently in progress.
   OverscrollMode current_overscroll_gesture_;

testing/buildbot/filters/site-per-process.interactive_ui_tests.filter

 # List below tests to be excluded from running.
-
-# https://crbug.com/666858: No drag-and-drop events should fire...
--*DragAndDropBrowserTest.CrossSiteDrag*