2008-05-30 Maciej Stachowiak <mjs@apple.com>
authormjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 May 2008 08:15:31 +0000 (08:15 +0000)
committermjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 May 2008 08:15:31 +0000 (08:15 +0000)
        Reviewed by Alexey.

        - speculative fix for "REGRESSION(r34143?): Frequent crash while browsing"
        https://bugs.webkit.org/show_bug.cgi?id=19285

        I'm pretty sure this fixes it but I have not been able to
        reproduce and am unsure if my theory of the bug is right.

        I belive the bug was because JSDOMWindowBase accessed
        JSDOMWindowShell in its destructor to remove itself from a
        hashtable, but GC destructor order is not guaranteed, so the
        hashtable may have been freed already. This patch changes things
        so that a non-GC object (the KJSProxy) does the tracking of live
        window objects for a frame. JSDOMWindowBase can null check the frame
        pointer to verify if it is still good.

        * bindings/js/JSDOMWindowBase.cpp:
        (WebCore::JSDOMWindowBase::~JSDOMWindowBase):
        * bindings/js/JSDOMWindowShell.cpp:
        (WebCore::JSDOMWindowShell::JSDOMWindowShell):
        * bindings/js/JSDOMWindowShell.h:
        (WebCore::JSDOMWindowShell::setWindow):
        * bindings/js/kjs_proxy.cpp:
        (WebCore::KJSProxy::clear):
        (WebCore::KJSProxy::initScript):
        (WebCore::KJSProxy::updateDocument):
        * bindings/js/kjs_proxy.h:
        (WebCore::KJSProxy::clearFormerWindow):
        * page/Frame.cpp:
        (WebCore::Frame::setDocument):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@34257 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bindings/js/JSDOMWindowBase.cpp
WebCore/bindings/js/JSDOMWindowShell.cpp
WebCore/bindings/js/JSDOMWindowShell.h
WebCore/bindings/js/kjs_proxy.cpp
WebCore/bindings/js/kjs_proxy.h
WebCore/page/Frame.cpp

index e44e673b559a3c74bb77e54f8e62d92f90af9238..ea6143288c3a94d3ffc346beb1d634f49d4c9c01 100644 (file)
@@ -1,3 +1,36 @@
+2008-05-30  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Alexey.
+        
+        - speculative fix for "REGRESSION(r34143?): Frequent crash while browsing"
+        https://bugs.webkit.org/show_bug.cgi?id=19285
+
+        I'm pretty sure this fixes it but I have not been able to
+        reproduce and am unsure if my theory of the bug is right.
+
+        I belive the bug was because JSDOMWindowBase accessed
+        JSDOMWindowShell in its destructor to remove itself from a
+        hashtable, but GC destructor order is not guaranteed, so the
+        hashtable may have been freed already. This patch changes things
+        so that a non-GC object (the KJSProxy) does the tracking of live
+        window objects for a frame. JSDOMWindowBase can null check the frame
+        pointer to verify if it is still good.
+        
+        * bindings/js/JSDOMWindowBase.cpp:
+        (WebCore::JSDOMWindowBase::~JSDOMWindowBase):
+        * bindings/js/JSDOMWindowShell.cpp:
+        (WebCore::JSDOMWindowShell::JSDOMWindowShell):
+        * bindings/js/JSDOMWindowShell.h:
+        (WebCore::JSDOMWindowShell::setWindow):
+        * bindings/js/kjs_proxy.cpp:
+        (WebCore::KJSProxy::clear):
+        (WebCore::KJSProxy::initScript):
+        (WebCore::KJSProxy::updateDocument):
+        * bindings/js/kjs_proxy.h:
+        (WebCore::KJSProxy::clearFormerWindow):
+        * page/Frame.cpp:
+        (WebCore::Frame::setDocument):
+
 2008-05-29  Chris Fleizach  <cfleizach@apple.com>
 
         Reviewed by Darin Adler.
