Making EventSender talk to the right WebWidget (for OOPIF support).

Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally EventSender transforms the
coordinates of the event before calling Blink, to make sure the
coordinates will work with the target widget.

Supporting EventSender via WebWidgetTestProxyBase requires
exposing WebWidgetTestProxyBase::web_view_test_proxy_base()
accessor and this in turn allows for clean-ups in some other
places - for example this allows to remove storing borrowed
pointers to TestRunner and WebViewTestProxyBase inside
WebWidgetTestClient.

BUG=616608
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/d322c5ca6d26e98fcd737cede72c7e1243e7cea5
Cr-Commit-Position: refs/heads/master@{#423762}

[Łukasz Anforowicz, 2016-06-02 21:05:19]

https://codereview.chromium.org/2036873002/diff/1/components/test_runner/even...
File components/test_runner/event_sender.cc (right):

https://codereview.chromium.org/2036873002/diff/1/components/test_runner/even...
components/test_runner/event_sender.cc:2725: return
widget()->handleInputEvent(event);
Even after this change http/tests/security/referrer-policy-redirect-link.html
keeps timing out.  Need further investigation...

https://codereview.chromium.org/2036873002/diff/1/components/test_runner/web_...
File components/test_runner/web_view_test_client.cc (right):

https://codereview.chromium.org/2036873002/diff/1/components/test_runner/web_...
components/test_runner/web_view_test_client.cc:53:
//web_test_proxy_base_->event_sender()->DoDragDrop(data, mask);
I am sure we can solve this somehow, but currently there is no way to get the
EventSender associated with the given WebLocalFrame...

[Łukasz Anforowicz, 2016-09-15 18:43:44]

Description was changed from

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

BUG=616608
==========

to

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each frame.
This means that javascript bindings injected into the frame can work
against a frame-specific widget.

BUG=616608
==========

[Łukasz Anforowicz, 2016-09-15 20:43:31]

lukasza@chromium.org changed reviewers:
+ lfg@chromium.org

[Łukasz Anforowicz, 2016-09-15 20:43:31]

Lucas, could you please take an _initial_ look?  (_initial_, because this CL
doesn't help in its current state - events from EventSender still don't work
from within OOPIFs).

Can you review the overall approach of 1) having a separate EventSender instance
per frame (i.e. per WebFrameTextProxyBase) and 2) having EventSender call
web_local_frame->localRoot()->frameWidget()->handleInputEvent(...) [rather than
web_view->handleInputEvent(...)].

Can you also help me understand why the new CL works fine without
--site-per-process, but with OOPIFs, I see that
EventDispatcher::dispatchEventPostProcess calls m_node->defaultEventHandler on
BODY element (rather than on A [aka HTMLAnchorElement] element as it does
without OOPIFs).  Do I need to transform the coordinates somewhere here?

[lfg, 2016-09-15 21:33:52]

On 2016/09/15 20:43:31, Łukasz Anforowicz wrote:
> Lucas, could you please take an _initial_ look?  (_initial_, because this CL
> doesn't help in its current state - events from EventSender still don't work
> from within OOPIFs).
> 
> Can you review the overall approach of 1) having a separate EventSender
instance
> per frame (i.e. per WebFrameTextProxyBase) and 2) having EventSender call
> web_local_frame->localRoot()->frameWidget()->handleInputEvent(...) [rather
than
> web_view->handleInputEvent(...)].
> 
> Can you also help me understand why the new CL works fine without
> --site-per-process, but with OOPIFs, I see that
> EventDispatcher::dispatchEventPostProcess calls m_node->defaultEventHandler on
> BODY element (rather than on A [aka HTMLAnchorElement] element as it does
> without OOPIFs).  Do I need to transform the coordinates somewhere here?

I started looking at it, but I think I'll patch it in and try to understand
this better tomorrow. My first question is why having one per-frame and not
one per-widget, since the input events are handled at the widget level? I
think the code would be better moved into WebWidgetTestProxy.

[Łukasz Anforowicz, 2016-09-15 22:38:42]

