From 35bd3cd019fa030cad7500cd0d4c47d93d24333e Mon Sep 17 00:00:00 2001 From: John Mellor Date: Thu, 3 Dec 2015 17:14:02 +0000 Subject: [PATCH] Stop requiring NotificationAction action and title to be non-empty Improve spec compliance. Note that http/tests/notifications/serviceworkerregistration-document-actions-throw.html still tests that they are set (but it's no longer possible to have an equivalent check in NotificationDataTest, since required WebIDL attributes are enforced at the V8 level by [1], and are bypassed when creating IDL objects from C++). [1]: https://chromium.googlesource.com/chromium/src/+/dfd8ec1b3edb7f876235bec1138ecec4f2da748d/third_party/WebKit/Source/bindings/templates/dictionary_v8.cpp#55 BUG=557624 Review URL: https://codereview.chromium.org/1487063002 Cr-Commit-Position: refs/heads/master@{#362469} (cherry picked from commit 56c5368ca167742af3dfdd0bf8f879984b2ad05b) Review URL: https://codereview.chromium.org/1495043002 . Cr-Commit-Position: refs/branch-heads/2564@{#217} Cr-Branched-From: 1283eca15bd9f772387f75241576cde7bdec7f54-refs/heads/master@{#359700} --- ...erregistration-document-actions-throw.html | 34 ----------------- .../notifications/NotificationData.cpp | 12 +----- .../notifications/NotificationDataTest.cpp | 37 ------------------- 3 files changed, 1 insertion(+), 82 deletions(-) diff --git a/third_party/WebKit/LayoutTests/http/tests/notifications/serviceworkerregistration-document-actions-throw.html b/third_party/WebKit/LayoutTests/http/tests/notifications/serviceworkerregistration-document-actions-throw.html index 813d6499a8320..5110ca78a6e33 100644 --- a/third_party/WebKit/LayoutTests/http/tests/notifications/serviceworkerregistration-document-actions-throw.html +++ b/third_party/WebKit/LayoutTests/http/tests/notifications/serviceworkerregistration-document-actions-throw.html @@ -48,40 +48,6 @@ }).catch(unreached_rejection(test)); }, 'showNotification() must reject if a NotificationAction has no title.'); - - async_test(function(test) { - var scope = 'resources/scope/' + location.pathname + "/emptyaction"; - - getActiveServiceWorkerWithMessagePort(test, script, scope).then(function(workerInfo) { - assert_inherits(workerInfo.registration, 'showNotification', 'showNotification() must be exposed.'); - - workerInfo.registration.showNotification('Title', { - actions: [{ action: "", title: "Foo" }] - }).then(unreached_fulfillment(test)).catch(function(error) { - assert_equals(error.name, 'TypeError'); - assert_equals(error.message, "Failed to execute 'showNotification' on 'ServiceWorkerRegistration': NotificationAction `action` must not be empty."); - test.done(); - }); - }).catch(unreached_rejection(test)); - - }, 'showNotification() must reject if a NotificationAction has an empty action.'); - - async_test(function(test) { - var scope = 'resources/scope/' + location.pathname + "/emptytitle"; - - getActiveServiceWorkerWithMessagePort(test, script, scope).then(function(workerInfo) { - assert_inherits(workerInfo.registration, 'showNotification', 'showNotification() must be exposed.'); - - workerInfo.registration.showNotification('Title', { - actions: [{ action: "foo", title: "" }] - }).then(unreached_fulfillment(test)).catch(function(error) { - assert_equals(error.name, 'TypeError'); - assert_equals(error.message, "Failed to execute 'showNotification' on 'ServiceWorkerRegistration': NotificationAction `title` must not be empty."); - test.done(); - }); - }).catch(unreached_rejection(test)); - - }, 'showNotification() must reject if a NotificationAction has an empty title.'); diff --git a/third_party/WebKit/Source/modules/notifications/NotificationData.cpp b/third_party/WebKit/Source/modules/notifications/NotificationData.cpp index 4052706c4241c..d7ce4f09e3ef5 100644 --- a/third_party/WebKit/Source/modules/notifications/NotificationData.cpp +++ b/third_party/WebKit/Source/modules/notifications/NotificationData.cpp @@ -75,18 +75,8 @@ WebNotificationData createWebNotificationData(ExecutionContext* executionContext const size_t maxActions = Notification::maxActions(); for (const NotificationAction& action : options.actions()) { - if (action.action().isEmpty()) { - exceptionState.throwTypeError("NotificationAction `action` must not be empty."); - return WebNotificationData(); - } - - if (action.title().isEmpty()) { - exceptionState.throwTypeError("NotificationAction `title` must not be empty."); - return WebNotificationData(); - } - if (actions.size() >= maxActions) - continue; + break; WebNotificationAction webAction; webAction.action = action.action(); diff --git a/third_party/WebKit/Source/modules/notifications/NotificationDataTest.cpp b/third_party/WebKit/Source/modules/notifications/NotificationDataTest.cpp index 39b1916cb1e86..50ae0d379e33d 100644 --- a/third_party/WebKit/Source/modules/notifications/NotificationDataTest.cpp +++ b/third_party/WebKit/Source/modules/notifications/NotificationDataTest.cpp @@ -179,43 +179,6 @@ TEST_F(NotificationDataTest, DirectionValues) } } -TEST_F(NotificationDataTest, RequiredActionProperties) -{ - NotificationOptions options; - - // The NotificationAction.action property is required. - { - NotificationAction action; - action.setTitle(kNotificationActionTitle); - - HeapVector actions; - actions.append(action); - - options.setActions(actions); - - TrackExceptionState exceptionState; - WebNotificationData notificationData = createWebNotificationData(executionContext(), kNotificationTitle, options, exceptionState); - ASSERT_TRUE(exceptionState.hadException()); - EXPECT_EQ("NotificationAction `action` must not be empty.", exceptionState.message()); - } - - // The NotificationAction.title property is required. - { - NotificationAction action; - action.setAction(kNotificationActionAction); - - HeapVector actions; - actions.append(action); - - options.setActions(actions); - - TrackExceptionState exceptionState; - WebNotificationData notificationData = createWebNotificationData(executionContext(), kNotificationTitle, options, exceptionState); - ASSERT_TRUE(exceptionState.hadException()); - EXPECT_EQ("NotificationAction `title` must not be empty.", exceptionState.message()); - } -} - TEST_F(NotificationDataTest, MaximumActionCount) { HeapVector actions;