Add the "silent" property to the NotificationOptions dictionary.

Add the "silent" property to the NotificationOptions dictionary.

The "silent" property allows authors to indicate that they do not want the
notification to make any noise, vibrations or lights when it's being
displayed on the user's device.

The value will be reflected as the "silent" attribute to the developer.
A layout test will be added in the third patch of this series. Further
testing is being added on the Chromium side.

The Web Notification specification:
  https://notifications.spec.whatwg.org/#object-members

Intent to Implement and Ship:
  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/TLRZrhZJJGo

This is part of a three-sided patch.
  [1] This patch.
  [2] https://codereview.chromium.org/965573002/
  [3] https://codereview.chromium.org/965463003/

BUG=442134
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191286

[Peter Beverloo, 2015-02-26 22:33:58]

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

[Peter Beverloo, 2015-02-26 22:33:59]

+mvanouwerkerk for review

[Michael van Ouwerkerk, 2015-02-27 11:31:16]

lgtm

https://codereview.chromium.org/960163003/diff/40001/Source/modules/notificat...
File Source/modules/notifications/Notification.h (right):

https://codereview.chromium.org/960163003/diff/40001/Source/modules/notificat...
Source/modules/notifications/Notification.h:122: void setSilent(bool silent) {
m_silent = silent; }
FIXME: If options's silent is true, and either options's sound is present or
options's vibrate is present, throw a TypeError exception.

Or maybe file a bug to that effect and link to it here.

[Peter Beverloo, 2015-02-27 12:32:37]

(Note that I won't submit this until the Intent to Implement and Shipped
passed.)

https://codereview.chromium.org/960163003/diff/40001/Source/modules/notificat...
File Source/modules/notifications/Notification.h (right):

https://codereview.chromium.org/960163003/diff/40001/Source/modules/notificat...
Source/modules/notifications/Notification.h:122: void setSilent(bool silent) {
m_silent = silent; }
On 2015/02/27 11:31:15, Michael van Ouwerkerk wrote:
> FIXME: If options's silent is true, and either options's sound is present or
> options's vibrate is present, throw a TypeError exception.
> 
> Or maybe file a bug to that effect and link to it here.

I don't think that's useful, because we don't implement either feature yet
("vibrate" and "sound").

When they do get implemented, obviously they should be implemented per the
specification, which includes these bits.

The tracking bugs are:
  https://crbug.com/442131 ("sound")
  https://crbug.com/442132 ("vibrate")

[Peter Beverloo, 2015-03-03 18:23:13]

Aaand LGTM3 is in. Putting this on the commit queue.

[Peter Beverloo, 2015-03-03 18:23:18]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2015-03-03 18:23:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/960163003/#ps60001 (title:
"add the attribute")

[commit-bot: I haz the power, 2015-03-03 18:24:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/960163003/60001

[commit-bot: I haz the power, 2015-03-04 04:21:52]

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

[commit-bot: I haz the power, 2015-03-04 04:21:53]

Failed to apply patch for Source/modules/notifications/Notification.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/modules/notifications/Notification.cpp
  Hunk #4 FAILED at 150.
  1 out of 4 hunks FAILED -- saving rejects to file
Source/modules/notifications/Notification.cpp.rej

Patch:       Source/modules/notifications/Notification.cpp
Index: Source/modules/notifications/Notification.cpp
diff --git a/Source/modules/notifications/Notification.cpp
b/Source/modules/notifications/Notification.cpp
index
00d9bde9ff702153837d666144b03de31d00e7c7..85282efdfa35cc8a1df8aebee98118077270850c
100644
--- a/Source/modules/notifications/Notification.cpp
+++ b/Source/modules/notifications/Notification.cpp
@@ -78,6 +78,7 @@ Notification* Notification::create(ExecutionContext* context,
const String& titl
     notification->setTag(options.tag());
     notification->setLang(options.lang());
     notification->setDir(options.dir());
+    notification->setSilent(options.silent());
     if (options.hasIcon()) {
         KURL iconUrl = options.icon().isEmpty() ? KURL() :
context->completeURL(options.icon());
         if (!iconUrl.isEmpty() && iconUrl.isValid())
@@ -103,6 +104,7 @@ Notification* Notification::create(ExecutionContext*
context, const String& pers
     notification->setLang(data.lang);
     notification->setBody(data.body);
     notification->setTag(data.tag);
+    notification->setSilent(data.silent);
 
     if (!data.icon.isEmpty())
         notification->setIconUrl(data.icon);
@@ -116,6 +118,7 @@ Notification::Notification(const String& title,
ExecutionContext* context)
     : ActiveDOMObject(context)
     , m_title(title)
     , m_dir("auto")
+    , m_silent(false)
     , m_state(NotificationStateIdle)
     , m_asyncRunner(this, &Notification::show)
 {
@@ -147,7 +150,7 @@ void Notification::show()
 
     // FIXME: Associate the appropriate text direction with the notification.
     // FIXME: Do CSP checks on the associated notification icon.
-    WebNotificationData notificationData(m_title,
WebNotificationData::DirectionLeftToRight, m_lang, m_body, m_tag, m_iconUrl);
+    WebNotificationData notificationData(m_title,
WebNotificationData::DirectionLeftToRight, m_lang, m_body, m_tag, m_iconUrl,
m_silent);
     notificationManager()->show(WebSerializedOrigin(*origin), notificationData,
this);
 
     m_state = NotificationStateShowing;

[commit-bot: I haz the power, 2015-03-04 04:22:28]

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

[Peter Beverloo, 2015-03-04 13:46:54]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2015-03-04 13:46:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/960163003/#ps80001 (title:
"rebase")

[commit-bot: I haz the power, 2015-03-04 13:47:02]

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

[commit-bot: I haz the power, 2015-03-04 14:55:37]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191286