New issue
Advanced search Search tips

Issue 837695 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 837684
issue 840380
issue 841020



Sign in to add a comment

Wire up properties in window-service as a library

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

Issue description

In particular WindowServiceClient::SetWindowProperty, WindowToWindowData, NewWindow, and NewTopLevel, need to be sure they are doing the right thing with properties. Where the right thing is mapping and/or validating properties.
 

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

Blocking: 840380

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

Blocking: 841020
Project Member

Comment 3 by bugdroid1@chromium.org, May 9 2018

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

commit 67eee9586343d858afae218edbdabff04cb16ee5
Author: Mike Wasserman <msw@chromium.org>
Date: Wed May 09 18:51:49 2018

ws: Add support for WindowServiceClient::SetWindowProperty

Add a PropertyConverter to WindowService to support property syncing.
Implement SetWindowProperty[Impl] on WindowServiceClient.
Add a unit test, split up the existing fixture, refine GL init.

Add a helper function for Ash window property registration.
(consolidate out-of-sync registrations between Chrome and Ash)
Add a helper function for window service init in Ash.

Make kRenderTitleAreaProperty public (mojo side used by Chrome)
Check that properties are not registered twice with the converter.
Optionally register kShadowElevationKey in helper (for Ash, not Chrome).
Update and add  crbug.com/837695  TODOs.

Bug:  837695 
Change-Id: Ic026d06e3aeb39bb7008f8d7fec2392fa08f176c
Reviewed-on: https://chromium-review.googlesource.com/1050575
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557260}
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/BUILD.gn
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/public/cpp/BUILD.gn
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/public/cpp/window_properties.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/public/cpp/window_properties.h
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/shell/content/client/shell_content_browser_client.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/window_manager.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/wm/property_util.h
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/wm/window_properties.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/wm/window_properties.h
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/ws/window_service_delegate_impl.cc
[add] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/ws/window_service_util.cc
[add] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ash/ws/window_service_util.h
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/chrome/browser/DEPS
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/chrome/browser/ash_service_registry.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/services/ui/ws2/BUILD.gn
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/services/ui/ws2/window_service.h
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/services/ui/ws2/window_service_client.h
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/services/ui/ws2/window_service_client_test_helper.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/services/ui/ws2/window_service_client_test_helper.h
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/services/ui/ws2/window_service_client_unittest.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ui/aura/mus/property_converter.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ui/aura/mus/property_converter.h
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ui/views/mus/desktop_window_tree_host_mus_unittest.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ui/views/mus/mus_client.cc
[modify] https://crrev.com/67eee9586343d858afae218edbdabff04cb16ee5/ui/wm/core/shadow_controller.cc

Project Member

Comment 4 by bugdroid1@chromium.org, May 11 2018

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

commit 7aec5f843746c07cab0be327fa1ee84cd4b66eee
Author: Mike Wasserman <msw@chromium.org>
Date: Fri May 11 21:07:52 2018

ws: Add property support to WindowServiceClient::NewTopLevelWindow

Call CreateAndParentTopLevelWindow from WindowServiceDelegateImpl.
Get the window type like ui/aura/mus/window_tree_client.cc
Pass PropertyConverter, etc. args to avoid ash::WindowManager.
Check access to possibly null WindowManager[Client], etc. ptrs.

Add a unit test, apply properties to windows in test delegates.

Bug:  837695 
Change-Id: I9f474c4b2701efc110b0738ae56fb5334d2fa641
Reviewed-on: https://chromium-review.googlesource.com/1055658
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558018}
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/frame/detached_title_area_renderer.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/frame/detached_title_area_renderer.h
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/test/ash_test_base.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/window_manager.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/wm/move_event_handler.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/wm/non_client_frame_controller.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/wm/non_client_frame_controller.h
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/wm/non_client_frame_controller_unittest.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/wm/top_level_window_factory.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/wm/top_level_window_factory.h
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/wm/top_level_window_factory_unittest.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/ws/window_service_delegate_impl.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/ash/ws/window_service_delegate_impl.h
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/services/ui/test_ws/test_ws.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/services/ui/ws2/test_window_service_delegate.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/services/ui/ws2/test_window_service_delegate.h
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/services/ui/ws2/window_service_client_test_helper.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/services/ui/ws2/window_service_client_test_helper.h
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/services/ui/ws2/window_service_client_unittest.cc
[modify] https://crrev.com/7aec5f843746c07cab0be327fa1ee84cd4b66eee/services/ui/ws2/window_service_delegate.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 14 2018

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

commit 72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21
Author: Mike Wasserman <msw@chromium.org>
Date: Mon May 14 06:09:18 2018

ws: Add property support to WindowServiceClient::NewWindow

Make a shared helper for getting the window type from properties.
Apply properties to the window like the old WindowTreeClient.

Bug:  837695 
Change-Id: Icfe1efcddb52b2c3ef9d3ed218e02c105885e230
Reviewed-on: https://chromium-review.googlesource.com/1056349
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558209}
[modify] https://crrev.com/72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21/ash/ws/window_service_delegate_impl.cc
[modify] https://crrev.com/72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21/services/ui/ws2/window_service_client_test_helper.cc
[modify] https://crrev.com/72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21/services/ui/ws2/window_service_client_test_helper.h
[modify] https://crrev.com/72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21/services/ui/ws2/window_service_client_unittest.cc
[modify] https://crrev.com/72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21/ui/aura/mus/property_utils.cc
[modify] https://crrev.com/72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21/ui/aura/mus/property_utils.h
[modify] https://crrev.com/72c2bf6c8f3d0adfbfe0a7fdab207073ccac6b21/ui/aura/mus/window_tree_client.cc

Project Member

Comment 6 by bugdroid1@chromium.org, May 15 2018

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

commit 892f5f33939fd2db292619dc72acaf0bc9bcaff8
Author: Mike Wasserman <msw@chromium.org>
Date: Tue May 15 00:22:36 2018

ws: Add property support for WindowToWindowData and client notifications

Add window properties to ws2 WindowData structs, like the original ws.
Add a helper function, PropertyConverter::GetTransportProperties.

Have ClientRoot notify clients on property changes, like bounds changes.
Add support for tracking client-initiated property changes.
(this only works for client roots? what about child windows?)

Encode test tracker property transport values as hex.
(so a false bool value is reported as "000..000" instead of "")

Add tests for WindowToWindowData and server-initiated property changes.

Bug:  837695 
Change-Id: I2a8e4ce7eeca0b8300cca9e20d668c178d77ee7e
Reviewed-on: https://chromium-review.googlesource.com/1058118
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558547}
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/client_change.h
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/client_root.cc
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/client_root.h
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/test_change_tracker.cc
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/window_service_client.cc
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/window_service_client.h
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/window_service_client_test_helper.cc
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/window_service_client_test_helper.h
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/services/ui/ws2/window_service_client_unittest.cc
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/ui/aura/mus/property_converter.cc
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/ui/aura/mus/property_converter.h
[modify] https://crrev.com/892f5f33939fd2db292619dc72acaf0bc9bcaff8/ui/aura/mus/window_tree_client.cc

Comment 7 by msw@chromium.org, May 15 2018

Status: Fixed (was: Assigned)
This is generally fixed.

Sign in to add a comment