Merge the code paths for closing different kinds of notifications.

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

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 149 matching lines @@
     DCHECK(origin);
 
     notificationManager()->show(WebSecurityOrigin(origin), m_data, loader->getResources(), this);
     m_loader.clear();
 
     m_state = State::Showing;
 }
 
 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.
     if (m_state != State::Showing && m_state != State::Closing)
         return;
 
     m_state = State::Closed;
     dispatchEvent(Event::create(EventTypeNames::close));
 }
 
+void Notification::dispatchErrorEvent()
+{
+    dispatchEvent(Event::create(EventTypeNames::error));
+}
+
 String Notification::title() const
 {
     return m_data.title;
 }
 
 String Notification::dir() const
 {
     switch (m_data.direction) {
     case WebNotificationData::DirectionLeftToRight:
         return "ltr";
@@ skipping 171 matching lines @@
 
 DEFINE_TRACE(Notification)
 {
     visitor->trace(m_prepareShowMethodRunner);
     visitor->trace(m_loader);
     EventTargetWithInlineData::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 } // namespace blink