Move notification resource loading from content/child to blink

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

Index: third_party/WebKit/Source/modules/notifications/Notification.cpp
diff --git a/third_party/WebKit/Source/modules/notifications/Notification.cpp b/third_party/WebKit/Source/modules/notifications/Notification.cpp
index 571912ded3ddb32035f64812da5fa4061b0fe21e..85dac91a39adc261be2a1622edff9a8d716d0242 100644
--- a/third_party/WebKit/Source/modules/notifications/Notification.cpp
+++ b/third_party/WebKit/Source/modules/notifications/Notification.cpp
@@ -43,6 +43,7 @@
 #include "modules/notifications/NotificationData.h"
 #include "modules/notifications/NotificationOptions.h"
 #include "modules/notifications/NotificationPermissionClient.h"
+#include "modules/notifications/NotificationResourcesLoader.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/UserGestureIndicator.h"
 #include "public/platform/Platform.h"
@@ -50,6 +51,7 @@
 #include "public/platform/WebString.h"
 #include "public/platform/modules/notifications/WebNotificationAction.h"
 #include "public/platform/modules/notifications/WebNotificationManager.h"
+#include "wtf/Functional.h"
 
 namespace blink {
 namespace {
@@ -99,7 +101,7 @@ Notification* Notification::create(ExecutionContext* context, const String& titl
         return nullptr;
 
     Notification* notification = new Notification(context, data);
-    notification->scheduleShow();
+    notification->schedulePrepareShow();
     notification->suspendIfNeeded();
 
     return notification;
@@ -121,7 +123,7 @@ Notification::Notification(ExecutionContext* context, const WebNotificationData&
     , m_data(data)
     , m_persistentId(kInvalidPersistentId)
     , m_state(NotificationStateIdle)
-    , m_asyncRunner(AsyncMethodRunner<Notification>::create(this, &Notification::show))
+    , m_prepareShowMethodRunner(AsyncMethodRunner<Notification>::create(this, &Notification::prepareShow))
 {
     ASSERT(notificationManager());
 }
@@ -130,15 +132,15 @@ Notification::~Notification()
 {
 }
 
-void Notification::scheduleShow()
+void Notification::schedulePrepareShow()
 {
     ASSERT(m_state == NotificationStateIdle);
-    ASSERT(!m_asyncRunner->isActive());
+    ASSERT(!m_prepareShowMethodRunner->isActive());
 
-    m_asyncRunner->runAsync();
+    m_prepareShowMethodRunner->runAsync();
 }
 
-void Notification::show()
+void Notification::prepareShow()
 {
     ASSERT(m_state == NotificationStateIdle);
     if (Notification::checkPermission(getExecutionContext()) != WebNotificationPermissionAllowed) {
@@ -146,10 +148,21 @@ void Notification::show()
         return;
     }
 
+    m_loader = new NotificationResourcesLoader(getExecutionContext(),
+        bind<NotificationResourcesLoader*>(&Notification::didLoadResources,

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

Why is it necessary to define the `UnboundParameters` template type? Because of
currying? Other users seem to omit it.

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

+hiroshige for how to write bind functions.

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

I'd be happy to see a working example of how to bind std::unique_ptr! I tried
adding it in FunctionalTest but no permutation of syntaxes would build.

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

You don't need to specify it when there are no unbound params, when you're
creating a closure. If there are then it is required in order to build. I dunno
what tricks base::Bind did to avoid this verbosity.

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

Do you want to call WTF::Function binding unique_ptr multiple times? If so,
probably we need something like base::Owned(). If not (i.e. we only call such
Function once), then more support for move-only types will resolve it.
+tzik@

[Michael van Ouwerkerk, 2016/04/14 14:54:41]

I wanted to curry a param that was to be moved. It's a regular class, wrapped in
OwnPtr, but std::unique_ptr is nicer. The WTF::Function is to be called only
once.

+            WeakPersistentThisPointer<Notification>(this)));
+    m_loader->start(m_data);
+}
+
+void Notification::didLoadResources(NotificationResourcesLoader* loader)
+{
+    DCHECK_EQ(loader, m_loader.get());
+
     SecurityOrigin* origin = getExecutionContext()->getSecurityOrigin();
     ASSERT(origin);
 
-    notificationManager()->show(WebSecurityOrigin(origin), m_data, this);
+    notificationManager()->show(WebSecurityOrigin(origin), m_data, loader->getResources(), this);
+    m_loader.clear();
 
     m_state = NotificationStateShowing;
 }
@@ -386,17 +399,18 @@ void Notification::stop()
 
     m_state = NotificationStateClosed;
 
-    m_asyncRunner->stop();
+    m_prepareShowMethodRunner->stop();

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

Maybe add a comment that the |m_loader|, when active, will stop itself?

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

Done.

 }
 
 bool Notification::hasPendingActivity() const
 {
-    return m_state == NotificationStateShowing || m_asyncRunner->isActive();
+    return m_state == NotificationStateShowing || m_prepareShowMethodRunner->isActive() || m_loader;
 }
 
 DEFINE_TRACE(Notification)
 {
-    visitor->trace(m_asyncRunner);
+    visitor->trace(m_prepareShowMethodRunner);
+    visitor->trace(m_loader);
     EventTargetWithInlineData::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }