Title: [111836] trunk/Source/WebCore
Revision
111836
Author
[email protected]
Date
2012-03-23 01:04:25 -0700 (Fri, 23 Mar 2012)

Log Message

Move Notifications APIs from DOMWindow.idl to DOMWindowNotifications.idl (Part 2)
https://bugs.webkit.org/show_bug.cgi?id=82026

Reviewed by Kentaro Hara.

This patch removes DOMWindow::resetNotifications, which was unneeded
special-case logic for clearing the notifications center.  The previous
patch that tried to accomplish the same thing did not override
willDetachPage, which is why it caused crashes.

There's actually a cleaner way to handle these cases, which will let us
implement reconnectFrame, but that will need to wait for the next
patch.

* notifications/DOMWindowNotifications.cpp:
(WebCore::DOMWindowNotifications::DOMWindowNotifications):
(WebCore::DOMWindowNotifications::from):
(WebCore::DOMWindowNotifications::webkitNotifications):
(WebCore):
(WebCore::DOMWindowNotifications::disconnectFrame):
(WebCore::DOMWindowNotifications::willDetachPage):
(WebCore::DOMWindowNotifications::reset):
* notifications/DOMWindowNotifications.h:
(DOMWindowNotifications):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::willDetachPage):
(WebCore::DOMWindow::disconnectDOMWindowProperties):
(WebCore::DOMWindow::clearDOMWindowProperties):
* page/DOMWindow.h:
(DOMWindow):
* page/Frame.cpp:
(WebCore::Frame::willDetachPage):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (111835 => 111836)


--- trunk/Source/WebCore/ChangeLog	2012-03-23 07:25:25 UTC (rev 111835)
+++ trunk/Source/WebCore/ChangeLog	2012-03-23 08:04:25 UTC (rev 111836)
@@ -1,3 +1,38 @@
+2012-03-23  Adam Barth  <[email protected]>
+
+        Move Notifications APIs from DOMWindow.idl to DOMWindowNotifications.idl (Part 2)
+        https://bugs.webkit.org/show_bug.cgi?id=82026
+
+        Reviewed by Kentaro Hara.
+
+        This patch removes DOMWindow::resetNotifications, which was unneeded
+        special-case logic for clearing the notifications center.  The previous
+        patch that tried to accomplish the same thing did not override
+        willDetachPage, which is why it caused crashes.
+
+        There's actually a cleaner way to handle these cases, which will let us
+        implement reconnectFrame, but that will need to wait for the next
+        patch.
+
+        * notifications/DOMWindowNotifications.cpp:
+        (WebCore::DOMWindowNotifications::DOMWindowNotifications):
+        (WebCore::DOMWindowNotifications::from):
+        (WebCore::DOMWindowNotifications::webkitNotifications):
+        (WebCore):
+        (WebCore::DOMWindowNotifications::disconnectFrame):
+        (WebCore::DOMWindowNotifications::willDetachPage):
+        (WebCore::DOMWindowNotifications::reset):
+        * notifications/DOMWindowNotifications.h:
+        (DOMWindowNotifications):
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::willDetachPage):
+        (WebCore::DOMWindow::disconnectDOMWindowProperties):
+        (WebCore::DOMWindow::clearDOMWindowProperties):
+        * page/DOMWindow.h:
+        (DOMWindow):
+        * page/Frame.cpp:
+        (WebCore::Frame::willDetachPage):
+
 2012-03-22  Adam Barth  <[email protected]>
 
         Move Notifications APIs from DOMWindow.idl to DOMWindowNotifications.idl (Part 1)

Modified: trunk/Source/WebCore/notifications/DOMWindowNotifications.cpp (111835 => 111836)


