Merge the code paths for closing different kinds of notifications.

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 2e748e7b4f7e307719789d565d32580e3a49cd8e..76554747750b011aa3e6a242b6c450b3dba1c2d0 100644
--- a/third_party/WebKit/Source/modules/notifications/Notification.cpp
+++ b/third_party/WebKit/Source/modules/notifications/Notification.cpp
@@ -167,45 +167,40 @@ void Notification::didLoadResources(NotificationResourcesLoader* loader)
 
 void Notification::close()
 {
-    if (m_state != State::Showing)
+    if (m_state != State::Showing || m_notificationId.isEmpty())

[johnme, 2016/09/23 16:14:58]

Is this possible? Or can it just be a DCHECK? If it is possible, should you set
state to Closing here and have didShowNotification perform the close if state is
Closing?

[Peter Beverloo, 2016/10/04 10:27:23]

This would happen in time it takes for the Show IPC to round-trip for
non-persistent notifications.

I've added an m_requestedClose to do this in a delayed manner, and added a TODO
for canceling the load instead of waiting for it to finish. Will follow up after
the reformat.

         return;
 
     // Schedule the "close" event to be fired for non-persistent notifications.
     // Persistent notifications won't get such events for programmatic closes.
     if (m_type == Type::NonPersistent) {
-        getExecutionContext()->postTask(BLINK_FROM_HERE, createSameThreadTask(&Notification::dispatchCloseEvent, wrapPersistent(this)));
+        getExecutionContext()->postTask(BLINK_FROM_HERE, createSameThreadTask(&Notification::didCloseNotification, wrapPersistent(this)));
         m_state = State::Closing;
-
-        notificationManager()->close(this);
-        return;
+    } else {
+        m_state = State::Closed;
     }
 
-    m_state = State::Closed;
-
     SecurityOrigin* origin = getExecutionContext()->getSecurityOrigin();
     DCHECK(origin);
 
-    notificationManager()->closePersistent(WebSecurityOrigin(origin), m_data.tag, m_notificationId);
+    notificationManager()->close(WebSecurityOrigin(origin), m_data.tag, m_notificationId);
 }
 
-void Notification::dispatchShowEvent()
+void Notification::didShowNotification(const WebString& notificationId)
 {
+    DCHECK(m_notificationId.isEmpty());
+    m_notificationId = notificationId;
+
     dispatchEvent(Event::create(EventTypeNames::show));
 }
 
-void Notification::dispatchClickEvent()
+void Notification::didClickNotification()
 {
     UserGestureIndicator gestureIndicator(DefinitelyProcessingNewUserGesture);
     ScopedWindowFocusAllowedIndicator windowFocusAllowed(getExecutionContext());
     dispatchEvent(Event::create(EventTypeNames::click));
 }
 
-void Notification::dispatchErrorEvent()
-{
-    dispatchEvent(Event::create(EventTypeNames::error));
-}
-
-void Notification::dispatchCloseEvent()
+void Notification::didCloseNotification()
 {
     // The notification will be showing when the user initiated the close, or it will be
     // closing if the developer initiated the close.
@@ -216,6 +211,11 @@ void Notification::dispatchCloseEvent()
     dispatchEvent(Event::create(EventTypeNames::close));
 }
 
+void Notification::dispatchErrorEvent()
+{
+    dispatchEvent(Event::create(EventTypeNames::error));
+}
+
 String Notification::title() const
 {
     return m_data.title;