On 2016/09/15 21:33:52, lfg wrote:
> On 2016/09/15 20:43:31, Łukasz Anforowicz wrote:
> > Lucas, could you please take an _initial_ look?  (_initial_, because this CL
> > doesn't help in its current state - events from EventSender still don't work
> > from within OOPIFs).
> > 
> > Can you review the overall approach of 1) having a separate EventSender
> instance
> > per frame (i.e. per WebFrameTextProxyBase) and 2) having EventSender call
> > web_local_frame->localRoot()->frameWidget()->handleInputEvent(...) [rather
> than
> > web_view->handleInputEvent(...)].
> > 
> > Can you also help me understand why the new CL works fine without
> > --site-per-process, but with OOPIFs, I see that
> > EventDispatcher::dispatchEventPostProcess calls m_node->defaultEventHandler
on
> > BODY element (rather than on A [aka HTMLAnchorElement] element as it does
> > without OOPIFs).  Do I need to transform the coordinates somewhere here?
> 
> I started looking at it, but I think I'll patch it in and try to understand
> this better tomorrow. My first question is why having one per-frame and not
> one per-widget, since the input events are handled at the widget level? I
> think the code would be better moved into WebWidgetTestProxy.

The reason is rather silly - I've started working on this CL before you've
introduced WebWidgetTestProxy.

That said, when trying to switch right now from WebFrameTestProxy to
WebWidgetTestProxy I hit a few issues:

- WebViewTestClient::startDragging needs to call EventSender::DoDragDrop, but
has only access to WebLocalFrame and/or WebView.  It was relatively easy to add
content-shell-layer method
(WebTestDelegate/BlinkTestRunner::GetWebFrameTestProxyBase) to convert
WebLocalFrame* into WebFrameTestProxyBase*, but it seems difficult for "widget"
case, because RenderWidget is internal to content (i.e. there is no
RenderWidget::FromWidget visible to content-shell-layer).

- Similar trouble shows up in WebFrameTestClient::showContextMenu (which has
access to WebLocalFrame / WebFrameTestProxyBase and needs to call something in
EventSender).

- I've also started thinking that tests should not assume that EventSender's
state is shared between frames.  I think this is more uniformly (and therefore
robustly) encouraged with EventSender-per-frame model (because with
EventSender-per-widget model different site isolation policies can result in
different number of widgets and different sharing - what works today thanks to
accidentally shared state might not work in another site isolation mode, right?)

Let's first figure out why clicking doesn't work in the current state of the CL
and then we can chat afterwards whether EventSender should be owned by
WebFrameTestProxyBase vs WebWidgetTestProxyBase (in the short-term and in the
long-term - possibly with more WebView/WebWidget split some of the issues above
will go away in the long-term).

[Łukasz Anforowicz, 2016-09-28 22:20:20]

Description was changed from

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each frame.
This means that javascript bindings injected into the frame can work
against a frame-specific widget.

BUG=616608
==========

to

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally WebWidgetTestProxyBase
transforms the coordinates of the event before calling Blink,
to make sure the coordinates will work with the target widget.

BUG=616608
==========

[Łukasz Anforowicz, 2016-09-28 22:31:08]

Description was changed from

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally WebWidgetTestProxyBase
transforms the coordinates of the event before calling Blink,
to make sure the coordinates will work with the target widget.

BUG=616608
==========

to

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally WebWidgetTestProxyBase
transforms the coordinates of the event before calling Blink,
to make sure the coordinates will work with the target widget.

Supporting EventSender via WebWidgetTestProxyBase requires
exposing WebWidgetTestProxyBase::web_view_test_proxy_base()
accessor and this in turn allows for clean-ups in some other
places - for example this allows to remove storing borrowed
pointers to TestRunner and WebViewTestProxyBase inside
WebWidgetTestClient.

BUG=616608
==========

[Łukasz Anforowicz, 2016-09-28 22:33:26]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-28 22:33:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Łukasz Anforowicz, 2016-09-28 22:44:32]

Description was changed from

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally WebWidgetTestProxyBase
transforms the coordinates of the event before calling Blink,
to make sure the coordinates will work with the target widget.

Supporting EventSender via WebWidgetTestProxyBase requires
exposing WebWidgetTestProxyBase::web_view_test_proxy_base()
accessor and this in turn allows for clean-ups in some other
places - for example this allows to remove storing borrowed
pointers to TestRunner and WebViewTestProxyBase inside
WebWidgetTestClient.

BUG=616608
==========

to

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally WebWidgetTestProxyBase
transforms the coordinates of the event before calling Blink,
to make sure the coordinates will work with the target widget.

Supporting EventSender via WebWidgetTestProxyBase requires
exposing WebWidgetTestProxyBase::web_view_test_proxy_base()
accessor and this in turn allows for clean-ups in some other
places - for example this allows to remove storing borrowed
pointers to TestRunner and WebViewTestProxyBase inside
WebWidgetTestClient.

BUG=616608
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-09-29 00:46:55]

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

[commit-bot: I haz the power, 2016-09-29 00:46:56]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Łukasz Anforowicz, 2016-09-29 20:45:58]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 20:46:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 21:38:05]

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