--- trunk/Source/WebCore/notifications/DOMWindowNotifications.cpp	2012-03-23 07:25:25 UTC (rev 111835)
+++ trunk/Source/WebCore/notifications/DOMWindowNotifications.cpp	2012-03-23 08:04:25 UTC (rev 111836)
@@ -37,7 +37,9 @@
 
 namespace WebCore {
 
-DOMWindowNotifications::DOMWindowNotifications()
+DOMWindowNotifications::DOMWindowNotifications(DOMWindow* window)
+    : DOMWindowProperty(window->frame())
+    , m_window(window)
 {
 }
 
@@ -47,25 +49,41 @@
 
 DOMWindowNotifications* DOMWindowNotifications::from(DOMWindow* window)
 {
-    DOMWindowNotifications* supplement = static_cast<DOMWindowNotifications*>(Supplement<DOMWindow>::from(window, supplementName()));
+    DEFINE_STATIC_LOCAL(AtomicString, supplementName, ("DOMWindowNotifications"));
+    DOMWindowNotifications* supplement = static_cast<DOMWindowNotifications*>(Supplement<DOMWindow>::from(window, supplementName));
     if (!supplement) {
-        supplement = new DOMWindowNotifications();
-        Supplement<DOMWindow>::provideTo(window, supplementName(), adoptPtr(supplement));
+        supplement = new DOMWindowNotifications(window);
+        Supplement<DOMWindow>::provideTo(window, supplementName, adoptPtr(supplement));
     }
     return supplement;
 }
 
 NotificationCenter* DOMWindowNotifications::webkitNotifications(DOMWindow* window)
 {
-    if (!window->isCurrentlyDisplayedInFrame())
+    return DOMWindowNotifications::from(window)->webkitNotifications();
+}
+
+void DOMWindowNotifications::disconnectFrame()
+{
+    reset();
+    DOMWindowProperty::disconnectFrame();
+}
+
+void DOMWindowNotifications::willDetachPage()
+{
+    reset();
+    DOMWindowProperty::willDetachPage();
+}
+
+NotificationCenter* DOMWindowNotifications::webkitNotifications()
+{
+    if (!m_window->isCurrentlyDisplayedInFrame())
         return 0;
 
-    DOMWindowNotifications* supplement = DOMWindowNotifications::from(window);
+    if (m_notificationCenter)
+        return m_notificationCenter.get();
 
-    if (supplement->m_notificationCenter)
-        return supplement->m_notificationCenter.get();
-
-    Document* document = window->document();
+    Document* document = m_window->document();
     if (!document)
         return 0;
     
@@ -75,28 +93,19 @@
 
     NotificationClient* provider = NotificationController::clientFrom(page);
     if (provider) 
-        supplement->m_notificationCenter = NotificationCenter::create(document, provider);    
+        m_notificationCenter = NotificationCenter::create(document, provider);    
 
-    return supplement->m_notificationCenter.get();
+    return m_notificationCenter.get();
 }
 
-void DOMWindowNotifications::reset(DOMWindow* window)
+void DOMWindowNotifications::reset()
 {
-    DOMWindowNotifications* supplement = static_cast<DOMWindowNotifications*>(Supplement<DOMWindow>::from(window, supplementName()));
-    if (!supplement)
+    if (!m_notificationCenter)
         return;
-    if (!supplement->m_notificationCenter)
-        return;
-    supplement->m_notificationCenter->disconnectFrame();
-    supplement->m_notificationCenter = 0;
+    m_notificationCenter->disconnectFrame();
+    m_notificationCenter = 0;
 }
 
-const AtomicString& DOMWindowNotifications::supplementName()
-{
-    DEFINE_STATIC_LOCAL(AtomicString, supplementName, ("DOMWindowNotifications"));
-    return supplementName;
-}
-
 } // namespace WebCore
 
 #endif // ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)

Modified: trunk/Source/WebCore/notifications/DOMWindowNotifications.h (111835 => 111836)


--- trunk/Source/WebCore/notifications/DOMWindowNotifications.h	2012-03-23 07:25:25 UTC (rev 111835)
+++ trunk/Source/WebCore/notifications/DOMWindowNotifications.h	2012-03-23 08:04:25 UTC (rev 111836)
@@ -38,20 +38,24 @@
 class DOMWindow;
 class NotificationCenter;
 
