Generalize validation of developer input for Web Notifications

Source/modules/notifications/NotificationData.cpp

+// Copyright 2015 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 "config.h"
+#include "modules/notifications/NotificationData.h"
+
+#include "bindings/core/v8/ExceptionState.h"
+#include "bindings/core/v8/SerializedScriptValue.h"
+#include "bindings/core/v8/SerializedScriptValueFactory.h"
+#include "core/dom/ExecutionContext.h"
+#include "modules/notifications/Notification.h"
+#include "modules/notifications/NotificationOptions.h"
+#include "modules/vibration/NavigatorVibration.h"
+#include "platform/weborigin/KURL.h"
+
+namespace blink {
+
+WebNotificationData createWebNotificationData(ExecutionContext* executionContext, const String& title, const NotificationOptions& options, ExceptionState& exceptionState)
+{
+    // If silent is true, the notification must not have a vibration pattern.
+    if (options.hasVibrate() && options.silent()) {
+        exceptionState.throwTypeError("Silent notifications must not specify vibration patterns.");
+        return WebNotificationData();
+    }
+
+    WebNotificationData webData;
+
+    // TODO(peter): Require a title to be given for notifications?
+    webData.title = title;
+
+    // TODO(peter): WebNotificationData should be able to reflect "auto" as well.
+    webData.direction = options.dir() == "rtl" ? WebNotificationData::DirectionRightToLeft : WebNotificationData::DirectionLeftToRight;
+
+    webData.lang = options.lang();
+    webData.body = options.body();
+    webData.tag = options.tag();
+
+    KURL iconUrl;
+
+    // TODO(peter): Apply the appropriate CORS checks on the |iconUrl|.
+    if (options.hasIcon() && !options.icon().isEmpty()) {
+        iconUrl = executionContext->completeURL(options.icon());
+        if (!iconUrl.isValid())
+            iconUrl = KURL();
+    }
+
+    webData.icon = iconUrl;
+    webData.vibrate = NavigatorVibration::sanitizeVibrationPattern(options.vibrate());
+    webData.silent = options.silent();
+
+    if (options.hasData()) {
+        RefPtr<SerializedScriptValue> serializedScriptValue = SerializedScriptValueFactory::instance().create(options.data().isolate(), options.data(), nullptr, exceptionState);
+        if (exceptionState.hadException())
+            return WebNotificationData();
+
+        Vector<char> serializedData;
+        serializedScriptValue->toWireBytes(serializedData);
+
+        webData.data = serializedData;
+    }
+
+    Vector<WebNotificationAction> actions;
+
+    const size_t maxActions = Notification::maxActions();

[johnme, 2015/08/03 15:41:36]

Nit: seems more legible if you don't inline this.

[Peter Beverloo, 2015/08/03 17:29:30]

Acknowledged.

+    for (const NotificationAction& action : options.actions()) {
+        if (action.title().isEmpty()) {
+            exceptionState.throwTypeError("Notification action titles must not be empty.");
+            return WebNotificationData();
+        }
+
+        // TODO(johnme): Ensure that |action.action()| isn't empty.

[johnme, 2015/08/03 15:41:36]

https://codereview.chromium.org/1263043003 has been on the CQ for 2 hours, and
should land soon. You'll probably need to rebase this accordingly.

[Peter Beverloo, 2015/08/03 17:29:30]

Done.

+
+        if (actions.size() >= maxActions)
+            continue;
+
+        WebNotificationAction webAction;
+        webAction.title = action.title();
+
+        actions.append(webAction);
+    }
+
+    webData.actions = actions;
+
+    return webData;
+}
+
+} // namespace blink