[commit-bot: I haz the power, 2016-09-29 21:38:06]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[lfg, 2016-09-29 22:12:52]

https://codereview.chromium.org/2036873002/diff/100001/components/test_runner...
File components/test_runner/event_sender.cc (right):

https://codereview.chromium.org/2036873002/diff/100001/components/test_runner...
components/test_runner/event_sender.cc:1355: std::unique_ptr<WebInputEvent>
scaled_event =
This is doing more than scaling, rename it?

https://codereview.chromium.org/2036873002/diff/100001/content/test/layouttes...
File content/test/layouttest_support.cc (right):

https://codereview.chromium.org/2036873002/diff/100001/content/test/layouttes...
content/test/layouttest_support.cc:207: switch
(web_widget_test_proxy_base->subtype()) {
I'm not totally sure, but I think you could get rid of the subtype by doing:
web_widget_test_proxy_base->web_widget()->isWebView(). Similar to the DCHECKs
above, you can also use isWebFrameWidget().

Either way, this is a bit ugly. Can you add a TODO(lfg): Remove once RenderView
no longer inherits from RenderWidget ?

[Łukasz Anforowicz, 2016-09-29 23:42:36]

Thanks Lucas,

What I did in patchset 6 was rather broken (as shown by the redness of the
tryjobs...) - what I did in ui/events/blink/blink_event_util.cc didn't handle
all event types.  In patchset 8 I simplified my changes and they should work for
all event types.  I don't quite like that the caller has to check against
nullptr (returned if no transformation is needed) but this was already true for
the old, already existing "scaling" function.

I am keeping my fingers crossed for the tryjobs.  If there is any redness, then
I probably should take a look and fix it before wasting your time on reviewing
my changes.

https://codereview.chromium.org/2036873002/diff/100001/components/test_runner...
File components/test_runner/event_sender.cc (right):

https://codereview.chromium.org/2036873002/diff/100001/components/test_runner...
components/test_runner/event_sender.cc:1355: std::unique_ptr<WebInputEvent>
scaled_event =
On 2016/09/29 22:12:52, lfg wrote:
> This is doing more than scaling, rename it?

Done (in patchset 8):
- s/unscaled_event/raw_event/
- s/scaled_event/widget_event/

https://codereview.chromium.org/2036873002/diff/100001/content/test/layouttes...
File content/test/layouttest_support.cc (right):

https://codereview.chromium.org/2036873002/diff/100001/content/test/layouttes...
content/test/layouttest_support.cc:207: switch
(web_widget_test_proxy_base->subtype()) {
On 2016/09/29 22:12:52, lfg wrote:
> I'm not totally sure, but I think you could get rid of the subtype by doing:
> web_widget_test_proxy_base->web_widget()->isWebView(). Similar to the DCHECKs
> above, you can also use isWebFrameWidget().

Thank you for the suggestion - things look much better this way (done in
patchset 7).
> 
> Either way, this is a bit ugly.
> Can you add a TODO(lfg): Remove once RenderView
> no longer inherits from RenderWidget ?

Sure (done in patchset 8).

[Łukasz Anforowicz, 2016-09-29 23:45:19]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 23:45:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 01:21:07]

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

[commit-bot: I haz the power, 2016-09-30 01:21:09]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2016-09-30 15:29:20]

Lucas, I think this CL is now ready for a real review - can you please take a
look?

[Łukasz Anforowicz, 2016-09-30 18:47:27]

Description was changed from

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally WebWidgetTestProxyBase
transforms the coordinates of the event before calling Blink,
to make sure the coordinates will work with the target widget.

Supporting EventSender via WebWidgetTestProxyBase requires
exposing WebWidgetTestProxyBase::web_view_test_proxy_base()
accessor and this in turn allows for clean-ups in some other
places - for example this allows to remove storing borrowed
pointers to TestRunner and WebViewTestProxyBase inside
WebWidgetTestClient.

BUG=616608
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally EventSender transforms the
coordinates of the event before calling Blink, to make sure the
coordinates will work with the target widget.

Supporting EventSender via WebWidgetTestProxyBase requires
exposing WebWidgetTestProxyBase::web_view_test_proxy_base()
accessor and this in turn allows for clean-ups in some other
places - for example this allows to remove storing borrowed
pointers to TestRunner and WebViewTestProxyBase inside
WebWidgetTestClient.

BUG=616608
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[lfg, 2016-09-30 21:12:12]

Thanks, lgtm!

