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 56c5368) Review URL: https://codereview.chromium.org/1495043002 . Cr-Commit-Position: refs/branch-heads/2564@{#217} Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
crosswalk-project · Dec 3, 2015 · 35bd3cd · 35bd3cd
1 parent e586fe0
commit 35bd3cd
Show file tree

Hide file tree

Showing 3 changed files with 1 addition and 82 deletions.
diff --git a/...ayoutTests/http/tests/notifications/serviceworkerregistration-document-actions-throw.html b/...ayoutTests/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.');
     </script>
   </body>
 </html>
diff --git 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
@@ -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<NotificationAction> 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<NotificationAction> 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<NotificationAction> actions;