Add badge to web notifications.

Add badge to web notifications.

* This is guarded by the NotificationBadge runtime flag.
* On Android M+ the badge is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the badge is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP
* Spec discussion: https://github.com/whatwg/notifications/issues/65
* Spec PR: https://github.com/mvano/notifications/commit/d79fe317ccc00dc7e27dde88928dc33bcf38e5b4
* Intent to implement and ship: https://goo.gl/4KwzyR

BUG=591394
Committed: https://crrev.com/2dc0200c983a414b1ec462b5fe6fee0076e67231
Cr-Commit-Position: refs/heads/master@{#381927}

[Michael van Ouwerkerk, 2016-03-02 16:13:09]

Description was changed from

==========
Add experimental small icon to web notifications.

* This is guarded by the NotificationExperimental runtime flag.
* On Android M+ the small icon is displayed in the status bar.
* On all Android versions the small icon is displayed in the notification.
* Not supported on desktop yet (it conflicts with the settings button).

BUG=591394
==========

to

==========
Add experimental small icon to web notifications.

* This is guarded by the NotificationExperimental runtime flag.
* On Android M+ the small icon is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the small icon is displayed in the notification.
* Not supported on desktop yet (it conflicts with the settings button).

BUG=591394
==========

[Michael van Ouwerkerk, 2016-03-02 16:13:47]

mvanouwerkerk@chromium.org changed reviewers:
+ peter@chromium.org

[Michael van Ouwerkerk, 2016-03-02 16:13:48]

Peter, could you take a look please?

[Michael van Ouwerkerk, 2016-03-02 16:14:44]

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

[commit-bot: I haz the power, 2016-03-02 16:14:57]

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

[Michael van Ouwerkerk, 2016-03-02 16:46:14]

Description was changed from

==========
Add experimental small icon to web notifications.

* This is guarded by the NotificationExperimental runtime flag.
* On Android M+ the small icon is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the small icon is displayed in the notification.
* Not supported on desktop yet (it conflicts with the settings button).

BUG=591394
==========

to

==========
Add experimental small icon to web notifications.

* This is guarded by the NotificationExperimental runtime flag.
* On Android M+ the small icon is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the small icon is displayed in the notification.
* Not supported on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP

BUG=591394
==========

[commit-bot: I haz the power, 2016-03-02 17:26:20]

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

[commit-bot: I haz the power, 2016-03-02 17:26:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peter Beverloo, 2016-03-03 17:55:39]

Code lgtm % comments, but–

  - Please be sure to send out an Intent to Implement (and Ship?) prior to
landing.
  - Update the webexposed tests to include `Notification.smallIcon`.

https://codereview.chromium.org/1750083004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
(right):

https://codereview.chromium.org/1750083004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:440:
*                  to display the whole notification. It may also be displayed
inside the
Let's explicitly call out the status bar?

https://codereview.chromium.org/1750083004/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java
(right):

https://codereview.chromium.org/1750083004/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java:173:
assertEquals(Color.WHITE, smallIconBitmap.getPixel(0, 0));
For M+, could we verify that the Icon returned by notification.getSmallIcon()
also (a) has an image, and (b) has been tinted?

https://codereview.chromium.org/1750083004/diff/1/chrome/browser/notification...
File chrome/browser/notifications/platform_notification_service_impl.cc (right):

https://codereview.chromium.org/1750083004/diff/1/chrome/browser/notification...
chrome/browser/notifications/platform_notification_service_impl.cc:492:
gfx::Image::CreateFrom1xBitmap(notification_resources.small_icon));
What does this actually do on desktop? Do we try to render it, or give the cog
precedence?

https://codereview.chromium.org/1750083004/diff/1/content/child/notifications...
File content/child/notifications/pending_notifications_tracker_unittest.cc
(right):

