extensions: Migrate other uses of webkitNotifications to Web Notifications API.

extensions: Migrate other uses of webkitNotifications to Web Notifications API.

This are the remaining uses of the deprecated webkitNotifications in
Chromium repo.

BUG=None
TEST=None
R=peter@chromium.org,kalman@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287199

[tfarina, 2014-08-01 01:59:22]

I don't claim it is all right. Please, review it carefully. As I don't know
JavaScript myself :(

Thank you for your patience,

[Peter Beverloo, 2014-08-01 08:46:06]

lgtm % comments

Thank you again! :-)

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html
(right):

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:330: if
(Notification.permission == "denied") {
It's possible for |permission| to be "default" here as well. It'd be better if
you'd check for:

if (Notification.permission != "granted")
  return;

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:339:
var n = new Notification("On " + server.name + changel, {
s/changel/channel/

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:340:
icon: "http://www.google.com/favicon.icon",
That file doesn't actually exist :-). Not your bad, but would you mind updating
this to:

https://www.google.com/favicon.ico

[not at google - send to devlin, 2014-08-01 16:52:29]

lgtm, thanks for doing this.

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/examples/api/notifications/background.js
(right):

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/api/notifications/background.js:28: if
("Notification" in window) {
i preferred it the old way, "if (window.Notification)"

though honestly what's the point in feature detecting at this stage. it's a
sample for notifications :)

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html
(right):

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:344:
n.onshow = function() {};
do you need to set this?

[Peter Beverloo, 2014-08-01 16:53:34]

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html
(right):

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:344:
n.onshow = function() {};
On 2014/08/01 16:52:29, kalman wrote:
> do you need to set this?

I figured that it's for demonstrative purposes given that the old version also
included |ondisplay|, and that this is an example.

It can safely be removed.

[not at google - send to devlin, 2014-08-01 16:59:51]

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html
(right):

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:344:
n.onshow = function() {};
> I figured that it's for demonstrative purposes

Good point. Leaving it in SGTM.

[tfarina, 2014-08-02 00:54:06]

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/examples/api/notifications/background.js
(right):

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/api/notifications/background.js:28: if
("Notification" in window) {
On 2014/08/01 16:52:28, kalman wrote:
> i preferred it the old way, "if (window.Notification)"
> 
> though honestly what's the point in feature detecting at this stage. it's a
> sample for notifications :)

Done.

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html
(right):

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:330: if
(Notification.permission == "denied") {
On 2014/08/01 08:46:06, Peter Beverloo wrote:
> It's possible for |permission| to be "default" here as well. It'd be better if
> you'd check for:
> 
> if (Notification.permission != "granted")
>   return;

Done.

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:339:
var n = new Notification("On " + server.name + changel, {
On 2014/08/01 08:46:06, Peter Beverloo wrote:
> s/changel/channel/

Done.

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:340:
icon: "http://www.google.com/favicon.icon",
On 2014/08/01 08:46:06, Peter Beverloo wrote:
> That file doesn't actually exist :-). Not your bad, but would you mind
updating
> this to:
> 
> https://www.google.com/favicon.ico

Done.

https://codereview.chromium.org/418173006/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/irc/servlet/index.html:344:
n.onshow = function() {};
On 2014/08/01 16:59:51, kalman wrote:
> > I figured that it's for demonstrative purposes
> 
> Good point. Leaving it in SGTM.

Done.

[tfarina, 2014-08-02 00:54:07]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-08-02 00:55:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/418173006/20001

[commit-bot: I haz the power, 2014-08-02 11:35:07]

Change committed as 287199