Remove the notificationProvider extension API

Remove the notificationProvider extension API

The NotificationProvider API was meant to provide a way for Chrome Apps
and extensions to take over handling of notifications from the Message
Center, for example to defer to the native notification center on a
system. However, the API was neither completed, nor shipped.

Today we're working on implementing support for native notification
centers directly to Chrome, not requiring any additional extension
support. This is already supported on Mac and is in development for
Windows. This makes an important use-case of the API redundant.

Let's remove the API. We can always revisit in the future.

BUG=397197, 482733
Review-Url: https://codereview.chromium.org/2659533003
Cr-Commit-Position: refs/heads/master@{#450333}
Committed: https://chromium.googlesource.com/chromium/src/+/f9fbe22a90c39cb811e94976923ebff0bc7f939f

[Peter Beverloo, 2017-01-26 15:39:03]

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

[commit-bot: I haz the power, 2017-01-26 15:39:14]

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

[Peter Beverloo, 2017-01-26 15:40:04]

peter@chromium.org changed reviewers:
+ dewittj@chromium.org, petewil@chromium.org

[Peter Beverloo, 2017-01-26 15:40:04]

Justin, Pete, I guess this is overdue?

[dewittj, 2017-01-26 16:59:44]

lgtm, thanks for doing some cleanup!

[commit-bot: I haz the power, 2017-01-26 17:04:34]

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

[commit-bot: I haz the power, 2017-01-26 17:04:36]

Dry run: This issue passed the CQ dry run.

[Peter Beverloo, 2017-01-26 17:10:04]

peter@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Peter Beverloo, 2017-01-26 17:10:04]

Thanks Justin!

+Devlin for extensions

[Peter Beverloo, 2017-01-26 18:46:26]

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

[Peter Beverloo, 2017-01-26 18:46:26]

Oh, and +isherman for extension_event_histogram_value.h

[Ilya Sherman, 2017-01-26 21:32:52]

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
File extensions/browser/extension_event_histogram_value.h (right):

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
extensions/browser/extension_event_histogram_value.h:428: // python
tools/metrics/histograms/update_extension_histograms.py
^^^

[Ilya Sherman, 2017-01-26 21:33:41]

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
File extensions/browser/extension_event_histogram_value.h (right):

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
extensions/browser/extension_event_histogram_value.h:213:
NOTIFICATION_PROVIDER_ON_UPDATED,  // unused
Sorry, to expand on my previous comment: Please change these names to be
prefixed with "DELETED_" as elsewhere in this file, then run the script
documented below to update histograms.xml.

[Peter Beverloo, 2017-01-27 15:11:22]

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
File extensions/browser/extension_event_histogram_value.h (right):

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
extensions/browser/extension_event_histogram_value.h:213:
NOTIFICATION_PROVIDER_ON_UPDATED,  // unused
On 2017/01/26 21:33:41, Ilya Sherman wrote:
> Sorry, to expand on my previous comment: Please change these names to be
> prefixed with "DELETED_" as elsewhere in this file, then run the script
> documented below to update histograms.xml.

Done.

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
extensions/browser/extension_event_histogram_value.h:428: // python
tools/metrics/histograms/update_extension_histograms.py
On 2017/01/26 21:32:52, Ilya Sherman wrote:
> ^^^

Done.

[Peter Beverloo, 2017-01-27 15:15:29]

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

[commit-bot: I haz the power, 2017-01-27 15:15:44]

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-27 16:36:29]

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

[commit-bot: I haz the power, 2017-01-27 16:36:31]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2017-01-27 22:56:10]

extension_event_histogram_value.h and histograms.xml lgtm

[Devlin, 2017-01-28 02:45:04]

https://codereview.chromium.org/2659533003/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/notifications/notifications_api.cc (right):