https://codereview.chromium.org/1750083004/diff/1/content/child/notifications...
content/child/notifications/pending_notifications_tracker_unittest.cc:168:
ASSERT_FALSE(GetResources(0u)->small_icon.drawsNothing());
nit: would it make sense to have the following in a few places?

NotificationResources* resources = GetResources(0u);

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/notifications/NotificationData.cpp
(right):

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/notifications/NotificationData.cpp:30: KURL
completeURL(ExecutionContext* executionContext, const String& url)
nit: Perhaps be explicit and make this return a WebURL, since we only assign it
to that?

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/notifications/NotificationOptions.idl
(right):

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/notifications/NotificationOptions.idl:19:
[RuntimeEnabled=NotificationExperimental] USVString smallIcon = "";
Why does this have a default value, but `icon` doesn't? Both are optional.

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:135:
NotificationExperimental status=experimental
I've got mixed feelings about this. On one hand I'm OK with it because it's not
super important, on the other hand this would have the side-effect of exposing
Notification.`vibrate` and `actions` with the incorrect IDL types.

We decided a while ago to not overload this flag further because it caused an
issue. Let's either give it its own flag, or have an I2I&S so that we can avoid
it altogether?

[Michael van Ouwerkerk, 2016-03-09 18:27:52]

Description was changed from

==========
Add experimental small icon to web notifications.

* This is guarded by the NotificationExperimental runtime flag.
* On Android M+ the small icon is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the small icon is displayed in the notification.
* Not supported on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP

BUG=591394
==========

to

==========
Add experimental small icon to web notifications.

* This is guarded by the NotificationSmallIcons runtime flag.
* On Android M+ the small icon is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the small icon is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP

BUG=591394
==========

[Michael van Ouwerkerk, 2016-03-09 18:28:25]

Thanks Peter.

https://codereview.chromium.org/1750083004/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
(right):

https://codereview.chromium.org/1750083004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java:440:
*                  to display the whole notification. It may also be displayed
inside the
On 2016/03/03 17:55:38, Peter Beverloo wrote:
> Let's explicitly call out the status bar?

Right, we're in Android code here.

https://codereview.chromium.org/1750083004/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java
(right):

https://codereview.chromium.org/1750083004/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilderTest.java:173:
assertEquals(Color.WHITE, smallIconBitmap.getPixel(0, 0));
On 2016/03/03 17:55:38, Peter Beverloo wrote:
> For M+, could we verify that the Icon returned by notification.getSmallIcon()
> also (a) has an image, and (b) has been tinted?

Done.

https://codereview.chromium.org/1750083004/diff/1/chrome/browser/notification...
File chrome/browser/notifications/platform_notification_service_impl.cc (right):

https://codereview.chromium.org/1750083004/diff/1/chrome/browser/notification...
chrome/browser/notifications/platform_notification_service_impl.cc:492:
gfx::Image::CreateFrom1xBitmap(notification_resources.small_icon));
On 2016/03/03 17:55:38, Peter Beverloo wrote:
> What does this actually do on desktop? Do we try to render it, or give the cog
> precedence?

If there are no buttons, they render on top of each other. Otherwise, the small
icon goes in the bottom right corner which is part of a button. We should
probably badge the main notification icon instead.

https://codereview.chromium.org/1750083004/diff/1/content/child/notifications...
File content/child/notifications/pending_notifications_tracker_unittest.cc
(right):

https://codereview.chromium.org/1750083004/diff/1/content/child/notifications...
content/child/notifications/pending_notifications_tracker_unittest.cc:168:
ASSERT_FALSE(GetResources(0u)->small_icon.drawsNothing());
On 2016/03/03 17:55:39, Peter Beverloo wrote:
> nit: would it make sense to have the following in a few places?
> 
> NotificationResources* resources = GetResources(0u);

Done.

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/notifications/NotificationData.cpp
(right):

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/notifications/NotificationData.cpp:30: KURL
completeURL(ExecutionContext* executionContext, const String& url)
On 2016/03/03 17:55:39, Peter Beverloo wrote:
> nit: Perhaps be explicit and make this return a WebURL, since we only assign
it
> to that?

