Issue 1824183002: Mash: Show app icons in shelf based on the Widget's app icon

James Cook

Description was changed from ========== WIP: Show different icons in shelf based on view::Widget WindowAppIcon ...

4 years, 9 months ago (2016-03-23 22:08:55 UTC) #1

James Cook

jamescook@chromium.org changed reviewers: + sky@chromium.org

4 years, 9 months ago (2016-03-23 22:09:36 UTC) #2

James Cook

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

4 years, 9 months ago (2016-03-23 23:03:45 UTC) #4

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1824183002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1824183002/100001

4 years, 9 months ago (2016-03-23 23:04:09 UTC) #5

sky

Test coverage would be nice too. https://codereview.chromium.org/1824183002/diff/100001/components/mus/public/cpp/lib/property_type_converters.cc File components/mus/public/cpp/lib/property_type_converters.cc (right): https://codereview.chromium.org/1824183002/diff/100001/components/mus/public/cpp/lib/property_type_converters.cc#newcode154 components/mus/public/cpp/lib/property_type_converters.cc:154: std::vector<uint8_t> vec(8 + ...

4 years, 9 months ago (2016-03-23 23:30:40 UTC) #6

commit-bot: I haz the power

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

4 years, 9 months ago (2016-03-23 23:46:37 UTC) #7

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 9 months ago (2016-03-23 23:46:38 UTC) #8

msw

Nice patch! https://codereview.chromium.org/1824183002/diff/100001/ash/mus/shelf_delegate_mus.cc File ash/mus/shelf_delegate_mus.cc (right): https://codereview.chromium.org/1824183002/diff/100001/ash/mus/shelf_delegate_mus.cc#newcode229 ash/mus/shelf_delegate_mus.cc:229: if (index == -1) optional nit: DCHECK ...

4 years, 9 months ago (2016-03-24 17:40:55 UTC) #10

James Cook

Description was changed from ========== Mash: Show app icons in shelf based on the Widget's ...

4 years, 9 months ago (2016-03-24 19:21:27 UTC) #11

Description was changed from

==========
Mash: Show app icons in shelf based on the Widget's app icon

* Use the large icon from WidgetDelegate::GetWindowAppIcon().
* In the mojo app's NativeWidgetMus, serialize the icon's SkBitmap as a vector
of bytes and set it as a shared window property.
* In the window manager, pass the serialized icon to any UserWindowObservers.
* In the system UI, deserialize the icon and use it on the shelf.
* Set an icon in task_viewer as a demonstration.

The window property serialization is handled by a custom TypeConverter for
vector<uint8>, SkBitmap. In the future it would be nice to use the serialized
form of the Skia Mojom struct skia.Bitmap, but that will require Mojo to
generate a public API to read the serialized bytes.

A screenshot of the shelf is attached to the bug.

BUG=595850
TEST=launch mash, run mojo:task_viewer, note QuickLaunch shelf icon is default
but TaskViewer icon is a gear
==========

to

==========
Mash: Show app icons in shelf based on the Widget's app icon

* Use the large icon from WidgetDelegate::GetWindowAppIcon().
* In the mojo app's NativeWidgetMus, serialize the icon's SkBitmap as a vector
of bytes and set it as a shared window property.
* In the window manager, pass the serialized icon to any UserWindowObservers.
* In the system UI, deserialize the icon and use it on the shelf.
* Set an icon in task_viewer as a demonstration.

The window property serialization is handled by a custom TypeConverter for
vector<uint8>, SkBitmap. In the future it would be nice to use the serialized
form of the Skia Mojom struct skia.Bitmap, but that will require Mojo to
generate a public API to read the serialized bytes.

Also fixed an issue where the app's primary widget and the window manager's
non-client-frame widget would fight over window titles.

A screenshot of the shelf is attached to the bug.

BUG=595850
TEST=launch mash, run mojo:task_viewer, note QuickLaunch shelf icon is default
but TaskViewer icon is a gear
==========

James Cook

jamescook@chromium.org changed reviewers: - msw@chromium.org

4 years, 9 months ago (2016-03-25 15:39:26 UTC) #12

James Cook

Scott, please take another look. https://codereview.chromium.org/1824183002/diff/100001/ash/mus/shelf_delegate_mus.cc File ash/mus/shelf_delegate_mus.cc (right): https://codereview.chromium.org/1824183002/diff/100001/ash/mus/shelf_delegate_mus.cc#newcode229 ash/mus/shelf_delegate_mus.cc:229: if (index == -1) ...

4 years, 9 months ago (2016-03-25 15:39:29 UTC) #13

sky

https://codereview.chromium.org/1824183002/diff/160001/components/mus/public/cpp/lib/property_type_converters.cc File components/mus/public/cpp/lib/property_type_converters.cc (right): https://codereview.chromium.org/1824183002/diff/160001/components/mus/public/cpp/lib/property_type_converters.cc#newcode197 components/mus/public/cpp/lib/property_type_converters.cc:197: memcpy(bitmap.getPixels(), &input[4], bitmap.getSize()); Do you need to lock and ...

4 years, 9 months ago (2016-03-25 15:56:35 UTC) #14

James Cook

Scott, please take another look. (And thanks for the quick review!) https://codereview.chromium.org/1824183002/diff/160001/components/mus/public/cpp/lib/property_type_converters.cc File components/mus/public/cpp/lib/property_type_converters.cc (right): ...

