Move notification resource loading from content/child to blink

third_party/WebKit/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp

Index: third_party/WebKit/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp
diff --git a/third_party/WebKit/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp b/third_party/WebKit/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp
index db1353dc0f7341ab1a032ace6e0e2a38d6faefe2..5d983a622814d14f0c560a727cfbc29237394c4f 100644
--- a/third_party/WebKit/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp
+++ b/third_party/WebKit/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp
@@ -12,12 +12,14 @@
 #include "modules/notifications/Notification.h"
 #include "modules/notifications/NotificationData.h"
 #include "modules/notifications/NotificationOptions.h"
+#include "modules/notifications/NotificationResourcesLoader.h"
 #include "modules/serviceworkers/ServiceWorkerRegistration.h"
 #include "platform/Histogram.h"
+#include "platform/heap/Handle.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebSecurityOrigin.h"
 #include "public/platform/modules/notifications/WebNotificationData.h"
-#include "public/platform/modules/notifications/WebNotificationManager.h"
+#include "wtf/RefPtr.h"
 
 namespace blink {
 namespace {
@@ -43,12 +45,21 @@ private:
 
 } // namespace
 
-ScriptPromise ServiceWorkerRegistrationNotifications::showNotification(ScriptState* scriptState, ServiceWorkerRegistration& serviceWorkerRegistration, const String& title, const NotificationOptions& options, ExceptionState& exceptionState)
+ServiceWorkerRegistrationNotifications::ServiceWorkerRegistrationNotifications(ServiceWorkerRegistration* registration)
+    : m_registration(registration)
+{
+}
+
+ServiceWorkerRegistrationNotifications::~ServiceWorkerRegistrationNotifications()
+{
+}
+
+ScriptPromise ServiceWorkerRegistrationNotifications::showNotification(ScriptState* scriptState, ServiceWorkerRegistration& registration, const String& title, const NotificationOptions& options, ExceptionState& exceptionState)
 {
     ExecutionContext* executionContext = scriptState->getExecutionContext();
 
     // If context object's active worker is null, reject promise with a TypeError exception.
-    if (!serviceWorkerRegistration.active())
+    if (!registration.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.
@@ -67,17 +78,13 @@ ScriptPromise ServiceWorkerRegistrationNotifications::showNotification(ScriptSta
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
-    WebNotificationShowCallbacks* callbacks = new CallbackPromiseAdapter<void, void>(resolver);
+    OwnPtr<WebNotificationShowCallbacks> callbacks = adoptPtr(new CallbackPromiseAdapter<void, void>(resolver));
+    ServiceWorkerRegistrationNotifications::from(registration).prepareShow(executionContext, data, callbacks.release());
 
-    SecurityOrigin* origin = executionContext->getSecurityOrigin();
-    WebNotificationManager* notificationManager = Platform::current()->notificationManager();
-    ASSERT(notificationManager);
-
-    notificationManager->showPersistent(WebSecurityOrigin(origin), data, serviceWorkerRegistration.webRegistration(), callbacks);
     return promise;
 }
 
-ScriptPromise ServiceWorkerRegistrationNotifications::getNotifications(ScriptState* scriptState, ServiceWorkerRegistration& serviceWorkerRegistration, const GetNotificationOptions& options)
+ScriptPromise ServiceWorkerRegistrationNotifications::getNotifications(ScriptState* scriptState, ServiceWorkerRegistration& registration, const GetNotificationOptions& options)
 {
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
@@ -87,8 +94,54 @@ ScriptPromise ServiceWorkerRegistrationNotifications::getNotifications(ScriptSta
     WebNotificationManager* notificationManager = Platform::current()->notificationManager();
     ASSERT(notificationManager);
 
-    notificationManager->getNotifications(options.tag(), serviceWorkerRegistration.webRegistration(), callbacks);
+    notificationManager->getNotifications(options.tag(), registration.webRegistration(), callbacks);
     return promise;
 }
 
+DEFINE_TRACE(ServiceWorkerRegistrationNotifications)
+{
+    visitor->trace(m_registration);
+    visitor->trace(m_loaders);
+    Supplement<ServiceWorkerRegistration>::trace(visitor);
+}
+
+const char* ServiceWorkerRegistrationNotifications::supplementName()
+{
+    return "ServiceWorkerRegistrationNotifications";
+}
+
+ServiceWorkerRegistrationNotifications& ServiceWorkerRegistrationNotifications::from(ServiceWorkerRegistration& registration)
+{
+    ServiceWorkerRegistrationNotifications* supplement = static_cast<ServiceWorkerRegistrationNotifications*>(Supplement<ServiceWorkerRegistration>::from(registration, supplementName()));
+    if (!supplement) {
+        supplement = new ServiceWorkerRegistrationNotifications(&registration);
+        provideTo(registration, supplementName(), supplement);
+    }
+    return *supplement;
+}
+
+void ServiceWorkerRegistrationNotifications::prepareShow(ExecutionContext* executionContext, const WebNotificationData& data, PassOwnPtr<WebNotificationShowCallbacks> callbacks)
+{
+    RefPtr<SecurityOrigin> origin = executionContext->getSecurityOrigin();
+    NotificationResourcesLoader* loader = new NotificationResourcesLoader(
+        executionContext,
+        bind<NotificationResourcesLoader*>(
+            &ServiceWorkerRegistrationNotifications::didLoadResources,
+            WeakPersistentThisPointer<ServiceWorkerRegistrationNotifications>(this),

[haraken, 2016/04/14 02:05:52]

+hiroshige

[Michael van Ouwerkerk, 2016/04/14 13:42:12]

So I think now that this should be ok. The problem I had was rooted in a cycle
of GC and non-GC types, that caused persistents to not be cleared in
TreadState::cleanup.

[hiroshige, 2016/04/14 14:49:22]

FYI for haraken: WeakPersistentThisPointer<> is just like binding WeakPersistent
as |this| pointer, with null check before invocation.

+            origin.release(), data, callbacks));
+    m_loaders.add(loader);
+    loader->start(data);
+}
+
+void ServiceWorkerRegistrationNotifications::didLoadResources(PassRefPtr<SecurityOrigin> origin, const WebNotificationData& data, PassOwnPtr<WebNotificationShowCallbacks> callbacks, NotificationResourcesLoader* loader)
+{
+    DCHECK(m_loaders.contains(loader));
+
+    WebNotificationManager* notificationManager = Platform::current()->notificationManager();
+    ASSERT(notificationManager);

[Peter Beverloo, 2016/04/13 18:32:28]

ASSERT or DCHECK?

[Michael van Ouwerkerk, 2016/04/14 13:42:12]

DCHECK then. I think the codebase-wide refactor is in progress, but no need to
use ASSERT in this new code.

+
+    notificationManager->showPersistent(WebSecurityOrigin(origin.get()), data, loader->getResources(), m_registration->webRegistration(), callbacks.leakPtr());
+    m_loaders.remove(loader);
+}
+
 } // namespace blink