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

content/browser/web_contents/web_contents_view_aura.h

 // Copyright (c) 2012 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.
 
 #ifndef CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_VIEW_AURA_H_
 #define CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_VIEW_AURA_H_
 
 #include <memory>
 #include <vector>
 
@@ skipping 69 matching lines @@
   void CompleteOverscrollNavigation(OverscrollMode mode);
 
   void OverscrollUpdateForWebContentsDelegate(float delta_y);
 
   ui::TouchSelectionController* GetSelectionController() const;
   TouchSelectionControllerClientAura* GetSelectionControllerClient() const;
 
   // 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;
   gfx::NativeWindow GetTopLevelNativeWindow() const override;
   void GetScreenInfo(ScreenInfo* screen_info) const override;
   void GetContainerBounds(gfx::Rect* out) const override;
   void SizeContents(const gfx::Size& size) override;
   void Focus() override;
   void SetInitialFocus() override;
   void StoreFocus() override;
@@ skipping 91 matching lines @@
   blink::WebDragOperationsMask current_drag_op_;
 
   std::unique_ptr<DropData> current_drop_data_;
 
   WebDragDestDelegate* drag_dest_delegate_;
 
   // We keep track of the RenderWidgetHost we're dragging over. If it changes
   // 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_;
 
   // This is the completed overscroll gesture. This is used for the animation
   // callback that happens in response to a completed overscroll gesture.
   OverscrollMode completed_overscroll_gesture_;
 
   // This manages the overlay window that shows the screenshot during a history
   // navigation triggered by the overscroll gesture.
   std::unique_ptr<OverscrollNavigationOverlay> navigation_overlay_;
 
   std::unique_ptr<GestureNavSimple> gesture_nav_simple_;
 
   bool init_rwhv_with_null_parent_for_testing_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsViewAura);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_WEB_CONTENTS_WEB_CONTENTS_VIEW_AURA_H_