4 years, 9 months ago (2016-03-25 16:52:39 UTC) #15

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1824183002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1824183002/180001

4 years, 9 months ago (2016-03-25 17:36:02 UTC) #18

commit-bot: I haz the power

Description was changed from ========== Mash: Show app icons in shelf based on the Widget's ...

4 years, 9 months ago (2016-03-25 18:12:02 UTC) #19

Message was sent while issue was closed.

Description was changed from

==========
Mash: Show app icons in shelf based on the Widget's app icon

* Use the large icon from WidgetDelegate::GetWindowAppIcon().
* In the mojo app's NativeWidgetMus, serialize the icon's SkBitmap as a vector
of bytes and set it as a shared window property.
* In the window manager, pass the serialized icon to any UserWindowObservers.
* In the system UI, deserialize the icon and use it on the shelf.
* Set an icon in task_viewer as a demonstration.

The window property serialization is handled by a custom TypeConverter for
vector<uint8>, SkBitmap. In the future it would be nice to use the serialized
form of the Skia Mojom struct skia.Bitmap, but that will require Mojo to
generate a public API to read the serialized bytes.

Also fixed an issue where the app's primary widget and the window manager's
non-client-frame widget would fight over window titles.

A screenshot of the shelf is attached to the bug.

BUG=595850
TEST=launch mash, run mojo:task_viewer, note QuickLaunch shelf icon is default
but TaskViewer icon is a gear
==========

to

==========
Mash: Show app icons in shelf based on the Widget's app icon

* Use the large icon from WidgetDelegate::GetWindowAppIcon().
* In the mojo app's NativeWidgetMus, serialize the icon's SkBitmap as a vector
of bytes and set it as a shared window property.
* In the window manager, pass the serialized icon to any UserWindowObservers.
* In the system UI, deserialize the icon and use it on the shelf.
* Set an icon in task_viewer as a demonstration.

The window property serialization is handled by a custom TypeConverter for
vector<uint8>, SkBitmap. In the future it would be nice to use the serialized
form of the Skia Mojom struct skia.Bitmap, but that will require Mojo to
generate a public API to read the serialized bytes.

Also fixed an issue where the app's primary widget and the window manager's
non-client-frame widget would fight over window titles.

A screenshot of the shelf is attached to the bug.

BUG=595850
TEST=launch mash, run mojo:task_viewer, note QuickLaunch shelf icon is default
but TaskViewer icon is a gear
==========

commit-bot: I haz the power

Description was changed from ========== Mash: Show app icons in shelf based on the Widget's ...

4 years, 9 months ago (2016-03-25 18:13:34 UTC) #21

Message was sent while issue was closed.

Description was changed from

==========
Mash: Show app icons in shelf based on the Widget's app icon

* Use the large icon from WidgetDelegate::GetWindowAppIcon().
* In the mojo app's NativeWidgetMus, serialize the icon's SkBitmap as a vector
of bytes and set it as a shared window property.
* In the window manager, pass the serialized icon to any UserWindowObservers.
* In the system UI, deserialize the icon and use it on the shelf.
* Set an icon in task_viewer as a demonstration.

The window property serialization is handled by a custom TypeConverter for
vector<uint8>, SkBitmap. In the future it would be nice to use the serialized
form of the Skia Mojom struct skia.Bitmap, but that will require Mojo to
generate a public API to read the serialized bytes.

Also fixed an issue where the app's primary widget and the window manager's
non-client-frame widget would fight over window titles.

A screenshot of the shelf is attached to the bug.

BUG=595850
TEST=launch mash, run mojo:task_viewer, note QuickLaunch shelf icon is default
but TaskViewer icon is a gear
==========

to

==========
Mash: Show app icons in shelf based on the Widget's app icon

* Use the large icon from WidgetDelegate::GetWindowAppIcon().
* In the mojo app's NativeWidgetMus, serialize the icon's SkBitmap as a vector
of bytes and set it as a shared window property.
* In the window manager, pass the serialized icon to any UserWindowObservers.
* In the system UI, deserialize the icon and use it on the shelf.
* Set an icon in task_viewer as a demonstration.

The window property serialization is handled by a custom TypeConverter for
vector<uint8>, SkBitmap. In the future it would be nice to use the serialized
form of the Skia Mojom struct skia.Bitmap, but that will require Mojo to
generate a public API to read the serialized bytes.

Also fixed an issue where the app's primary widget and the window manager's
non-client-frame widget would fight over window titles.

A screenshot of the shelf is attached to the bug.

BUG=595850
TEST=launch mash, run mojo:task_viewer, note QuickLaunch shelf icon is default
but TaskViewer icon is a gear

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

commit-bot: I haz the power

4 years, 9 months ago (2016-03-25 18:13:35 UTC) #22

Message was sent while issue was closed.

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

Issue 1824183002: Mash: Show app icons in shelf based on the Widget's app icon (Closed)

Description

Patch Set 1 #

Patch Set 2 : brute force array copies #

Patch Set 3 : it works #

Patch Set 4 : cleanup #

Patch Set 5 : fix native_widget_mus dep #

Patch Set 6 : //ui/resources #

Patch Set 7 : review comments #

Patch Set 8 : rebase #

Patch Set 9 : added unit tests #

Patch Set 10 : more review comments #

Messages