Make input event forwarding work in --site-per-process.

Make input event forwarding work in --site-per-process.

This moves forwardInputEvent to WebRemoteFrame and fixes a few uses of
RemoteFrames.

BUG=346764
TEST=Input events work in out-of-process iframes.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183569

[Charlie Reis, 2014-10-10 03:42:38]

creis@chromium.org changed reviewers:
+ kenrb@chromium.org

[Charlie Reis, 2014-10-10 03:42:39]

Ken, can you take a look?  This gets us to forward input events via RemoteFrames
and WebRemoteFrameClient.  The Chrome half is
https://codereview.chromium.org/647653002/, and I think they can land in either
order.

https://codereview.chromium.org/642203002/diff/1/Source/web/WebInputEventConv...
File Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/642203002/diff/1/Source/web/WebInputEventConv...
Source/web/WebInputEventConversion.cpp:492: ScrollView* view = widget ?
toScrollView(widget->parent()) : 0;
This was a bit awkward, but we don't have a widget for the RemoteFrame.  Not
sure if we would have a ScrollView for it even if we did.

[kenrb, 2014-10-10 19:59:08]

lgtm

https://codereview.chromium.org/642203002/diff/1/Source/web/WebInputEventConv...
File Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/642203002/diff/1/Source/web/WebInputEventConv...
Source/web/WebInputEventConversion.cpp:492: ScrollView* view = widget ?
toScrollView(widget->parent()) : 0;
On 2014/10/10 03:42:39, Charlie Reis wrote:
> This was a bit awkward, but we don't have a widget for the RemoteFrame.  Not
> sure if we would have a ScrollView for it even if we did.

I am guessing webFrame->frame()->view() is null in ChromeClientImpl? We might
want a comment about that, because it should be changed back to a reference
later, when RemoteFrame has a RemoteFrameView. After that, this will need to
check widget.isScrollView(), because RemoteFrameView is a Widget but not a
ScrollView.

[dcheng, 2014-10-10 20:18:51]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2014-10-10 20:18:52]

Just a drive by

https://codereview.chromium.org/642203002/diff/1/Source/core/page/ChromeClient.h
File Source/core/page/ChromeClient.h (right):

https://codereview.chromium.org/642203002/diff/1/Source/core/page/ChromeClien...
Source/core/page/ChromeClient.h:231: virtual void
forwardInputEvent(blink::RemoteFrame*, blink::Event*) { }
Nit: let's remove blink:: while we're changing these lines, since everything is
in the same namespace now.

[Charlie Reis, 2014-10-10 20:25:24]

creis@chromium.org changed reviewers:
+ japhet@chromium.org

[Charlie Reis, 2014-10-10 20:25:24]

Thanks!  Nits addressed.

Nate, can you review for OWNERS approval?

https://codereview.chromium.org/642203002/diff/1/Source/core/page/ChromeClient.h
File Source/core/page/ChromeClient.h (right):

https://codereview.chromium.org/642203002/diff/1/Source/core/page/ChromeClien...
Source/core/page/ChromeClient.h:231: virtual void
forwardInputEvent(blink::RemoteFrame*, blink::Event*) { }
On 2014/10/10 20:18:51, dcheng wrote:
> Nit: let's remove blink:: while we're changing these lines, since everything
is
> in the same namespace now.

Done.

https://codereview.chromium.org/642203002/diff/1/Source/web/WebInputEventConv...
File Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/642203002/diff/1/Source/web/WebInputEventConv...
Source/web/WebInputEventConversion.cpp:492: ScrollView* view = widget ?
toScrollView(widget->parent()) : 0;
On 2014/10/10 19:59:08, kenrb wrote:
> On 2014/10/10 03:42:39, Charlie Reis wrote:
> > This was a bit awkward, but we don't have a widget for the RemoteFrame.  Not
> > sure if we would have a ScrollView for it even if we did.
> 
> I am guessing webFrame->frame()->view() is null in ChromeClientImpl? We might
> want a comment about that, because it should be changed back to a reference
> later, when RemoteFrame has a RemoteFrameView. After that, this will need to
> check widget.isScrollView(), because RemoteFrameView is a Widget but not a
> ScrollView. 

Yes, and done.

[Nate Chapin, 2014-10-10 20:37:06]

LGTM

https://codereview.chromium.org/642203002/diff/280001/Source/core/page/Chrome...
File Source/core/page/ChromeClient.h (right):

https://codereview.chromium.org/642203002/diff/280001/Source/core/page/Chrome...
Source/core/page/ChromeClient.h:231: virtual void
forwardInputEvent(RemoteFrame*, Event*) { }
Is the FIXME accurate? If not, I'm interested in discussing why this lives on
ChromeClient instead of RemoteFrameClient, but if this will go away soon, it
doesn't matter.

[kenrb, 2014-10-10 20:47:41]

On 2014/10/10 20:37:06, Nate Chapin wrote:
> LGTM
> 
>
https://codereview.chromium.org/642203002/diff/280001/Source/core/page/Chrome...
> File Source/core/page/ChromeClient.h (right):
> 
>
https://codereview.chromium.org/642203002/diff/280001/Source/core/page/Chrome...
> Source/core/page/ChromeClient.h:231: virtual void
> forwardInputEvent(RemoteFrame*, Event*) { }
> Is the FIXME accurate? If not, I'm interested in discussing why this lives on
> ChromeClient instead of RemoteFrameClient, but if this will go away soon, it
> doesn't matter.

