diff options
author | peter <peter@chromium.org> | 2015-04-15 13:44:28 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-15 20:44:44 +0000 |
commit | 539f51d6af62097059c34b1c19dfbc4352413ad7 (patch) | |
tree | 6fdc003d363f0d38dfa6e6093d2a9e72b983d417 | |
parent | 3ec02d642465872d9ab7d7db600d0480a57b3cab (diff) | |
download | chromium_src-539f51d6af62097059c34b1c19dfbc4352413ad7.zip chromium_src-539f51d6af62097059c34b1c19dfbc4352413ad7.tar.gz chromium_src-539f51d6af62097059c34b1c19dfbc4352413ad7.tar.bz2 |
Implement Notification.data for persistent notifications.
This patch implements the Notification.data property for persistent
notifications, where the data is stored in the Notification Database.
The data associated with non-persistent notifications will never leave
Blink, so no changes are required in Chromium to support this.
As an implementation-defined limit, the size of the data will be
limited to a maximum of 1024^2 characters, with a UMA histogram tracking
the size of what developers are trying to store. The limit can then be
adjusted in future releases to match this more accurately, or as an
argument to introduce such a limit in the specification.
Further layout tests are being introduced in the following CL:
https://codereview.chromium.org/1052693002
BUG=442129
Review URL: https://codereview.chromium.org/1052683002
Cr-Commit-Position: refs/heads/master@{#325295}
11 files changed, 89 insertions, 10 deletions
diff --git a/chrome/test/data/notifications/platform_notification_service.html b/chrome/test/data/notifications/platform_notification_service.html index 7c4eb86..d0b4124 100644 --- a/chrome/test/data/notifications/platform_notification_service.html +++ b/chrome/test/data/notifications/platform_notification_service.html @@ -54,7 +54,10 @@ body: 'Contents', tag: 'replace-id', icon: 'icon.png', - silent: true + silent: true, + data: [ + { property: 'value' } + ] }); } diff --git a/content/browser/notifications/notification_database_data.proto b/content/browser/notifications/notification_database_data.proto index 8b02197..1fb6808 100644 --- a/content/browser/notifications/notification_database_data.proto +++ b/content/browser/notifications/notification_database_data.proto @@ -35,6 +35,7 @@ message NotificationDatabaseDataProto { optional string tag = 5; optional string icon = 6; optional bool silent = 7; + optional bytes data = 8; } optional NotificationData notification_data = 4; diff --git a/content/browser/notifications/notification_database_data_conversions.cc b/content/browser/notifications/notification_database_data_conversions.cc index d183dd4..d656f7d 100644 --- a/content/browser/notifications/notification_database_data_conversions.cc +++ b/content/browser/notifications/notification_database_data_conversions.cc @@ -40,6 +40,11 @@ bool DeserializeNotificationDatabaseData(const std::string& input, notification_data->icon = GURL(payload.icon()); notification_data->silent = payload.silent(); + if (payload.data().length()) { + notification_data->data.assign(payload.data().begin(), + payload.data().end()); + } + return true; } @@ -64,6 +69,11 @@ bool SerializeNotificationDatabaseData(const NotificationDatabaseData& input, payload->set_icon(notification_data.icon.spec()); payload->set_silent(notification_data.silent); + if (notification_data.data.size()) { + payload->set_data(¬ification_data.data.front(), + notification_data.data.size()); + } + NotificationDatabaseDataProto message; message.set_notification_id(input.notification_id); message.set_origin(input.origin.spec()); diff --git a/content/browser/notifications/notification_database_data_unittest.cc b/content/browser/notifications/notification_database_data_unittest.cc index df147d0..f57bfa8 100644 --- a/content/browser/notifications/notification_database_data_unittest.cc +++ b/content/browser/notifications/notification_database_data_unittest.cc @@ -19,8 +19,12 @@ const char kNotificationLang[] = "nl"; const char kNotificationBody[] = "Hello, world!"; const char kNotificationTag[] = "my_tag"; const char kNotificationIconUrl[] = "https://example.com/icon.png"; +const unsigned char kNotificationData[] = { 0xdf, 0xff, 0x0, 0x0, 0xff, 0xdf }; TEST(NotificationDatabaseDataTest, SerializeAndDeserializeData) { + std::vector<char> developer_data( + kNotificationData, kNotificationData + arraysize(kNotificationData)); + PlatformNotificationData notification_data; notification_data.title = base::ASCIIToUTF16(kNotificationTitle); notification_data.direction = @@ -30,6 +34,7 @@ TEST(NotificationDatabaseDataTest, SerializeAndDeserializeData) { notification_data.tag = kNotificationTag; notification_data.icon = GURL(kNotificationIconUrl); notification_data.silent = true; + notification_data.data = developer_data; NotificationDatabaseData database_data; database_data.notification_id = kNotificationId; @@ -64,6 +69,10 @@ TEST(NotificationDatabaseDataTest, SerializeAndDeserializeData) { EXPECT_EQ(notification_data.tag, copied_notification_data.tag); EXPECT_EQ(notification_data.icon, copied_notification_data.icon); EXPECT_EQ(notification_data.silent, copied_notification_data.silent); + + ASSERT_EQ(developer_data.size(), copied_notification_data.data.size()); + for (size_t i = 0; i < developer_data.size(); ++i) + EXPECT_EQ(developer_data[i], copied_notification_data.data[i]); } } // namespace content diff --git a/content/child/notifications/notification_data_conversions.cc b/content/child/notifications/notification_data_conversions.cc index a1b8128..639e555 100644 --- a/content/child/notifications/notification_data_conversions.cc +++ b/content/child/notifications/notification_data_conversions.cc @@ -25,6 +25,7 @@ PlatformNotificationData ToPlatformNotificationData( platform_data.tag = base::UTF16ToUTF8(web_data.tag); platform_data.icon = GURL(web_data.icon.string()); platform_data.silent = web_data.silent; + platform_data.data.assign(web_data.data.begin(), web_data.data.end()); return platform_data; } @@ -43,6 +44,7 @@ WebNotificationData ToWebNotificationData( web_data.tag = blink::WebString::fromUTF8(platform_data.tag); web_data.icon = blink::WebURL(platform_data.icon); web_data.silent = platform_data.silent; + web_data.data = platform_data.data; return web_data; } diff --git a/content/child/notifications/notification_data_conversions_unittest.cc b/content/child/notifications/notification_data_conversions_unittest.cc index f6a1424..daded25 100644 --- a/content/child/notifications/notification_data_conversions_unittest.cc +++ b/content/child/notifications/notification_data_conversions_unittest.cc @@ -4,6 +4,8 @@ #include "content/child/notifications/notification_data_conversions.h" +#include <stdint.h> + #include "base/strings/utf_string_conversions.h" #include "content/public/common/platform_notification_data.h" #include "testing/gtest/include/gtest/gtest.h" @@ -12,17 +14,18 @@ #include "third_party/WebKit/public/platform/modules/notifications/WebNotificationData.h" namespace content { -namespace { const char kNotificationTitle[] = "My Notification"; const char kNotificationLang[] = "nl"; const char kNotificationBody[] = "Hello, world!"; const char kNotificationTag[] = "my_tag"; const char kNotificationIconUrl[] = "https://example.com/icon.png"; - -} +const unsigned char kNotificationData[] = { 0xdf, 0xff, 0x0, 0x0, 0xff, 0xdf }; TEST(NotificationDataConversionsTest, ToPlatformNotificationData) { + std::vector<char> developer_data( + kNotificationData, kNotificationData + arraysize(kNotificationData)); + blink::WebNotificationData web_data( blink::WebString::fromUTF8(kNotificationTitle), blink::WebNotificationData::DirectionLeftToRight, @@ -30,7 +33,8 @@ TEST(NotificationDataConversionsTest, ToPlatformNotificationData) { blink::WebString::fromUTF8(kNotificationBody), blink::WebString::fromUTF8(kNotificationTag), blink::WebURL(GURL(kNotificationIconUrl)), - true /* silent */); + true /* silent */, + blink::WebVector<char>(developer_data)); PlatformNotificationData platform_data = ToPlatformNotificationData(web_data); EXPECT_EQ(base::ASCIIToUTF16(kNotificationTitle), platform_data.title); @@ -41,6 +45,10 @@ TEST(NotificationDataConversionsTest, ToPlatformNotificationData) { EXPECT_EQ(kNotificationTag, platform_data.tag); EXPECT_EQ(kNotificationIconUrl, platform_data.icon.spec()); EXPECT_TRUE(platform_data.silent); + + ASSERT_EQ(developer_data.size(), platform_data.data.size()); + for (size_t i = 0; i < developer_data.size(); ++i) + EXPECT_EQ(developer_data[i], platform_data.data[i]); } TEST(NotificationDataConversionsTest, @@ -60,6 +68,9 @@ TEST(NotificationDataConversionsTest, } TEST(NotificationDataConversionsTest, ToWebNotificationData) { + std::vector<char> developer_data( + kNotificationData, kNotificationData + arraysize(kNotificationData)); + PlatformNotificationData platform_data; platform_data.title = base::ASCIIToUTF16(kNotificationTitle); platform_data.direction = @@ -69,6 +80,7 @@ TEST(NotificationDataConversionsTest, ToWebNotificationData) { platform_data.tag = kNotificationTag; platform_data.icon = GURL(kNotificationIconUrl); platform_data.silent = true; + platform_data.data = developer_data; blink::WebNotificationData web_data = ToWebNotificationData(platform_data); EXPECT_EQ(kNotificationTitle, web_data.title); @@ -79,6 +91,10 @@ TEST(NotificationDataConversionsTest, ToWebNotificationData) { EXPECT_EQ(kNotificationTag, web_data.tag); EXPECT_EQ(kNotificationIconUrl, web_data.icon.string()); EXPECT_TRUE(web_data.silent); + + ASSERT_EQ(developer_data.size(), web_data.data.size()); + for (size_t i = 0; i < developer_data.size(); ++i) + EXPECT_EQ(developer_data[i], web_data.data[i]); } TEST(NotificationDataConversionsTest, ToWebNotificationDataDirectionality) { diff --git a/content/child/notifications/notification_manager.cc b/content/child/notifications/notification_manager.cc index 96963df..b442baf 100644 --- a/content/child/notifications/notification_manager.cc +++ b/content/child/notifications/notification_manager.cc @@ -4,7 +4,10 @@ #include "content/child/notifications/notification_manager.h" +#include <cmath> + #include "base/lazy_instance.h" +#include "base/metrics/histogram_macros.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "base/thread_task_runner_handle.h" @@ -95,6 +98,24 @@ void NotificationManager::showPersistent( service_worker_registration)->registration_id(); scoped_ptr<blink::WebNotificationShowCallbacks> owned_callbacks(callbacks); + + // Verify that the author-provided payload size does not exceed our limit. + // This is an implementation-defined limit to prevent abuse of notification + // data as a storage mechanism. A UMA histogram records the requested sizes, + // which enables us to track how much data authors are attempting to store. + // + // If the size exceeds this limit, reject the showNotification() promise. This + // is outside of the boundaries set by the specification, but it gives authors + // an indication that something has gone wrong. + size_t author_data_size = notification_data.data.size(); + UMA_HISTOGRAM_MEMORY_KB("Notifications.AuthorDataSizeKB", + static_cast<int>(ceil(author_data_size / 1024.0))); + + if (author_data_size > PlatformNotificationData::kMaximumDeveloperDataSize) { + owned_callbacks->onError(); + return; + } + if (notification_data.icon.isEmpty()) { DisplayPersistentNotification(origin, notification_data, diff --git a/content/common/platform_notification_messages.h b/content/common/platform_notification_messages.h index a4ae706..7ad1e6b 100644 --- a/content/common/platform_notification_messages.h +++ b/content/common/platform_notification_messages.h @@ -43,6 +43,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::PlatformNotificationData) IPC_STRUCT_TRAITS_MEMBER(tag) IPC_STRUCT_TRAITS_MEMBER(icon) IPC_STRUCT_TRAITS_MEMBER(silent) + IPC_STRUCT_TRAITS_MEMBER(data) IPC_STRUCT_TRAITS_END() // Messages sent from the browser to the renderer. diff --git a/content/public/common/platform_notification_data.cc b/content/public/common/platform_notification_data.cc index 5b918692..387dee6 100644 --- a/content/public/common/platform_notification_data.cc +++ b/content/public/common/platform_notification_data.cc @@ -6,9 +6,7 @@ namespace content { -PlatformNotificationData::PlatformNotificationData() - : direction(NotificationDirectionLeftToRight), - silent(false) {} +PlatformNotificationData::PlatformNotificationData() {} PlatformNotificationData::~PlatformNotificationData() {} diff --git a/content/public/common/platform_notification_data.h b/content/public/common/platform_notification_data.h index f6c999d..5b608c0 100644 --- a/content/public/common/platform_notification_data.h +++ b/content/public/common/platform_notification_data.h @@ -6,6 +6,7 @@ #define CONTENT_PUBLIC_COMMON_PLATFORM_NOTIFICATION_DATA_H_ #include <string> +#include <vector> #include "base/strings/string16.h" #include "content/common/content_export.h" @@ -20,6 +21,10 @@ struct CONTENT_EXPORT PlatformNotificationData { PlatformNotificationData(); ~PlatformNotificationData(); + // The maximum size of developer-provided data to be stored in the |data| + // property of this structure. + static const size_t kMaximumDeveloperDataSize = 1024 * 1024; + enum NotificationDirection { NotificationDirectionLeftToRight, NotificationDirectionRightToLeft, @@ -31,7 +36,7 @@ struct CONTENT_EXPORT PlatformNotificationData { base::string16 title; // Hint to determine the directionality of the displayed notification. - NotificationDirection direction; + NotificationDirection direction = NotificationDirectionLeftToRight; // BCP 47 language tag describing the notification's contents. Optional. std::string lang; @@ -48,7 +53,11 @@ struct CONTENT_EXPORT PlatformNotificationData { // Whether default notification indicators (sound, vibration, light) should // be suppressed. - bool silent; + bool silent = false; + + // Developer-provided data associated with the notification, in the form of + // a serialized string. Must not exceed |kMaximumDeveloperDataSize| bytes. + std::vector<char> data; }; } // namespace content diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 8e63c6e..79a9a94 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -24222,6 +24222,15 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> +<histogram name="Notifications.AuthorDataSizeKB" units="KB"> + <owner>peter@chromium.org</owner> + <summary> + The size, in kilobytes, of the author-provided data associated with a Web + Notification. Recorded when a persistent Web Notification is being shown by + the developer. + </summary> +</histogram> + <histogram name="Notifications.Database.DeleteResult" enum="NotificationDatabaseStatus"> <owner>peter@chromium.org</owner> |