-class DOMWindowNotifications : public Supplement<DOMWindow> {
+class DOMWindowNotifications : public Supplement<DOMWindow>, public DOMWindowProperty {
 public:
     virtual ~DOMWindowNotifications();
 
     static NotificationCenter* webkitNotifications(DOMWindow*);
     static DOMWindowNotifications* from(DOMWindow*);
 
-    static void reset(DOMWindow*);
+    virtual void disconnectFrame() OVERRIDE;
+    // FIXME: Support reconnectFrame().
+    virtual void willDetachPage() OVERRIDE;
 
 private:
-    DOMWindowNotifications();
+    explicit DOMWindowNotifications(DOMWindow*);
 
-    static const AtomicString& supplementName();
+    NotificationCenter* webkitNotifications();
+    void reset();
 
+    DOMWindow* m_window;
     RefPtr<NotificationCenter> m_notificationCenter;
 };
 

Modified: trunk/Source/WebCore/page/DOMWindow.cpp (111835 => 111836)


--- trunk/Source/WebCore/page/DOMWindow.cpp	2012-03-23 07:25:25 UTC (rev 111835)
+++ trunk/Source/WebCore/page/DOMWindow.cpp	2012-03-23 08:04:25 UTC (rev 111836)
@@ -474,12 +474,6 @@
 {
     InspectorInstrumentation::frameWindowDiscarded(m_frame, this);
 
-#if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)
-    // Clearing Notifications requests involves accessing the client so it must be done
-    // before the frame is detached.
-    resetNotifications();
-#endif
-
     HashSet<DOMWindowProperty*>::iterator stop = m_properties.end();
     for (HashSet<DOMWindowProperty*>::iterator it = m_properties.begin(); it != stop; ++it)
         (*it)->willDetachPage();
@@ -523,15 +517,6 @@
     HashSet<DOMWindowProperty*>::iterator stop = m_properties.end();
     for (HashSet<DOMWindowProperty*>::iterator it = m_properties.begin(); it != stop; ++it)
         (*it)->disconnectFrame();
-
-#if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)
-    // FIXME: Notifications shouldn't have different disconnection logic than
-    // the rest of the DOMWindowProperties.
-    // There is currently no way to reconnect them in resumeFromPageCache() so
-    // they will be broken after returning to a cached page.
-    // This should be fixed as part of https://bugs.webkit.org/show_bug.cgi?id=79636
-    resetNotifications();
-#endif
 }
 
 void DOMWindow::reconnectDOMWindowProperties()
@@ -567,11 +552,6 @@
     m_sessionStorage = 0;
     m_localStorage = 0;
     m_applicationCache = 0;
-#if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)
-    // FIXME: Notifications shouldn't have different disconnection logic than
-    // the rest of the DOMWindowProperties.
-    resetNotifications();
-#endif
 #if ENABLE(BLOB)
     m_domURL = 0;
 #endif
@@ -780,13 +760,6 @@
     return m_localStorage.get();
 }
 
-#if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)
-void DOMWindow::resetNotifications()
-{
-    DOMWindowNotifications::reset(this);
-}
-#endif
-
 void DOMWindow::postMessage(PassRefPtr<SerializedScriptValue> message, MessagePort* port, const String& targetOrigin, DOMWindow* source, ExceptionCode& ec)
 {
     MessagePortArray ports;

Modified: trunk/Source/WebCore/page/DOMWindow.h (111835 => 111836)


--- trunk/Source/WebCore/page/DOMWindow.h	2012-03-23 07:25:25 UTC (rev 111835)
+++ trunk/Source/WebCore/page/DOMWindow.h	2012-03-23 08:04:25 UTC (rev 111836)
@@ -355,10 +355,6 @@
         Storage* sessionStorage(ExceptionCode&) const;
         Storage* localStorage(ExceptionCode&) const;
 
-#if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)
-        void resetNotifications();
-#endif
-
 #if ENABLE(QUOTA)
         StorageInfo* webkitStorageInfo() const;
 #endif

Modified: trunk/Source/WebCore/page/Frame.cpp (111835 => 111836)


--- trunk/Source/WebCore/page/Frame.cpp	2012-03-23 07:25:25 UTC (rev 111835)
+++ trunk/Source/WebCore/page/Frame.cpp	2012-03-23 08:04:25 UTC (rev 111836)
@@ -687,11 +687,6 @@
     if (Frame* parent = tree()->parent())
         parent->loader()->checkLoadComplete();
 
-#if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)
-    if (m_domWindow)
-        m_domWindow->resetNotifications();
-#endif
-
     HashSet<FrameDestructionObserver*>::iterator stop = m_destructionObservers.end();
     for (HashSet<FrameDestructionObserver*>::iterator it = m_destructionObservers.begin(); it != stop; ++it)
         (*it)->willDetachPage();
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to