Wire up aura::PropertyConverter for ash |
||||||||
Issue descriptionaura::PropertyConverter is used to convert properties for transport to server and back. For example, ui::mojom::WindowManager::kAlwaysOnTop_Property corresponds to aura::client::kAlwaysOnTopKey. We need to write an implementation of aura::PropertyConverter that can map between these values. Ideally this information would be baked into how the properties are defined (e.g. DEFINE_WINDOW_PROPERTY_KEY/DECLARE_WINDOW_PROPERTY_TYPE), but I suspect that would take some time.
,
Nov 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f21bef6f51847273fc2443ef995ae8b18d384810 commit f21bef6f51847273fc2443ef995ae8b18d384810 Author: sky <sky@chromium.org> Date: Wed Nov 09 04:29:30 2016 Adds some TODOs with references to bugs BUG= 663522 663526 663525 663617 663618 663619 TEST=none TBR=erg@chromium.org, msw@chromium.org R=erg@chromium.org, msw@chromium.org Review-Url: https://codereview.chromium.org/2481363005 Cr-Commit-Position: refs/heads/master@{#430861} [modify] https://crrev.com/f21bef6f51847273fc2443ef995ae8b18d384810/ui/views/mus/desktop_window_tree_host_mus.cc [modify] https://crrev.com/f21bef6f51847273fc2443ef995ae8b18d384810/ui/views/mus/mus_client.cc
,
Nov 11 2016
I'm taking a look.
,
Nov 11 2016
Hey Scott, I was initially trying to get clever using templates in the converter itself, but that gets tricky because some aura window property keys can be void* instead of aura::WindowProperty<T>*. It seems like we could convert all the native window properties to regular properties and nix the char/void* key type? [Get|Set]NativeWindowProperty types (I don't see what's blocking these from being non-native...): Profile* Profile::kProfileKey (only used in chrome/browser/*) ui::Window* kMusWindowKey (only used in ui/aura/mus/mus_util.cc) void* TooltipManager::kGroupingPropertyKey (only used in ui/views/*) ui/aura/window_unittest.cc examples I could instead try converting aura window properties to byte arrays, like mus? That seems like it should work in theory, though exported pointer property types couldn't be shared across mojo application bounds, even though it'd look like they are? This would likely also require an audit of users' expectations. Here's a breakdown of the aura property types: DECLARE_EXPORTED_WINDOW_PROPERTY_TYPE (14 types, 5 primitive, 9 pointers) DECLARE_WINDOW_PROPERTY_TYPE (43 types, 6 primitives, 37 pointers) I could also just go for a dumb brute force approach for now with the properties we care about... I'm open to other ideas, and willing to bail if you'd prefer someone with more experience take over... WDYT?
,
Nov 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28625ccf0b3bcbcf118f06024f78d975f7575d5b commit 28625ccf0b3bcbcf118f06024f78d975f7575d5b Author: msw <msw@chromium.org> Date: Sat Nov 12 02:12:33 2016 Implement aura::PropertyConverter for mus interop. Provide an implementation for the interface. TODO: Add other properties besides always-on-top. Use aura::Window's internal int64_t property accessors. TODO: This is wasteful for smaller types (eg. bool). Use the existing property type converters. TODO: Use Mojo's generated byte array Serialize function? Some misc. TODO ideas to improve this or follow up: TODO: Store aura::Window props by key *name*? TODO: Store aura::Window props as std::vector<uint8_t>? TODO: Optimize for smaller property types? TODO: Let clients extend the set of known properties? TODO: Nix WindowTreeClientDelegate::GetPropertyConverter? BUG= 663522 TEST=Automated R=sky@chromium.org TBR=tsepez@chromium.org Review-Url: https://codereview.chromium.org/2488353005 Cr-Commit-Position: refs/heads/master@{#431743} [modify] https://crrev.com/28625ccf0b3bcbcf118f06024f78d975f7575d5b/ui/aura/BUILD.gn [modify] https://crrev.com/28625ccf0b3bcbcf118f06024f78d975f7575d5b/ui/aura/mus/DEPS [add] https://crrev.com/28625ccf0b3bcbcf118f06024f78d975f7575d5b/ui/aura/mus/property_converter.cc [modify] https://crrev.com/28625ccf0b3bcbcf118f06024f78d975f7575d5b/ui/aura/mus/property_converter.h [modify] https://crrev.com/28625ccf0b3bcbcf118f06024f78d975f7575d5b/ui/aura/mus/window_tree_client_unittest.cc [modify] https://crrev.com/28625ccf0b3bcbcf118f06024f78d975f7575d5b/ui/aura/test/aura_test_base.cc [modify] https://crrev.com/28625ccf0b3bcbcf118f06024f78d975f7575d5b/ui/aura/window.h [modify] https://crrev.com/28625ccf0b3bcbcf118f06024f78d975f7575d5b/ui/views/mus/mus_client.cc
,
Nov 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4f5e732f00545db86e98aaff3245a2b3611d9168 commit 4f5e732f00545db86e98aaff3245a2b3611d9168 Author: msw <msw@chromium.org> Date: Thu Nov 17 07:00:36 2016 Expand aura::PropertyConverter support. Add PropertyConverter support for strings and rects. Allow clients to register additional properties. Add a few more properties and lots of testing. BUG= 663522 TEST=Automated R=sky@chromium.org Review-Url: https://codereview.chromium.org/2499933003 Cr-Commit-Position: refs/heads/master@{#432792} [modify] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/BUILD.gn [modify] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/client/aura_constants.cc [modify] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/mus/property_converter.cc [modify] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/mus/property_converter.h [add] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/mus/property_converter_unittest.cc [modify] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/mus/window_tree_client_unittest.cc [modify] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/test/aura_test_base.cc [modify] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/test/aura_test_base.h [modify] https://crrev.com/4f5e732f00545db86e98aaff3245a2b3611d9168/ui/aura/window_property.h
,
Nov 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f84ab8b028d958dfd73d17cddb320dd319492910 commit f84ab8b028d958dfd73d17cddb320dd319492910 Author: msw <msw@chromium.org> Date: Wed Nov 23 00:47:16 2016 Add gfx::ImageSkia and icon support to aura::PropertyConverter. Split up mus app and window icon properties (like aura). Remove unused ash::mus::GetWindowAppIcon; add/update tests. BUG= 663522 TEST=Automated. R=sky@chromium.org TBR=tsepez@chromium.org Review-Url: https://codereview.chromium.org/2519583002 Cr-Commit-Position: refs/heads/master@{#434039} [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/ash/mus/property_util.cc [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/ash/mus/property_util.h [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/ash/mus/window_manager.cc [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/services/ui/public/interfaces/window_manager.mojom [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/ui/aura/mus/property_converter.cc [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/ui/aura/mus/property_converter.h [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/ui/aura/mus/property_converter_unittest.cc [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/ui/views/mus/native_widget_mus.cc [modify] https://crrev.com/f84ab8b028d958dfd73d17cddb320dd319492910/ui/views/mus/native_widget_mus_unittest.cc
,
Dec 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6337d443bf35c959350e8cacdfdaca2ed8af39f6 commit 6337d443bf35c959350e8cacdfdaca2ed8af39f6 Author: msw <msw@chromium.org> Date: Thu Dec 08 19:23:25 2016 Remove unnecessary chrome window property util functions. aura::PropertyConverter makes these functions obsolete. BUG= 663522 TEST=No [title/shelf] changes for settings, task manager, app panel windows. R=sky@chromium.org Review-Url: https://codereview.chromium.org/2560973002 Cr-Commit-Position: refs/heads/master@{#437304} [modify] https://crrev.com/6337d443bf35c959350e8cacdfdaca2ed8af39f6/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/6337d443bf35c959350e8cacdfdaca2ed8af39f6/chrome/browser/ui/ash/launcher/settings_window_observer.cc [delete] https://crrev.com/4aff4df5f338661a2b9c5449ebfe0c33d4311d75/chrome/browser/ui/ash/property_util.cc [delete] https://crrev.com/4aff4df5f338661a2b9c5449ebfe0c33d4311d75/chrome/browser/ui/ash/property_util.h [modify] https://crrev.com/6337d443bf35c959350e8cacdfdaca2ed8af39f6/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc [modify] https://crrev.com/6337d443bf35c959350e8cacdfdaca2ed8af39f6/chrome/browser/ui/views/task_manager_view.cc
,
Dec 15 2016
Mike, can we call this done?
,
Dec 15 2016
Let me try to naively hook up window and app icons first. (services/ui/public/interfaces/window_manager.mojom cites this bug) Otherwise, it's generally fixed, and we have Issue 655874 and Issue 667939 for optimizing image passing.
,
Dec 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0c069a141a3acf78acc9fa357a558bf326ba4b73 commit 0c069a141a3acf78acc9fa357a558bf326ba4b73 Author: msw <msw@chromium.org> Date: Thu Dec 22 14:23:25 2016 Register app and window icon properties for aura mus conversion. This ports aura app and window icon properties over mus. BUG= 663522 TEST=chrome --mash task_viewer shelf item has an icon. R=sadrul@chromium.org TBR=tsepez@chromium.org Review-Url: https://codereview.chromium.org/2591333002 Cr-Commit-Position: refs/heads/master@{#440411} [modify] https://crrev.com/0c069a141a3acf78acc9fa357a558bf326ba4b73/services/ui/public/interfaces/window_manager.mojom [modify] https://crrev.com/0c069a141a3acf78acc9fa357a558bf326ba4b73/ui/aura/mus/property_converter.cc
,
Dec 22 2016
This is now generally fixed.
,
Mar 4 2017
,
Apr 17 2017
,
May 30 2017
,
Aug 1 2017
,
Oct 14 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by sky@chromium.org
, Nov 8 2016