Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Reland "Make pointer capture work in same origin frame"" #15888

Closed
wants to merge 1 commit into from

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

This reverts commit 081ad5e9f15e8b6e7c6cafe9ba408988b6e78bdc.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 641316 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMDgxYWQ1ZTlmMTVlOGI2ZTdjNmNhZmU5YmE0MDg5ODhiNmU3OGJkYww

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29/78993

Sample Failed Step: webkit_layout_tests

Sample Flaky Test: virtual/mouseevent_fractional/fast/events/pointerevents/mouse-pointer-capture-in-iframe.html

Original change's description:

Reland "Make pointer capture work in same origin frame"

This is a reland of d44248f2a896421c1382c1dbbf0d7e1a9b798470

Original change is reverted by findit due to test flaky.
The flaky test is:
fast/events/pointerevents/mouse-pointer-capture-in-iframe.html

This is reland with fixing tests.

Original change's description:

Make pointer capture work in same origin frame

We used to send mouse event to the subframe before apply the pointer
capture target, and it causes the pointer capture doesn't work when
capture to a outer frame target and move to inner frame.

This CL changes three things:

  1. 'Rename' the |capturing_mouse_events_element_| to
    |capturing_subframe_element_| as it only used for the frame capture
    (There is plan to remove the frame capture logic once we have pointer
    capture work correctly)
  2. On HandleMouse*Event, instead of always perform a hit test, we use
    either frame capture target or pointer capture target to re-construct
    the HitTestResult.
  3. When using the capture target, update the hover active state for
    capture target.

This change makes captured pointer event sent correctly when over
same origin frame, and also decrease the hit_test_count because of
frame capturing.

Note that after this change, we still NOT allow set/release pointer
capture across same-origin frame as the pointer id and active statue
is per frame.

See design doc:
https://docs.google.com/document/d/1cOZu98UuKk5bdARUQKmj2Q8YoEMpd9l78T0k-cf5ttc/

This change is under a blink flag UnifiedPointerCaptureInBlink.

Change-Id: I61c6a02086535d2a145df9d414df0bdc9673101e
Bug: 936190, 919908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1446556
Commit-Queue: Ella Ge <eirage@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#640055}

Bug: 936190, 919908
Change-Id: Icc374c16b1aee0b9125593801c8787f28c7db554
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1520941
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Commit-Queue: Ella Ge <eirage@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641316}

Change-Id: I9fb0a95a8736bbb22c8c7e247b00cf3263820f76
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 936190, 919908

Reviewed-on: https://chromium-review.googlesource.com/1527201
WPT-Export-Revision: 0467409e6ea232bf6be4f20a672806440a1f7970

This reverts commit 081ad5e9f15e8b6e7c6cafe9ba408988b6e78bdc.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 641316 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMDgxYWQ1ZTlmMTVlOGI2ZTdjNmNhZmU5YmE0MDg5ODhiNmU3OGJkYww

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29/78993

Sample Failed Step: webkit_layout_tests

Sample Flaky Test: virtual/mouseevent_fractional/fast/events/pointerevents/mouse-pointer-capture-in-iframe.html

Original change's description:
> Reland "Make pointer capture work in same origin frame"
>
> This is a reland of d44248f2a896421c1382c1dbbf0d7e1a9b798470
>
> Original change is reverted by findit due to test flaky.
> The flaky test is:
> fast/events/pointerevents/mouse-pointer-capture-in-iframe.html
>
> This is reland with fixing tests.
>
> Original change's description:
> > Make pointer capture work in same origin frame
> >
> > We used to send mouse event to the subframe before apply the pointer
> > capture target, and it causes the pointer capture doesn't work when
> > capture to a outer frame target and move to inner frame.
> >
> > This CL changes three things:
> > 1. 'Rename' the |capturing_mouse_events_element_| to
> > |capturing_subframe_element_| as it only used for the frame capture
> > (There is plan to remove the frame capture logic once we have pointer
> > capture work correctly)
> > 2. On HandleMouse*Event, instead of always perform a hit test, we use
> > either frame capture target or pointer capture target to re-construct
> > the HitTestResult.
> > 3. When using the capture target, update the hover active state for
> > capture target.
> >
> > This change makes captured pointer event sent correctly when over
> > same origin frame, and also decrease the hit_test_count because of
> > frame capturing.
> >
> > Note that after this change, we still NOT allow set/release pointer
> > capture across same-origin frame as the pointer id and active statue
> > is per frame.
> >
> > See design doc:
> > https://docs.google.com/document/d/1cOZu98UuKk5bdARUQKmj2Q8YoEMpd9l78T0k-cf5ttc/
> >
> > This change is under a blink flag UnifiedPointerCaptureInBlink.
> >
> > Change-Id: I61c6a02086535d2a145df9d414df0bdc9673101e
> > Bug: 936190, 919908
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1446556
> > Commit-Queue: Ella Ge <eirage@chromium.org>
> > Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
> > Reviewed-by: David Bokan <bokan@chromium.org>
> > Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#640055}
>
> Bug: 936190, 919908
> Change-Id: Icc374c16b1aee0b9125593801c8787f28c7db554
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1520941
> Reviewed-by: David Bokan <bokan@chromium.org>
> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
> Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
> Commit-Queue: Ella Ge <eirage@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#641316}

Change-Id: I9fb0a95a8736bbb22c8c7e247b00cf3263820f76
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 936190, 919908
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@KyleJu
Copy link
Contributor

KyleJu commented May 10, 2019

Close this PR because the Chromium CL has been abandoned.

@KyleJu KyleJu closed this May 10, 2019
@KyleJu KyleJu deleted the chromium-export-cl-1527201 branch May 10, 2019 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants