Add the data attribute to the Notification object

Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp

 // 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 "config.h"
 #include "modules/notifications/ServiceWorkerRegistrationNotifications.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
+#include "bindings/core/v8/ScriptValue.h"
+#include "bindings/core/v8/SerializedScriptValue.h"
+#include "bindings/core/v8/SerializedScriptValueFactory.h"
 #include "bindings/core/v8/V8ThrowException.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContext.h"
 #include "modules/notifications/Notification.h"
 #include "modules/notifications/NotificationOptions.h"
 #include "platform/weborigin/KURL.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebSerializedOrigin.h"
 #include "public/platform/modules/notifications/WebNotificationData.h"
 #include "public/platform/modules/notifications/WebNotificationManager.h"
 
 namespace blink {
 
-ScriptPromise ServiceWorkerRegistrationNotifications::showNotification(ScriptState* scriptState, ServiceWorkerRegistration& serviceWorkerRegistration, const String& title, const NotificationOptions& options)
+ScriptPromise ServiceWorkerRegistrationNotifications::showNotification(ScriptState* scriptState, ServiceWorkerRegistration& serviceWorkerRegistration, const String& title, const NotificationOptions& options, ExceptionState& exceptionState)
 {
     ExecutionContext* executionContext = scriptState->executionContext();
 
     // If context object's active worker is null, reject promise with a TypeError exception.
     if (!serviceWorkerRegistration.active())
         return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(scriptState->isolate(), "No active registration available on the ServiceWorkerRegistration."));
 
     // If permission for notification's origin is not "granted", reject promise with a TypeError exception, and terminate these substeps.
     if (Notification::checkPermission(executionContext) != WebNotificationPermissionAllowed)
         return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(scriptState->isolate(), "No notification permission has been granted for this origin."));
 
     RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
     // FIXME: Do the appropriate CORS checks on the icon URL.
 
     KURL iconUrl;
     if (options.hasIcon() && !options.icon().isEmpty()) {
         iconUrl = executionContext->completeURL(options.icon());
         if (!iconUrl.isValid())
             iconUrl = KURL();
     }
 
     WebNotificationData::Direction dir = options.dir() == "rtl" ? WebNotificationData::DirectionRightToLeft : WebNotificationData::DirectionLeftToRight;
-    WebNotificationData notification(title, dir, options.lang(), options.body(), options.tag(), iconUrl, options.silent());
+

[Peter Beverloo, 2015/03/10 23:34:27]

Mmm this duplication is not great. It happens in more places too. Would you mind
adding a FIXME around line 42 for unifying these code paths at some point in the
future? I'm not sure about a nice way to do that yet :-).

[Sanghyun Park, 2015/03/11 10:59:52]

I cannot understand your comment properly. 
Is your meant that these codes are duplicated with Notification.cpp's code?
If my understanding is right, I'll add "FIXME" comment.
And I'll try to find solution. :)

+    RefPtr<SerializedScriptValue> data;
+    if (options.hasData()) {
+        data = SerializedScriptValueFactory::instance().create(options.data(), 0, exceptionState, options.data().isolate());

[Peter Beverloo, 2015/03/10 23:34:27]

nit: dito as in Notification.cpp, if exceptionState.hadException() is true then
we need to reject the |promise| and abort.

[Sanghyun Park, 2015/03/11 10:59:52]

I'll fix this.

+        data->registerMemoryAllocatedWithCurrentScriptContext();

[Peter Beverloo, 2015/03/10 23:34:27]

nit: dito as in Notification.cpp, we shouldn't have
registerMemoryAllocatedWithCurrentScriptContext() here.

[Sanghyun Park, 2015/03/11 10:59:52]

Ditto

+    }
+
+    WebNotificationData notification(title, dir, options.lang(), options.body(), options.tag(), iconUrl, options.silent(), data.release());

[Peter Beverloo, 2015/03/10 23:34:27]

note: data.release() is fine here, because we won't be using |data| anywhere
else anymore.

     WebNotificationShowCallbacks* callbacks = new CallbackPromiseAdapter<void, void>(resolver);
 
     SecurityOrigin* origin = executionContext->securityOrigin();
     ASSERT(origin);
 
     WebNotificationManager* notificationManager = Platform::current()->notificationManager();
     ASSERT(notificationManager);
 
     notificationManager->showPersistent(WebSerializedOrigin(*origin), notification, serviceWorkerRegistration.webRegistration(), callbacks);
     return promise;
 }
 
 } // namespace blink