Changeset 61316 in webkit

Timestamp:

Jun 17, 2010 2:29:23 AM (14 years ago)

Author:

eric@webkit.org

Message:

2010-06-17 John Gregg <​johnnyg@google.com>

Reviewed by David Levin.

Move the call to the notification presenter that a Notification is being
destroyed from the destructor (not safe) to the ActiveDOMObject::contextDestroyed
method.

Also fix up an incorrect reference loss in the V8 bindings code for Notifications.
​https://bugs.webkit.org/show_bug.cgi?id=40097

No new tests; code paths are well-covered by existing tests.

• bindings/v8/custom/V8NotificationCenterCustom.cpp: (WebCore::V8NotificationCenter::createHTMLNotificationCallback): (WebCore::V8NotificationCenter::createNotificationCallback):
• notifications/Notification.cpp: (WebCore::Notification::~Notification): (WebCore::Notification::contextDestroyed):
• notifications/Notification.h:

2010-06-17 John Gregg <​johnnyg@google.com>

Reviewed by David Levin.

Undo the build fix with the correct patch: Chromium NotificationPresenter
is now informed of the destruction before it actually happens, so this
use of the Notification object is correct.
​https://bugs.webkit.org/show_bug.cgi?id=40097

• src/NotificationPresenterImpl.cpp: (WebKit::NotificationPresenterImpl::notificationObjectDestroyed):

Location:

trunk

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/v8/custom/V8NotificationCenterCustom.cpp (modified) (2 diffs)
• WebCore/notifications/Notification.cpp (modified) (2 diffs)
• WebCore/notifications/Notification.h (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/NotificationPresenterImpl.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-06-17  John Gregg  <johnnyg@google.com>
+
+        Reviewed by David Levin.
+
+        Move the call to the notification presenter that a Notification is being
+        destroyed from the destructor (not safe) to the ActiveDOMObject::contextDestroyed
+        method.
+
+        Also fix up an incorrect reference loss in the V8 bindings code for Notifications.
+        https://bugs.webkit.org/show_bug.cgi?id=40097
+
+        No new tests; code paths are well-covered by existing tests.
+
+        * bindings/v8/custom/V8NotificationCenterCustom.cpp:
+        (WebCore::V8NotificationCenter::createHTMLNotificationCallback):
+        (WebCore::V8NotificationCenter::createNotificationCallback):
+        * notifications/Notification.cpp:
+        (WebCore::Notification::~Notification):
+        (WebCore::Notification::contextDestroyed):
+        * notifications/Notification.h:
+
 2010-06-17  Csaba Osztrogonác  <ossy@webkit.org>
 

trunk/WebCore/bindings/v8/custom/V8NotificationCenterCustom.cpp

@@ -59 +59 @@
         return throwError(ec);
 
+    notification->ref();
     return toV8(notification.get());
 }
@@ -73 +74 @@
         return throwError(ec);
 
+    notification->ref();
     return toV8(notification.get());
 }

trunk/WebCore/notifications/Notification.cpp

@@ -91 +91 @@
         cancel();
     }
-    if (m_presenter)
-        m_presenter->notificationObjectDestroyed(this);
 }
 
@@ -141 +139 @@
 }
 
+void Notification::contextDestroyed()
+{
+    ActiveDOMObject::contextDestroyed();
+    if (m_presenter)
+        m_presenter->notificationObjectDestroyed(this);
+}
 
 void Notification::startLoading()

trunk/WebCore/notifications/Notification.h

@@ -88 +88 @@
         virtual Notification* toNotification() { return this; }
 
+        // ActiveDOMObject interface
+        virtual void contextDestroyed();
+
         void stopLoading();
 

trunk/WebKit/chromium/ChangeLog

@@ +1 @@
+2010-06-17  John Gregg  <johnnyg@google.com>
+
+        Reviewed by David Levin.
+
+        Undo the build fix with the correct patch: Chromium NotificationPresenter
+        is now informed of the destruction before it actually happens, so this
+        use of the Notification object is correct. 
+        https://bugs.webkit.org/show_bug.cgi?id=40097
+
+        * src/NotificationPresenterImpl.cpp:
+        (WebKit::NotificationPresenterImpl::notificationObjectDestroyed):
+
 2010-06-16  Kent Tamura  <tkent@chromium.org>
 

trunk/WebKit/chromium/src/NotificationPresenterImpl.cpp

@@ -89 +89 @@
 void NotificationPresenterImpl::notificationObjectDestroyed(Notification* notification)
 {
-    // TODO(pkasting): We cannot ref an object that's being destroyed.  Either
-    // this function needs to be called earlier than in ~Notification(), or it
-    // needs to not ref this object.
-    //m_presenter->objectDestroyed(PassRefPtr<Notification>(notification));
+    m_presenter->objectDestroyed(PassRefPtr<Notification>(notification));
 }