Wait for mousemove DOM event, before simulating mousedown for dragstart tests.

Wait for mousemove DOM event, before simulating mousedown for dragstart tests.

Waiting until mousemove DOM event is received is important, because
otherwise (because of lag introduced by forwarding events between
renderers) button-less mousemove event can arrive at the target frame
*after* mousedown event (confusing the renderer whether mouse button is
held down or not and therefore preventing dragstart from happening).

This CL seems to fix the flakiness that used to be locally reproducible
with --site-per-process.  Right now we consistently pass 20 repetitions
of CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0.

BUG=671445
Committed: https://crrev.com/eeb42c523def9bf6d2c514bf403f2cb67117cb8b
Cr-Commit-Position: refs/heads/master@{#438583}

[Łukasz Anforowicz, 2016-12-13 23:13:23]

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

[commit-bot: I haz the power, 2016-12-13 23:13:55]

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-12-14 01:48:13]

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

[Łukasz Anforowicz, 2016-12-14 01:48:13]

lfg@, can you PTAL?  This CL waits until cross-renderer forwarding of input
events settles down, and only then proceeds to simulating the next mouse event
necessary to start a drag - this should avoid the problem (and the source of
flakiness) you've outlined in https://crbug.com/671445#c7.  Does this CL go in
the right direction?  Or do you feel that this CL tries to paper over other
underlying issues (like the fact that forwarding should be avoided [but then see
https://crbug.com/671445#c8] and/or the fact that forwarding events to other
renderers can change the relative order of events).  WDYT?

[commit-bot: I haz the power, 2016-12-14 02:03:03]

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

[commit-bot: I haz the power, 2016-12-14 02:03:04]

Dry run: This issue passed the CQ dry run.

[lfg, 2016-12-14 16:00:46]

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

[lfg, 2016-12-14 16:00:47]

+kenrb

On 2016/12/14 01:48:13, Łukasz Anforowicz wrote:
> lfg@, can you PTAL?  This CL waits until cross-renderer forwarding of input
> events settles down, and only then proceeds to simulating the next mouse event
> necessary to start a drag - this should avoid the problem (and the source of
> flakiness) you've outlined in https://crbug.com/671445#c7.  Does this CL go in
> the right direction?  Or do you feel that this CL tries to paper over other
> underlying issues (like the fact that forwarding should be avoided [but then
see
> https://crbug.com/671445#c8] and/or the fact that forwarding events to other
> renderers can change the relative order of events).  WDYT?

I talked with Ken today, and he mentioned that this is a known issue: bug
647378. The drag IPCs are sent from the browser process directly to the
subframe, but the parent widget captures the mouse and ends up forwarding it to
the subframe.

Can you add a comment mentioning bug 647378 and that once that is fixed we don't
need to wait on this anymore, since the events shouldn't arrive out of order.
Other than that, lgtm.

[Łukasz Anforowicz, 2016-12-14 17:34:06]

On 2016/12/14 16:00:47, lfg wrote:
> +kenrb
> 
> On 2016/12/14 01:48:13, Łukasz Anforowicz wrote:
> > lfg@, can you PTAL?  This CL waits until cross-renderer forwarding of input
> > events settles down, and only then proceeds to simulating the next mouse
event
> > necessary to start a drag - this should avoid the problem (and the source of
> > flakiness) you've outlined in https://crbug.com/671445#c7.  Does this CL go
in
> > the right direction?  Or do you feel that this CL tries to paper over other
> > underlying issues (like the fact that forwarding should be avoided [but then
> see
> > https://crbug.com/671445#c8] and/or the fact that forwarding events to other
> > renderers can change the relative order of events).  WDYT?
> 
> I talked with Ken today, and he mentioned that this is a known issue: bug
> 647378. The drag IPCs are sent from the browser process directly to the
> subframe, but the parent widget captures the mouse and ends up forwarding it
to
> the subframe.
> 
> Can you add a comment mentioning bug 647378 and that once that is fixed we
don't
> need to wait on this anymore, since the events shouldn't arrive out of order.
> Other than that, lgtm.

Done.

[Łukasz Anforowicz, 2016-12-14 17:34:17]

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

[commit-bot: I haz the power, 2016-12-14 17:34:52]

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-12-14 18:40:23]

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

[commit-bot: I haz the power, 2016-12-14 18:40:24]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2016-12-14 19:02:38]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-12-14 19:02:39]

The patchset sent to the CQ was uploaded after l-g-t-m from lfg@chromium.org
Link to the patchset: https://codereview.chromium.org/2577553002/#ps20001
(title: "Expanded a comment as suggested by lfg@ (to refer to bug 647378).")

[commit-bot: I haz the power, 2016-12-14 19:03:53]

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-12-14 19:22:23]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481742158846230,
"parent_rev": "d9c06a47c376f79be54de5e3076521356c7a26e7", "commit_rev":
"feecd0e8395565226eff46d298155bd777174732"}

[commit-bot: I haz the power, 2016-12-14 19:22:52]

Description was changed from

==========
Wait for mousemove DOM event, before simulating mousedown for dragstart tests.

Waiting until mousemove DOM event is received is important, because
otherwise (because of lag introduced by forwarding events between
renderers) button-less mousemove event can arrive at the target frame
*after* mousedown event (confusing the renderer whether mouse button is
held down or not and therefore preventing dragstart from happening).

This CL seems to fix the flakiness that used to be locally reproducible
with --site-per-process.  Right now we consistently pass 20 repetitions
of CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0.

BUG=671445
==========

to

==========
Wait for mousemove DOM event, before simulating mousedown for dragstart tests.

Waiting until mousemove DOM event is received is important, because
otherwise (because of lag introduced by forwarding events between
renderers) button-less mousemove event can arrive at the target frame
*after* mousedown event (confusing the renderer whether mouse button is
held down or not and therefore preventing dragstart from happening).

This CL seems to fix the flakiness that used to be locally reproducible
with --site-per-process.  Right now we consistently pass 20 repetitions
of CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0.

BUG=671445

Review-Url: https://codereview.chromium.org/2577553002
==========

[commit-bot: I haz the power, 2016-12-14 19:22:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-14 19:25:46]

Description was changed from

==========
Wait for mousemove DOM event, before simulating mousedown for dragstart tests.

Waiting until mousemove DOM event is received is important, because
otherwise (because of lag introduced by forwarding events between
renderers) button-less mousemove event can arrive at the target frame
*after* mousedown event (confusing the renderer whether mouse button is
held down or not and therefore preventing dragstart from happening).

This CL seems to fix the flakiness that used to be locally reproducible
with --site-per-process.  Right now we consistently pass 20 repetitions
of CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0.

BUG=671445

Review-Url: https://codereview.chromium.org/2577553002
==========

to

==========
Wait for mousemove DOM event, before simulating mousedown for dragstart tests.

Waiting until mousemove DOM event is received is important, because
otherwise (because of lag introduced by forwarding events between
renderers) button-less mousemove event can arrive at the target frame
*after* mousedown event (confusing the renderer whether mouse button is
held down or not and therefore preventing dragstart from happening).

This CL seems to fix the flakiness that used to be locally reproducible
with --site-per-process.  Right now we consistently pass 20 repetitions
of CrossSiteSubframe/DragAndDropBrowserTest.DragImageBetweenFrames/0.

BUG=671445

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

[commit-bot: I haz the power, 2016-12-14 19:25:47]

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