[Media>UI] Set large icon for lockscreen and wearable if it is specified by the page

[Media>UI] Set large icon for lockscreen and wearable if it is specified by the page

With MediaSession API, the page can specify custom media artwork. The
quality of these images are usually good enough to be used as lockscreen
and wearable background. This CL enables this usage.

BUG=678207
Review-Url: https://codereview.chromium.org/2626213004
Cr-Commit-Position: refs/heads/master@{#443800}
Committed: https://chromium.googlesource.com/chromium/src/+/5f9b3d7676beeb194091dae650ada4ee1a59f13e

[Zhiqiang Zhang (Slow), 2017-01-13 10:54:33]

zqzhang@chromium.org changed reviewers:
+ avayvod@chromium.org

[Zhiqiang Zhang (Slow), 2017-01-13 10:54:33]

PTAL

The change is a bit ugly because we have to add new fields to
MediaNotificationInfo to let MediaNotificationManager know if the largeIcon is
an artwork image from the page. Do you have better ideas how to implement this?

[Zhiqiang Zhang (Slow), 2017-01-13 16:14:58]

Description was changed from

==========
[Media>UI] Set large icon for lockscreen and wearable if it is specified by the
page

With MediaSession API, the page can specify custom media artwork. The
quality of these images are usually good enough to be used as lockscreen
and wearable background. This CL enables this usage.

BUG=678207
==========

to

==========
[Media>UI] Set large icon for lockscreen and wearable if it is specified by the
page

With MediaSession API, the page can specify custom media artwork. The
quality of these images are usually good enough to be used as lockscreen
and wearable background. This CL enables this usage.

BUG=678207
==========

[whywhat, 2017-01-13 17:17:09]

Few thoughts:
- can MNM detect if the large icon is good enough to be shown on wearables and
lock screen without the flag and just do it independent from whether it's set by
the API or acquired in another way
- failing that, the minimum feedback I have is to split the flag in a separate
independent property and method, this way the true/false values are more
readable when set; you could use an enum instead of a bool if you want to them
always be set together;
- you could also try to combine all icon related fields (icon, largeIcon,
defaultIcon, isLargeIconCustom) into a separate object hanging off of
notification info similar to MediaMetadata;
- maybe the flag could be renamed into something like useAsAlbumArt or something
else about its purpose, not about when it's set so it makes more sense for other
notifications;
- what about tests?

[Zhiqiang Zhang (Slow), 2017-01-13 17:37:42]

On 2017/01/13 17:17:09, whywhat wrote:
> Few thoughts:
> - can MNM detect if the large icon is good enough to be shown on wearables and
> lock screen without the flag and just do it independent from whether it's set
by
> the API or acquired in another way
> - failing that, the minimum feedback I have is to split the flag in a separate
> independent property and method, this way the true/false values are more
> readable when set; you could use an enum instead of a bool if you want to them
> always be set together;
> - you could also try to combine all icon related fields (icon, largeIcon,
> defaultIcon, isLargeIconCustom) into a separate object hanging off of
> notification info similar to MediaMetadata;
> - maybe the flag could be renamed into something like useAsAlbumArt or
something
> else about its purpose, not about when it's set so it makes more sense for
other
> notifications;
> - what about tests?

Thanks for the comments. PTAL

We talked with fbeaufort@ before lunch and agreed it would be good if we could
have different images for the largeIcon and lockscreen, especially in
consideration of resolution.
However we cannot prevent websites to crazy stuffs providing totally different
images for different resolution.

So I prefer to split the largeIcon property into notificationLargeIcon and
mediaSessionImage, which will be passed to the notification and
AndroidMediaSession respectively.

For tests, it's really hard to do it since MNM interacts with the support
library.

[whywhat, 2017-01-13 19:43:38]

lgtm

[Zhiqiang Zhang (Slow), 2017-01-14 16:04:23]

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

[commit-bot: I haz the power, 2017-01-14 16:04:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-14 16:37:31]

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

[commit-bot: I haz the power, 2017-01-14 16:37:32]

Dry run: This issue passed the CQ dry run.

[Zhiqiang Zhang (Slow), 2017-01-14 17:08:29]

The CQ bit was checked by zqzhang@chromium.org

[commit-bot: I haz the power, 2017-01-14 17:08:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-14 17:12:02]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484413709039610,
"parent_rev": "daabfe6a4b48609f1e5e417481924ef5b440d664", "commit_rev":
"5f9b3d7676beeb194091dae650ada4ee1a59f13e"}

[commit-bot: I haz the power, 2017-01-14 17:12:38]

Description was changed from

==========
[Media>UI] Set large icon for lockscreen and wearable if it is specified by the
page

With MediaSession API, the page can specify custom media artwork. The
quality of these images are usually good enough to be used as lockscreen
and wearable background. This CL enables this usage.

BUG=678207
==========

to

==========
[Media>UI] Set large icon for lockscreen and wearable if it is specified by the
page

With MediaSession API, the page can specify custom media artwork. The
quality of these images are usually good enough to be used as lockscreen
and wearable background. This CL enables this usage.

BUG=678207

Review-Url: https://codereview.chromium.org/2626213004
Cr-Commit-Position: refs/heads/master@{#443800}
Committed:
https://chromium.googlesource.com/chromium/src/+/5f9b3d7676beeb194091dae650ad...
==========

[commit-bot: I haz the power, 2017-01-14 17:12:39]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5f9b3d7676beeb194091dae650ad...