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

content/browser/web_contents/web_contents_view_aura.cc

Index: content/browser/web_contents/web_contents_view_aura.cc
diff --git a/content/browser/web_contents/web_contents_view_aura.cc b/content/browser/web_contents/web_contents_view_aura.cc
index 709ea5dd5d1cbccdfe063040cbdc20419c3638c7..dba6ff9264201fda8daf6e5d610fd3ef9bc8c896 100644
--- a/content/browser/web_contents/web_contents_view_aura.cc
+++ b/content/browser/web_contents/web_contents_view_aura.cc
@@ -516,6 +516,8 @@ WebContentsViewAura::WebContentsViewAura(WebContentsImpl* web_contents,
       current_drag_op_(blink::WebDragOperationNone),
       drag_dest_delegate_(nullptr),
       current_rvh_for_drag_(nullptr),
+      drag_source_rph_(nullptr),
+      drag_source_rvh_(nullptr),
       current_overscroll_gesture_(OVERSCROLL_NONE),
       completed_overscroll_gesture_(OVERSCROLL_NONE),
       navigation_overlay_(nullptr),
@@ -563,8 +565,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);
@@ -907,6 +909,9 @@ void WebContentsViewAura::StartDragging(
   base::WeakPtr<RenderWidgetHostImpl> source_rwh_weak_ptr =
       source_rwh->GetWeakPtr();
 
+  drag_source_rph_ = source_rwh->GetProcess();

[lfg, 2016/12/12 20:34:21]

Would it be possible that a RenderProcessHost is destroyed and a new
RenderProcessHost is recreated at the same memory address as the previous one?
Shouldn't we be storing the RenderProcessHost IDs instead?

[Charlie Reis, 2016/12/12 20:57:02]

Yes, I'm concerned about the void* pointers as well, here and the one added long
ago in https://chromiumcodereview.appspot.com/10388105/ (just above this in the
.h file).  Storing the process ID and RVH routing ID would seem safer to me.

[paulmeyer, 2016/12/13 20:57:30]

Done.

[paulmeyer, 2016/12/13 20:57:30]

Done.

+  drag_source_rvh_ = web_contents_->GetRenderViewHost();
+
   ui::TouchSelectionController* selection_controller = GetSelectionController();
   if (selection_controller)
     selection_controller->HideAndDisallowShowingAutomatically();
@@ -1130,12 +1135,18 @@ 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 (drag_source_rvh_ == web_contents_->GetRenderViewHost() &&

[lfg, 2016/12/12 20:34:21]

I don't understand this check. This would only be false if the
WebContentsViewAura that we called OnDragEntered is different than the
WebContentsViewAura that we called StartDragging. In that case, won't
drag_source_rvh_ be null?

Is it possible to fail this check while drag_source_rvh_ != null?

[paulmeyer, 2016/12/13 20:57:30]

Per offline discussion, I will continue checking the RVH, but will store it as
(process ID, routing ID) pair instead of a pointer.

+      drag_source_rph_ != target_rwh->GetProcess()) {
+    return;
+  }
 
+  current_rwh_for_drag_ = target_rwh->GetWeakPtr();
+  current_rvh_for_drag_ = 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());
@@ -1171,6 +1182,11 @@ int WebContentsViewAura::OnDragUpdated(const ui::DropTargetEvent& event) {
           web_contents_->GetRenderViewHost()->GetWidget()->GetView(),
           event.location(), &transformed_pt);
 
+  if (drag_source_rvh_ == web_contents_->GetRenderViewHost() &&
+      drag_source_rph_ != target_rwh->GetProcess()) {
+    return ui::DragDropTypes::DRAG_NONE;
+  }
+
   if (target_rwh != current_rwh_for_drag_.get()) {
     if (current_rwh_for_drag_)
       current_rwh_for_drag_->DragTargetDragLeave();
@@ -1216,6 +1232,11 @@ int WebContentsViewAura::OnPerformDrop(const ui::DropTargetEvent& event) {
           web_contents_->GetRenderViewHost()->GetWidget()->GetView(),
           event.location(), &transformed_pt);
 
+  if (drag_source_rvh_ == web_contents_->GetRenderViewHost() &&
+      drag_source_rph_ != target_rwh->GetProcess()) {
+    return ui::DragDropTypes::DRAG_NONE;
+  }
+
   if (target_rwh != current_rwh_for_drag_.get()) {
     if (current_rwh_for_drag_)
       current_rwh_for_drag_->DragTargetDragLeave();