Done.

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/notifications/NotificationOptions.idl
(right):

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/notifications/NotificationOptions.idl:19:
[RuntimeEnabled=NotificationExperimental] USVString smallIcon = "";
On 2016/03/03 17:55:39, Peter Beverloo wrote:
> Why does this have a default value, but `icon` doesn't? Both are optional.

Done.

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/1750083004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:135:
NotificationExperimental status=experimental
On 2016/03/03 17:55:39, Peter Beverloo wrote:
> I've got mixed feelings about this. On one hand I'm OK with it because it's
not
> super important, on the other hand this would have the side-effect of exposing
> Notification.`vibrate` and `actions` with the incorrect IDL types.
> 
> We decided a while ago to not overload this flag further because it caused an
> issue. Let's either give it its own flag, or have an I2I&S so that we can
avoid
> it altogether?

Done.

[Michael van Ouwerkerk, 2016-03-09 18:28:38]

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

[commit-bot: I haz the power, 2016-03-09 18:28:52]

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

[commit-bot: I haz the power, 2016-03-09 19:59:40]

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

[commit-bot: I haz the power, 2016-03-09 19:59:41]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Michael van Ouwerkerk, 2016-03-10 11:12:13]

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

[commit-bot: I haz the power, 2016-03-10 11:12:29]

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

[Michael van Ouwerkerk, 2016-03-10 11:14:54]

mvanouwerkerk@chromium.org changed reviewers:
+ jochen@chromium.org

[Michael van Ouwerkerk, 2016-03-10 11:14:54]

Jochen, could you take a look please as owner of:
RuntimeEnabledFeatures.in
content/

[Michael van Ouwerkerk, 2016-03-10 11:15:25]

mvanouwerkerk@chromium.org changed reviewers:
+ newt@chromium.org

[Michael van Ouwerkerk, 2016-03-10 11:15:26]

Newton, could you take a look please as owner of:
web_notification_small_icon.xml

[Michael van Ouwerkerk, 2016-03-10 11:17:02]

mvanouwerkerk@chromium.org changed reviewers:
+ mkwst@chromium.org

[Michael van Ouwerkerk, 2016-03-10 11:17:03]

Mike, could you take a look please as owner of platform_notification_messages.h?

[commit-bot: I haz the power, 2016-03-10 13:28:47]

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

[commit-bot: I haz the power, 2016-03-10 13:28:48]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jochen (gone - plz use gerrit), 2016-03-10 15:50:50]

lgtm

[Mike West, 2016-03-11 07:20:54]

Adding the icon to IPC LGTM.

[newt (away), 2016-03-14 23:12:34]

web_notification_small_icon.xml lgtm. Sorry for the delay -- I was away.

[Michael van Ouwerkerk, 2016-03-17 12:29:54]

Description was changed from

==========
Add experimental small icon to web notifications.

* This is guarded by the NotificationSmallIcons runtime flag.
* On Android M+ the small icon is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the small icon is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP

BUG=591394
==========

to

==========
Add badge to web notifications.

* This is guarded by the NotificationBadge runtime flag.
* On Android M+ the badge is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the badge is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP
* Spec discussion: https://github.com/whatwg/notifications/issues/65
* Spec PR:
https://github.com/mvano/notifications/commit/d79fe317ccc00dc7e27dde88928dc33...

BUG=591394
==========

[Michael van Ouwerkerk, 2016-03-17 12:32:07]

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

[commit-bot: I haz the power, 2016-03-17 12:32:19]

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

[commit-bot: I haz the power, 2016-03-17 14:15:44]

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

[commit-bot: I haz the power, 2016-03-17 14:15:45]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Michael van Ouwerkerk, 2016-03-17 17:59:29]

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

[commit-bot: I haz the power, 2016-03-17 17:59:59]

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

