New issue
Advanced search Search tips

Issue 837705 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 837684



Sign in to add a comment

Wire up cursors for WindowService as a library

Project Member Reported by sky@chromium.org, Apr 27 2018

Issue description

There is the single function SetCursor for this.
 

Comment 1 by sky@chromium.org, May 8 2018

Blocking: 841020

Comment 2 by sky@chromium.org, May 9 2018

Owner: est...@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by sky@chromium.org, May 29 2018

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12c660481b5e613f20f7c0a544145b6fdcf88989

commit 12c660481b5e613f20f7c0a544145b6fdcf88989
Author: Evan Stade <estade@chromium.org>
Date: Sat Jun 16 04:09:33 2018

ws2: Support cursors for embedded windows.

For top level client windows, the cursor is stored in
WmNativeWidgetAura. For embed roots, it's stored in ServerWindow.
CompoundEventFilter will retrieve the stored cursor while
handling mouse events by using aura::WindowDelegate::GetCursor.
Depending on the target of the mouse event, the WindowDelegate
will be either WindowDelegateImpl (which retrieves it from
ServerWindow or delegates to the top level's delegate) or
WmNativeWidgetAura (which returns its own copy of the cursor).

To test, hover over input field of KSV via:
1. chrome --keyboard-shortcut-viewer-app  Ctrl+Alt+/
or
2. ash_shell_with_content

Bug:  837705 
Change-Id: I247b41937aada5a9b161eb469d8230d06890a63b
Reviewed-on: https://chromium-review.googlesource.com/1068661
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567870}
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ash/BUILD.gn
[add] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ash/cursor_unittest.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ash/test/ash_test_base.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ash/test/ash_test_base.h
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ash/wm/non_client_frame_controller.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ash/wm/non_client_frame_controller.h
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ash/ws/window_service_delegate_impl.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ash/ws/window_service_delegate_impl.h
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/server_window.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/server_window.h
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/window_delegate_impl.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/window_service_delegate.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/window_service_delegate.h
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/window_tree.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/window_tree.h
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/window_tree_test_helper.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/services/ui/ws2/window_tree_test_helper.h
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/testing/buildbot/filters/mash.ash_unittests.filter
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ui/wm/core/compound_event_filter.cc
[modify] https://crrev.com/12c660481b5e613f20f7c0a544145b6fdcf88989/ui/wm/core/compound_event_filter.h

Comment 5 by sky@chromium.org, Jun 18 2018

Blocking: -841020
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a6dc11ea630378ea89ca4f587e5eebc666151120

commit a6dc11ea630378ea89ca4f587e5eebc666151120
Author: Evan Stade <estade@chromium.org>
Date: Thu Jun 28 18:51:40 2018

Handle custom/bitmap cursors in ws2.

Go through CursorFactoryOzone to load the PlatformCursor (which
confusingly could be CursorData, but for now is X11CursorOzone). Also
set fields on the cross-platform Cursor type so that code which relies
on it such as CursorWindowController::UpdateCursorImage will work.
That code path can be exercised with --ash-enable-cursor-motion-blur
although there is currently no use of custom cursors with ws2.

Bug:  837705 
Change-Id: I9f725c5a5350e28cf8bded6b53bf48759cb644f8
Reviewed-on: https://chromium-review.googlesource.com/1105562
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571213}
[modify] https://crrev.com/a6dc11ea630378ea89ca4f587e5eebc666151120/ash/cursor_unittest.cc
[modify] https://crrev.com/a6dc11ea630378ea89ca4f587e5eebc666151120/services/ui/public/interfaces/cursor/cursor.mojom
[modify] https://crrev.com/a6dc11ea630378ea89ca4f587e5eebc666151120/services/ui/public/interfaces/cursor/cursor_struct_traits.cc
[modify] https://crrev.com/a6dc11ea630378ea89ca4f587e5eebc666151120/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
[modify] https://crrev.com/a6dc11ea630378ea89ca4f587e5eebc666151120/services/ui/ws/threaded_image_cursors.cc
[modify] https://crrev.com/a6dc11ea630378ea89ca4f587e5eebc666151120/services/ui/ws2/window_tree.cc
[modify] https://crrev.com/a6dc11ea630378ea89ca4f587e5eebc666151120/ui/base/cursor/cursor_data.cc
[modify] https://crrev.com/a6dc11ea630378ea89ca4f587e5eebc666151120/ui/base/cursor/cursor_data.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55db9ad971f8dd277a6df660d864a997287233a7

commit 55db9ad971f8dd277a6df660d864a997287233a7
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Jun 29 01:23:25 2018

Revert "Handle custom/bitmap cursors in ws2."

This reverts commit a6dc11ea630378ea89ca4f587e5eebc666151120.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 571213 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2E2ZGMxMWVhNjMwMzc4ZWE4OWNhNGY1ODdlNWVlYmM2NjYxNTExMjAM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/28074

Sample Failed Step: ash_unittests

Original change's description:
> Handle custom/bitmap cursors in ws2.
> 
> Go through CursorFactoryOzone to load the PlatformCursor (which
> confusingly could be CursorData, but for now is X11CursorOzone). Also
> set fields on the cross-platform Cursor type so that code which relies
> on it such as CursorWindowController::UpdateCursorImage will work.
> That code path can be exercised with --ash-enable-cursor-motion-blur
> although there is currently no use of custom cursors with ws2.
> 
> Bug:  837705 
> Change-Id: I9f725c5a5350e28cf8bded6b53bf48759cb644f8
> Reviewed-on: https://chromium-review.googlesource.com/1105562
> Commit-Queue: Evan Stade <estade@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#571213}

Change-Id: I5bda2cd762ef24cbf5b31eac96a7e0d1e96e84c8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  837705 
Reviewed-on: https://chromium-review.googlesource.com/1119586
Cr-Commit-Position: refs/heads/master@{#571352}
[modify] https://crrev.com/55db9ad971f8dd277a6df660d864a997287233a7/ash/cursor_unittest.cc
[modify] https://crrev.com/55db9ad971f8dd277a6df660d864a997287233a7/services/ui/public/interfaces/cursor/cursor.mojom
[modify] https://crrev.com/55db9ad971f8dd277a6df660d864a997287233a7/services/ui/public/interfaces/cursor/cursor_struct_traits.cc
[modify] https://crrev.com/55db9ad971f8dd277a6df660d864a997287233a7/services/ui/public/interfaces/cursor/cursor_struct_traits_unittest.cc
[modify] https://crrev.com/55db9ad971f8dd277a6df660d864a997287233a7/services/ui/ws/threaded_image_cursors.cc
[modify] https://crrev.com/55db9ad971f8dd277a6df660d864a997287233a7/services/ui/ws2/window_tree.cc
[modify] https://crrev.com/55db9ad971f8dd277a6df660d864a997287233a7/ui/base/cursor/cursor_data.cc
[modify] https://crrev.com/55db9ad971f8dd277a6df660d864a997287233a7/ui/base/cursor/cursor_data.h

Comment 8 Deleted

Status: Fixed (was: Started)
crrev.com/c12c1537204fef8843d2323dd5f4 was the re-land.

Cursors are working for the browser, both in native views and on sites like https://davidwalsh.name/demo/css-custom-cursor.php

They don't work for embedded frames, seemingly, like in the codepen widgets here: https://css-tricks.com/using-css-cursors/ but I believe that's a shortcoming of Blink.

Sign in to add a comment