[Łukasz Anforowicz, 2016-09-30 21:25:03]

lukasza@chromium.org changed reviewers:
+ oshima@chromium.org

[Łukasz Anforowicz, 2016-09-30 21:25:04]

oshima@, could you please take a quick look?  You've introduced
EventSender::ScaleEvent in r402191 - because of this I wanted to double-check
with you this CL, where I am replacing it (almost) everywhere with
EventSender::TransformScreenToWidgetCoordinates (which in addition to scaling,
also shifts/translates coordinates to the widget-specific origin).

It is probably best to start reading with:

- EventSender::HandleInputEventOnViewOrPopup in
components/test_runner/event_sender.cc

- TransformScreenToWidgetCoordinates in content/test/layouttest_support.cc

- TranslateAndScaleWebInputEvent in ui/events/blink/blink_event_util.cc

[Łukasz Anforowicz, 2016-10-03 20:15:14]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-03 20:16:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-03 21:40:41]

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

[commit-bot: I haz the power, 2016-10-03 21:40:42]

Dry run: This issue passed the CQ dry run.

[oshima, 2016-10-03 23:34:33]

On 2016/09/30 21:25:04, Łukasz Anforowicz wrote:
> oshima@, could you please take a quick look?  You've introduced
> EventSender::ScaleEvent in r402191 - because of this I wanted to double-check
> with you this CL, where I am replacing it (almost) everywhere with
> EventSender::TransformScreenToWidgetCoordinates (which in addition to scaling,
> also shifts/translates coordinates to the widget-specific origin).
> 
> It is probably best to start reading with:
> 
> - EventSender::HandleInputEventOnViewOrPopup in
> components/test_runner/event_sender.cc
> 
> - TransformScreenToWidgetCoordinates in content/test/layouttest_support.cc
> 
> - TranslateAndScaleWebInputEvent in ui/events/blink/blink_event_util.cc

Sorry about the delay. I'll take a look at this evening.

[oshima, 2016-10-04 17:46:26]

lgtm with optional nit

https://codereview.chromium.org/2036873002/diff/160001/components/test_runner...
File components/test_runner/event_sender.cc (right):

https://codereview.chromium.org/2036873002/diff/160001/components/test_runner...
components/test_runner/event_sender.cc:2809: if
(!WebInputEvent::isKeyboardEventType(popup_friendly_event->type))
nit: looks like we can cehck the type first before converting the event?

[Łukasz Anforowicz, 2016-10-04 21:40:46]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 21:41:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Łukasz Anforowicz, 2016-10-04 21:42:18]

https://codereview.chromium.org/2036873002/diff/160001/components/test_runner...
File components/test_runner/event_sender.cc (right):

https://codereview.chromium.org/2036873002/diff/160001/components/test_runner...
components/test_runner/event_sender.cc:2809: if
(!WebInputEvent::isKeyboardEventType(popup_friendly_event->type))
On 2016/10/04 17:46:25, oshima wrote:
> nit: looks like we can cehck the type first before converting the event?

Done.

[Łukasz Anforowicz, 2016-10-04 21:43:17]

lukasza@chromium.org changed reviewers:
+ jochen@chromium.org

[Łukasz Anforowicz, 2016-10-04 21:43:18]

jochen@, could you please take a look?

[commit-bot: I haz the power, 2016-10-04 23:18:25]

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

[commit-bot: I haz the power, 2016-10-04 23:18:26]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2016-10-06 11:46:02]

lgtm

[Łukasz Anforowicz, 2016-10-06 14:01:25]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-10-06 14:01:25]

The patchset sent to the CQ was uploaded after l-g-t-m from lfg@chromium.org,
oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2036873002/#ps200001
(title: "Addressed CR feedback from oshima@.")

[Łukasz Anforowicz, 2016-10-06 14:01:28]

The CQ bit was unchecked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-10-06 14:06:32]

lukasza@chromium.org changed reviewers:
+ tdresser@chromium.org

[Łukasz Anforowicz, 2016-10-06 14:06:33]

tdresser@ - can you PTAL at:
    ui/events/blink/blink_event_util.cc
    ui/events/blink/blink_event_util.h

[tdresser, 2016-10-06 14:10:37]

LGTM

https://codereview.chromium.org/2036873002/diff/200001/ui/events/blink/blink_...
File ui/events/blink/blink_event_util.h (right):

https://codereview.chromium.org/2036873002/diff/200001/ui/events/blink/blink_...
ui/events/blink/blink_event_util.h:57: int delta_y,
Should we pass a Point here instead?