It is accurate, although we don't have a clear timeline for it because it
depends on compositing surfaces. Renderer-to-renderer input event forwarding is
not expected to have acceptable responsiveness, especially if you have nested
out-of-process iframes, so the ultimate design is that the browser determines
the target of each event without having to check with any of the renderers.

[Charlie Reis, 2014-10-10 20:50:34]

On 2014/10/10 20:47:41, kenrb wrote:
> On 2014/10/10 20:37:06, Nate Chapin wrote:
> > Is the FIXME accurate? If not, I'm interested in discussing why this lives
on
> > ChromeClient instead of RemoteFrameClient, but if this will go away soon, it
> > doesn't matter.
> 
> It is accurate, although we don't have a clear timeline for it because it
> depends on compositing surfaces. Renderer-to-renderer input event forwarding
is
> not expected to have acceptable responsiveness, especially if you have nested
> out-of-process iframes, so the ultimate design is that the browser determines
> the target of each event without having to check with any of the renderers.

+1, we intend to remove but it is blocked on surfaces work.

I agree that RemoteFrameClient is probably a better place for this to live in
the meantime.  Ok if I move it over in my next CL?

[Charlie Reis, 2014-10-10 20:59:12]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2014-10-10 21:00:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/642203002/280001

[commit-bot: I haz the power, 2014-10-10 21:44:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-10 21:44:07]

Failed to apply patch for Source/web/WebInputEventConversion.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/web/WebInputEventConversion.cpp
  Hunk #1 FAILED at 484.
  Hunk #2 succeeded at 518 (offset -1 lines).
  Hunk #3 succeeded at 648 (offset -1 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
Source/web/WebInputEventConversion.cpp.rej

Patch:       Source/web/WebInputEventConversion.cpp
Index: Source/web/WebInputEventConversion.cpp
diff --git a/Source/web/WebInputEventConversion.cpp
b/Source/web/WebInputEventConversion.cpp
index
362b99b254876736cbb944ee74d105b86e6a2732..b69412bee92ea14d33fe024220fdad787a923b25
100644
--- a/Source/web/WebInputEventConversion.cpp
+++ b/Source/web/WebInputEventConversion.cpp
@@ -484,12 +484,15 @@ static IntPoint
convertAbsoluteLocationForRenderObject(const LayoutPoint& locati
     return
roundedIntPoint(convertAbsoluteLocationForRenderObjectFloat(location,
renderObject));
 }
 
-static void updateWebMouseEventFromCoreMouseEvent(const MouseRelatedEvent&
event, const Widget& widget, const RenderObject& renderObject, WebMouseEvent&
webEvent)
+// FIXME: Change |widget| to const Widget& after RemoteFrames get
+// RemoteFrameViews.  When this happens, check widget.isScrollView(), because
+// RemoteFrameView is a Widget but not a ScrollView.
+static void updateWebMouseEventFromCoreMouseEvent(const MouseRelatedEvent&
event, const Widget* widget, const RenderObject& renderObject, WebMouseEvent&
webEvent)
 {
     webEvent.timeStampSeconds = event.timeStamp() / millisPerSecond;
     webEvent.modifiers = getWebInputModifiers(event);
 
-    ScrollView* view =  toScrollView(widget.parent());
+    ScrollView* view = widget ? toScrollView(widget->parent()) : 0;
     IntPoint windowPoint = IntPoint(event.absoluteLocation().x(),
event.absoluteLocation().y());
     if (view)
         windowPoint = view->contentsToWindow(windowPoint);
@@ -519,7 +522,7 @@ WebMouseEventBuilder::WebMouseEventBuilder(const Widget*
widget, const RenderObj
     else
         return; // Skip all other mouse events.
 
-    updateWebMouseEventFromCoreMouseEvent(event, *widget, *renderObject,
*this);
+    updateWebMouseEventFromCoreMouseEvent(event, widget, *renderObject, *this);
 
     switch (event.button()) {
     case LeftButton:
@@ -649,7 +652,7 @@ WebMouseWheelEventBuilder::WebMouseWheelEventBuilder(const
Widget* widget, const
     if (event.type() != EventTypeNames::wheel && event.type() !=
EventTypeNames::mousewheel)
         return;
     type = WebInputEvent::MouseWheel;
-    updateWebMouseEventFromCoreMouseEvent(event, *widget, *renderObject,
*this);
+    updateWebMouseEventFromCoreMouseEvent(event, widget, *renderObject, *this);
     deltaX = -event.deltaX();
     deltaY = -event.deltaY();
     wheelTicksX = event.ticksX();

[commit-bot: I haz the power, 2014-10-10 21:45:07]

The CQ bit was unchecked by commit-bot@chromium.org

[Charlie Reis, 2014-10-10 21:51:44]

https://codereview.chromium.org/642203002/diff/520001/Source/web/WebInputEven...
File Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/642203002/diff/520001/Source/web/WebInputEven...
Source/web/WebInputEventConversion.cpp:493: FrameView* view = widget ?
toFrameView(widget->parent()) : 0;
Looks like this was updated recently to use FrameViews instead of ScrollViews,
so I removed that part of the comment.

[Charlie Reis, 2014-10-10 21:51:51]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2014-10-10 21:52:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/642203002/520001

[commit-bot: I haz the power, 2014-10-11 00:06:47]

Committed patchset #4 (id:520001) as 183569