Implement support for notification.vibrate

chrome/browser/notifications/platform_notification_service_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/notifications/desktop_notification_profile_util.h"
@@ skipping 31 matching lines @@
 #include "base/strings/string_number_conversions.h"
 #endif
 
 using content::BrowserContext;
 using content::BrowserThread;
 using content::PlatformNotificationContext;
 using message_center::NotifierId;
 
 namespace {
 
+const int kMinimumVibrationDurationMs = 1;  // 1 millisecond
+const int kMaximumVibrationDurationMs = 10000;  // 10 seconds
+
 // Callback to provide when deleting the data associated with persistent Web
 // Notifications from the notification database.
 void OnPersistentNotificationDataDeleted(bool success) {
   // TODO(peter): Record UMA for notification deletion requests created by the
   // PlatformNotificationService.
 }
 
 // Persistent notifications fired through the delegate do not care about the
 // lifetime of the Service Worker responsible for executing the event.
 void OnEventDispatchComplete(content::PersistentNotificationStatus status) {
@@ skipping 254 matching lines @@
 
   // TODO(peter): Icons for Web Notifications are currently always requested for
   // 1x scale, whereas the displays on which they can be displayed can have a
   // different pixel density. Be smarter about this when the API gets updated
   // with a way for developers to specify images of different resolutions.
   Notification notification(origin, notification_data.title,
       notification_data.body, gfx::Image::CreateFrom1xBitmap(icon),
       display_source, notification_data.tag, delegate);
 
   notification.set_context_message(display_source);
+
+  // Though the Blink implementation already sanitizes vibration times, don't

[Peter Beverloo, 2015/05/05 20:42:44]

nit: Theoretically this doesn't have to come from Blink. What about:

// Make sure that the vibration values are within reasonable bounds.

[Peter Beverloo, 2015/05/05 20:42:44]

optional nit: You could add a unit test for this in
PlatformNotificationServiceTest.

[Sanghyun Park, 2015/05/06 14:53:25]

Done.

+  // trust any values passed from the client.

[dcheng, 2015/05/05 21:56:33]

I'd prefer to see this sanitization moved to the IPC message handler.

[Sanghyun Park, 2015/05/06 14:53:25]

If logic to use vibrate before this sanitization is added in the future in
NotificationMessageFilter(IPC handling), dcheng's suggestion is  better than
this.

But I think it will not happen, because This function is handling the
notification's data.
And DisplayNotification(use this CreateNotificationFromData function) is called
in IPC message handler function.

@Peter.
WDYT?

[Peter Beverloo, 2015/05/06 15:35:17]

I agree with Daniel's suggestion.

Daniel prefers to see the validation in NotificationMessageFilter because the
primary reason it's there is that we don't trust data coming from the renderer -
other consumers in the browser process are (generally) trusted.

It's most common to do this validation in the message filter because that's
(roughly) where the data comes in, which makes it much easier to validate that
the data is *always* safe to use, rather than only when actually displaying the
notification.

To illustrate this - we would currently store an invalid pattern in the
notification database, and deserialize this again to the developer when
re-creating the notification. When the validation/sanitation would be done early
in the message filter, we would store the sanitized pattern in the database
instead.

[Sanghyun Park, 2015/05/06 17:33:32]

Okay, I see.
I upload patch about this.

Dear Daniel and Peter thanks for your guide and detail explanation :)

+  std::vector<int> vibration_pattern;
+  for (int pattern_entry : notification_data.vibration_pattern) {
+    pattern_entry = std::min(kMaximumVibrationDurationMs,
+        std::max(kMinimumVibrationDurationMs, pattern_entry));
+
+    vibration_pattern.push_back(pattern_entry);
+  }
+  notification.set_vibration_pattern(vibration_pattern);
+
   notification.set_silent(notification_data.silent);
 
   // Web Notifications do not timeout.
   notification.set_never_timeout(true);
 
   return notification;
 }
 
 NotificationUIManager*
 PlatformNotificationServiceImpl::GetNotificationUIManager() const {
@@ skipping 48 matching lines @@
       formatted_origin.push_back(':');
       formatted_origin.append(base::UTF8ToUTF16(origin.port()));
     }
     return formatted_origin;
   }
 
   // TODO(dewittj): Once file:// URLs are passed in to the origin
   // GURL here, begin returning the path as the display name.
   return net::FormatUrl(origin, languages);
 }