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

content/browser/web_contents/web_contents_view_aura.h

Index: content/browser/web_contents/web_contents_view_aura.h
diff --git a/content/browser/web_contents/web_contents_view_aura.h b/content/browser/web_contents/web_contents_view_aura.h
index 68690959d5a6cf6b427f931f0ea78f704f68d0a4..ddce5ed98c6ac88141bb612db46f3b71256b6b63 100644
--- a/content/browser/web_contents/web_contents_view_aura.h
+++ b/content/browser/web_contents/web_contents_view_aura.h
@@ -87,6 +87,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 ValidDragTarget(RenderWidgetHostImpl* target_rwh) const;

[Charlie Reis, 2016/12/14 18:06:50]

nit: IsValidDragTarget

[paulmeyer, 2016/12/14 18:27:49]

Done.

+
   // Overridden from WebContentsView:
   gfx::NativeView GetNativeView() const override;
   gfx::NativeView GetContentNativeView() const override;
@@ -198,10 +203,16 @@ 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

[Charlie Reis, 2016/12/13 23:01:16]

Please elaborate in both comments what each int in the pair is, since it's very
easy to guess wrong.

[Łukasz Anforowicz, 2016/12/13 23:07:24]

Not sure if it is applicable here + don't want to block the CL, but please note
that there is GlobalRoutingID in content/browser/loader/global_routing_id.h (a
more strongly-typed alternative to std::pair<int, int>).

[Charlie Reis, 2016/12/14 18:06:50]

That's a good idea-- let's use that.  (Looks like it won't be a problem from
this file to depend on content/browser/loader, per DEPS.)

[paulmeyer, 2016/12/14 18:27:48]

Okay, GlobalRoutingID seems like a better thing to use here.

+  // avoid sending the drag exited message after leaving the current view.
+  std::pair<int, int> current_rvh_for_drag_;
+
+  // We track the IDs of the source RenderProcessHost and RenderViewHost from

[Charlie Reis, 2016/12/13 23:01:16]

Nasko brought up the question about why this isn't tracking RenderWidgetHost. 
That's because we want to support A-B-A within the same page, right?  It's worth
clarifying in the comment that we're intentionally tracking this at the page
level.

(This will matter as we get rid of RenderViewHost and turn it more into a page
structure.)

[paulmeyer, 2016/12/14 18:27:48]

RenderViewHost is tracked to check the "same-page" part, while the
RenderProcessHost is tracked to check the "cross-site" part. The RenderViewHost
isn't being used in place of RenderWidgetHost; rather it's the RenderProcessHost
that is being used in place of the RenderWidgetHost to allow A-B-A in same page.
I'll add to the comment to clarify this.

+  // which the current drag originated. These are used to ensure that drag
+  // events do not fire over a cross-site frame in the same page (with respect
+  // to the source frame). See crbug.com/666858.

[Charlie Reis, 2016/12/14 18:06:50]

We should also clarify that the RPH and RVH here may not be in the same process,
which isn't immediately obvious.  The RPH is from the frame that started the
drag (possibly an OOPIF), and the RVH corresponds to the main frame.

[paulmeyer, 2016/12/14 18:27:48]

Done.

+  int drag_source_rph_;

[Charlie Reis, 2016/12/13 23:01:16]

It's hard to tell from the name what this is.  Maybe drag_source_process_id_?

[paulmeyer, 2016/12/14 18:27:48]

Actually, I noticed there are some other uses of the term "source" and even
"drag_source" but aren't really the same as these, which is confusing. Because
of this, I'm going to change "source" to "start" in these, since they are
tracked specifically from dragStart.

I'll rename this to drag_start_process_id_.

[Charlie Reis, 2016/12/14 18:36:36]

Acknowledged.

+  std::pair<int, int> drag_source_rvh_;

[Charlie Reis, 2016/12/13 23:01:16]

drag_source_view_ids_?

[Charlie Reis, 2016/12/14 18:06:50]

Or drag_source_view_global_routing_id_.

[paulmeyer, 2016/12/14 18:27:48]

Renamed to drag_start_view_id_.

 
   // The overscroll gesture currently in progress.
   OverscrollMode current_overscroll_gesture_;