https://codereview.chromium.org/2659533003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/notifications/notifications_api.cc:128: bool
NotificationBitmapToGfxImage(
nit: add function level comment

https://codereview.chromium.org/2659533003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/notifications/notifications_api.cc:131: const
extensions::api::notifications::NotificationBitmap&
s/extensions::api::notifications/notifications::

https://codereview.chromium.org/2659533003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/notifications/notifications_api.cc:137: const int
BYTES_PER_PIXEL = 4;
I think this was just copy-pasted, but since we're here, let's make this
kBytesPerPixel

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
File extensions/browser/extension_event_histogram_value.h (right):

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
extensions/browser/extension_event_histogram_value.h:211:
NOTIFICATION_PROVIDER_ON_CLEARED,  // unused
instead of // unused, prefixed these with DELETED_ (as e.g. line 193 does)

[Peter Beverloo, 2017-02-09 16:03:10]

PTAL

https://codereview.chromium.org/2659533003/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/notifications/notifications_api.cc (right):

https://codereview.chromium.org/2659533003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/notifications/notifications_api.cc:128: bool
NotificationBitmapToGfxImage(
On 2017/01/28 02:45:03, Devlin (OOO feb 8 and feb 9) wrote:
> nit: add function level comment

Done.

https://codereview.chromium.org/2659533003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/notifications/notifications_api.cc:131: const
extensions::api::notifications::NotificationBitmap&
On 2017/01/28 02:45:03, Devlin (OOO feb 8 and feb 9) wrote:
> s/extensions::api::notifications/notifications::

Done.

https://codereview.chromium.org/2659533003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/notifications/notifications_api.cc:137: const int
BYTES_PER_PIXEL = 4;
On 2017/01/28 02:45:03, Devlin (OOO feb 8 and feb 9) wrote:
> I think this was just copy-pasted, but since we're here, let's make this
> kBytesPerPixel

Done.

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
File extensions/browser/extension_event_histogram_value.h (right):

https://codereview.chromium.org/2659533003/diff/1/extensions/browser/extensio...
extensions/browser/extension_event_histogram_value.h:211:
NOTIFICATION_PROVIDER_ON_CLEARED,  // unused
On 2017/01/28 02:45:03, Devlin (OOO feb 8 and feb 9) wrote:
> instead of // unused, prefixed these with DELETED_ (as e.g. line 193 does)

Acknowledged.

[Peter Beverloo, 2017-02-09 16:06:33]

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

[commit-bot: I haz the power, 2017-02-09 16:06:55]

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-02-09 17:16:53]

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

[commit-bot: I haz the power, 2017-02-09 17:16:54]

Dry run: This issue passed the CQ dry run.

[Peter Beverloo, 2017-02-13 14:16:27]

Devlin - friendly ping

[Devlin, 2017-02-13 22:43:03]

On 2017/02/13 14:16:27, Peter Beverloo wrote:
> Devlin - friendly ping

lgtm!  Thanks for the cleanup. :D

[Peter Beverloo, 2017-02-14 12:13:18]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2017-02-14 12:13:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
dewittj@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2659533003/#ps40001
(title: "Remove the notificationProvider extension API")

[commit-bot: I haz the power, 2017-02-14 12:13:42]

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-02-14 13:22:49]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487074398249500,
"parent_rev": "6fd678c91c89cfe647fbbbecf98bf19d8979052c", "commit_rev":
"f9fbe22a90c39cb811e94976923ebff0bc7f939f"}

[commit-bot: I haz the power, 2017-02-14 13:23:27]

Description was changed from

==========
Remove the notificationProvider extension API

The NotificationProvider API was meant to provide a way for Chrome Apps
and extensions to take over handling of notifications from the Message
Center, for example to defer to the native notification center on a
system. However, the API was neither completed, nor shipped.

Today we're working on implementing support for native notification
centers directly to Chrome, not requiring any additional extension
support. This is already supported on Mac and is in development for
Windows. This makes an important use-case of the API redundant.

Let's remove the API. We can always revisit in the future.

BUG=397197, 482733
==========

to

==========
Remove the notificationProvider extension API

The NotificationProvider API was meant to provide a way for Chrome Apps
and extensions to take over handling of notifications from the Message
Center, for example to defer to the native notification center on a
system. However, the API was neither completed, nor shipped.

Today we're working on implementing support for native notification
centers directly to Chrome, not requiring any additional extension
support. This is already supported on Mac and is in development for
Windows. This makes an important use-case of the API redundant.

Let's remove the API. We can always revisit in the future.

BUG=397197, 482733

Review-Url: https://codereview.chromium.org/2659533003
Cr-Commit-Position: refs/heads/master@{#450333}
Committed:
https://chromium.googlesource.com/chromium/src/+/f9fbe22a90c39cb811e94976923e...
==========

[commit-bot: I haz the power, 2017-02-14 13:23:29]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/f9fbe22a90c39cb811e94976923e...