[Łukasz Anforowicz, 2016-10-06 16:34:38]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 16:34:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 17:38:50]

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

[commit-bot: I haz the power, 2016-10-06 17:38:51]

Dry run: Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[Łukasz Anforowicz, 2016-10-06 17:40:42]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 17:41:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Łukasz Anforowicz, 2016-10-06 18:03:19]

Thanks for the suggestion.

https://codereview.chromium.org/2036873002/diff/200001/ui/events/blink/blink_...
File ui/events/blink/blink_event_util.h (right):

https://codereview.chromium.org/2036873002/diff/200001/ui/events/blink/blink_...
ui/events/blink/blink_event_util.h:57: int delta_y,
On 2016/10/06 14:10:37, tdresser wrote:
> Should we pass a Point here instead?

I've ended up using gfx::Vector2d for this.

[commit-bot: I haz the power, 2016-10-06 18:26:43]

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

[commit-bot: I haz the power, 2016-10-06 18:26:44]

Dry run: Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[Łukasz Anforowicz, 2016-10-06 18:34:57]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-10-06 18:34:57]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
oshima@chromium.org, tdresser@chromium.org, lfg@chromium.org
Link to the patchset: https://codereview.chromium.org/2036873002/#ps240001
(title: "Actually, in //ui layer it seems more appropriate to use gfx::Vector2d,
rather than blink::WebPoint.")

[commit-bot: I haz the power, 2016-10-06 18:35:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-06 20:33:20]

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

[commit-bot: I haz the power, 2016-10-06 20:33:22]

Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[Łukasz Anforowicz, 2016-10-06 20:36:57]

Unfortunately, I see that this CL doesn't pass linux_site_isolation tryjobs
anymore (it was passing them just 2 days ago... :-( ).  I'll have to investigate
why the CL under review doesn't work anymore after r423030.

[Łukasz Anforowicz, 2016-10-06 20:58:45]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 20:59:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Łukasz Anforowicz, 2016-10-06 21:02:48]

lukasza@chromium.org changed reviewers:
+ bokan@chromium.org

[Łukasz Anforowicz, 2016-10-06 21:02:49]

bokan@, could you PTAL at the changes I've made (trying to account for your
r423030) in the last patchset to content/test/layouttest_support.cc?  Does this
look right? (tests started passing locally after this change, so I hope this is
the right thing to do, but I am not at all familiar with the
widget/layout/views/windows area...).

[bokan, 2016-10-06 22:38:17]

looks right to me. lgtm

[Łukasz Anforowicz, 2016-10-06 22:42:34]

The CQ bit was unchecked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-10-06 22:42:42]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-10-06 22:42:42]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
oshima@chromium.org, tdresser@chromium.org, lfg@chromium.org
Link to the patchset: https://codereview.chromium.org/2036873002/#ps280001
(title: "Use RenderWidget::viewRect() [rather than windowRect()] in
TransformScreenToWidgetCoordinates.")

[commit-bot: I haz the power, 2016-10-06 22:43:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-07 00:32:58]

Committed patchset #15 (id:280001)

[commit-bot: I haz the power, 2016-10-07 00:35:17]

Description was changed from

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally EventSender transforms the
coordinates of the event before calling Blink, to make sure the
coordinates will work with the target widget.

Supporting EventSender via WebWidgetTestProxyBase requires
exposing WebWidgetTestProxyBase::web_view_test_proxy_base()
accessor and this in turn allows for clean-ups in some other
places - for example this allows to remove storing borrowed
pointers to TestRunner and WebViewTestProxyBase inside
WebWidgetTestClient.

BUG=616608
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Making EventSender talk to the right WebWidget (for OOPIF support).

After the CL, there is a separate EventSender instance for each
WebWidgetTestProxyBase.  Additionally EventSender transforms the
coordinates of the event before calling Blink, to make sure the
coordinates will work with the target widget.

Supporting EventSender via WebWidgetTestProxyBase requires
exposing WebWidgetTestProxyBase::web_view_test_proxy_base()
accessor and this in turn allows for clean-ups in some other
places - for example this allows to remove storing borrowed
pointers to TestRunner and WebViewTestProxyBase inside
WebWidgetTestClient.

BUG=616608
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/d322c5ca6d26e98fcd737cede72c7e1243e7cea5
Cr-Commit-Position: refs/heads/master@{#423762}
==========

[commit-bot: I haz the power, 2016-10-07 00:35:19]

Patchset 15 (id:??) landed as
https://crrev.com/d322c5ca6d26e98fcd737cede72c7e1243e7cea5
Cr-Commit-Position: refs/heads/master@{#423762}