[commit-bot: I haz the power, 2016-03-17 19:25:02]

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

[commit-bot: I haz the power, 2016-03-17 19:25:04]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2016-03-18 10:15:06]

Description was changed from

==========
Add badge to web notifications.

* This is guarded by the NotificationBadge runtime flag.
* On Android M+ the badge is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the badge is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP
* Spec discussion: https://github.com/whatwg/notifications/issues/65
* Spec PR:
https://github.com/mvano/notifications/commit/d79fe317ccc00dc7e27dde88928dc33...

BUG=591394
==========

to

==========
Add badge to web notifications.

* This is guarded by the NotificationBadge runtime flag.
* On Android M+ the badge is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the badge is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP
* Spec discussion: https://github.com/whatwg/notifications/issues/65
* Spec PR:
https://github.com/mvano/notifications/commit/d79fe317ccc00dc7e27dde88928dc33...
* Intent to implement and ship: https://goo.gl/4KwzyR

BUG=591394
==========

[Michael van Ouwerkerk, 2016-03-18 10:15:23]

The CQ bit was checked by mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2016-03-18 10:15:24]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
mkwst@chromium.org, jochen@chromium.org, newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1750083004/#ps80001
(title: "Fix test.")

[commit-bot: I haz the power, 2016-03-18 10:15:42]

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

[commit-bot: I haz the power, 2016-03-18 10:21:53]

Description was changed from

==========
Add badge to web notifications.

* This is guarded by the NotificationBadge runtime flag.
* On Android M+ the badge is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the badge is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP
* Spec discussion: https://github.com/whatwg/notifications/issues/65
* Spec PR:
https://github.com/mvano/notifications/commit/d79fe317ccc00dc7e27dde88928dc33...
* Intent to implement and ship: https://goo.gl/4KwzyR

BUG=591394
==========

to

==========
Add badge to web notifications.

* This is guarded by the NotificationBadge runtime flag.
* On Android M+ the badge is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the badge is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP
* Spec discussion: https://github.com/whatwg/notifications/issues/65
* Spec PR:
https://github.com/mvano/notifications/commit/d79fe317ccc00dc7e27dde88928dc33...
* Intent to implement and ship: https://goo.gl/4KwzyR

BUG=591394
==========

[commit-bot: I haz the power, 2016-03-18 10:21:55]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-18 10:22:53]

Description was changed from

==========
Add badge to web notifications.

* This is guarded by the NotificationBadge runtime flag.
* On Android M+ the badge is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the badge is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP
* Spec discussion: https://github.com/whatwg/notifications/issues/65
* Spec PR:
https://github.com/mvano/notifications/commit/d79fe317ccc00dc7e27dde88928dc33...
* Intent to implement and ship: https://goo.gl/4KwzyR

BUG=591394
==========

to

==========
Add badge to web notifications.

* This is guarded by the NotificationBadge runtime flag.
* On Android M+ the badge is displayed in the status bar,
  otherwise Chrome falls back to the default Chrome icon.
* On all Android versions the badge is displayed in the notification.
* Not quite correct on desktop yet (it conflicts with the settings button).
* Screenshots: https://imgur.com/a/wewTP
* Spec discussion: https://github.com/whatwg/notifications/issues/65
* Spec PR:
https://github.com/mvano/notifications/commit/d79fe317ccc00dc7e27dde88928dc33...
* Intent to implement and ship: https://goo.gl/4KwzyR

BUG=591394

Committed: https://crrev.com/2dc0200c983a414b1ec462b5fe6fee0076e67231
Cr-Commit-Position: refs/heads/master@{#381927}
==========

[commit-bot: I haz the power, 2016-03-18 10:22:54]

Patchset 5 (id:??) landed as
https://crrev.com/2dc0200c983a414b1ec462b5fe6fee0076e67231
Cr-Commit-Position: refs/heads/master@{#381927}