index 35ecfa6c70d66eeedbb7fbe14c270b4b5248e98b..d266d0199aef0d3bceadda83dd9f546d7fe6e028 100644 (file)
@@ -202,7 +202,8 @@ void JSDOMWindowBase::updateDocument()
 
 JSDOMWindowBase::~JSDOMWindowBase()
 {
-    d->m_shell->clearFormerWindow(asJSDOMWindow(this));
+    if (m_impl->frame())
+        m_impl->frame()->scriptProxy()->clearFormerWindow(asJSDOMWindow(this));
 
     clearAllTimeouts();
 
index 00220ba14e22af30a0f930ac6d0ea21416a53c09..7b5c31fb672a4b00faf5422999c41c2209076f8a 100644 (file)
@@ -44,7 +44,6 @@ const ClassInfo JSDOMWindowShell::s_info = { "JSDOMWindowShell", 0, 0, 0 };
 JSDOMWindowShell::JSDOMWindowShell(DOMWindow* domWindow)
     : Base(jsNull())
     , m_window(0)
-    , m_liveFormerWindows(new HashSet<JSDOMWindow*>)
 {
     m_window = new JSDOMWindow(domWindow, this);
     setPrototype(m_window->prototype());
@@ -145,14 +144,6 @@ void JSDOMWindowShell::clear()
 }
 
 
-void JSDOMWindowShell::updateDocument()
-{
-    m_window->updateDocument();
-    HashSet<JSDOMWindow*>::iterator end = m_liveFormerWindows->end();
-    for (HashSet<JSDOMWindow*>::iterator it = m_liveFormerWindows->begin(); it != end; ++it)
-        (*it)->updateDocument();
-}
-
 // ----
 // Conversion methods
 // ----
index 57c272ce792bbfb43c6bb021ebff5d5fc26413a7..398001006256528e24b0c9d46090b7a5b21ea906 100644 (file)
@@ -47,7 +47,6 @@ namespace WebCore {
         void setWindow(JSDOMWindow* window)
         {
             ASSERT_ARG(window, window);
-            m_liveFormerWindows->add(m_window);
             m_window = window;
             setPrototype(window->prototype());
         }
@@ -76,13 +75,9 @@ namespace WebCore {
         DOMWindow* impl() const;
         void disconnectFrame();
         void clear();
-        void updateDocument();
-
-        void clearFormerWindow(JSDOMWindow* window) { m_liveFormerWindows->remove(window); }
 
     private:
         JSDOMWindow* m_window;
-        HashSet<JSDOMWindow*>* m_liveFormerWindows;
     };
 
     KJS::JSValue* toJS(KJS::ExecState*, Frame*);
index bb11460b522ae2694ad7b087b6921bc6bd686862..ba098b133dc23d4919f8c16c6586cc0cf05ee3a4 100644 (file)
@@ -112,6 +112,7 @@ void KJSProxy::clear()
 
     JSLock lock;
     m_windowShell->window()->clear();
+    m_liveFormerWindows.add(m_windowShell->window());
     m_windowShell->setWindow(new JSDOMWindow(m_frame->domWindow(), m_windowShell));
     if (Page* page = m_frame->page()) {
         attachDebugger(page->debugger());
@@ -155,7 +156,7 @@ void KJSProxy::initScript()
     JSLock lock;
 
     m_windowShell = new JSDOMWindowShell(m_frame->domWindow());
-    m_windowShell->updateDocument();
+    updateDocument();
 
     if (Page* page = m_frame->page()) {
         attachDebugger(page->debugger());
@@ -210,4 +211,15 @@ void KJSProxy::attachDebugger(KJS::Debugger* debugger)
         currentDebugger->detach(m_windowShell->window());
 }
 
+void KJSProxy::updateDocument()
+{
+    JSLock lock;
+    if (m_windowShell)
+        m_windowShell->window()->updateDocument();
+    HashSet<JSDOMWindow*>::iterator end = m_liveFormerWindows.end();
+    for (HashSet<JSDOMWindow*>::iterator it = m_liveFormerWindows.begin(); it != end; ++it)
+        (*it)->updateDocument();
+}
+
+
 } // namespace WebCore
index b74b196036247a43903f3082898f7a5bbab656ac..732a4f2f5818ece0df19ff930985d77e31573386 100644 (file)
@@ -77,6 +77,9 @@ public:
     void setPaused(bool b) { m_paused = b; }
     bool isPaused() const { return m_paused; }
 
+    void clearFormerWindow(JSDOMWindow* window) { m_liveFormerWindows.remove(window); }
+    void updateDocument();
+
 private:
     void initScriptIfNeeded()
     {
@@ -86,6 +89,7 @@ private:
     void initScript();
 
     KJS::ProtectedPtr<JSDOMWindowShell> m_windowShell;
+    HashSet<JSDOMWindow*> m_liveFormerWindows;
     Frame* m_frame;
     int m_handlerLineno;
     
index 08aa942037b39a21eccb6edd385622818862cb60..c74f84262fd6c45f4c525a104221b5a0d517d5b8 100644 (file)
@@ -260,10 +260,7 @@ void Frame::setDocument(PassRefPtr<Document> newDoc)
         d->m_doc->attach();
 
     // Update the cached 'document' property, which is now stale.
-    if (d->m_doc && d->m_jscript.haveWindowShell()) {
-        JSLock lock;
-        d->m_jscript.windowShell()->updateDocument();
-    }
+    d->m_jscript.updateDocument();
 }
 
 Settings* Frame::settings() const