Reviewed by Darin.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Feb 2005 23:48:00 +0000 (23:48 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Feb 2005 23:48:00 +0000 (23:48 +0000)
<rdar://problem/3942428> reproducible crash loading cbs.sportsline.com

This change reverts the fix for <rdar://problem/3805311>, and
re-fixes it in a different (better) way. Instead of preventing
programmatic open from setting the parsing flag, instead make sure
that programmatic close resets it.

        * khtml/khtml_part.cpp:
        (KHTMLPart::openURL):
        (KHTMLPart::didExplicitOpen):
        (KHTMLPart::closeURL):
        (KHTMLPart::begin):
        (KHTMLPart::end):
        (KHTMLPart::endIfNotLoading):
        (KHTMLPart::slotFinishedParsing):
        (KHTMLPart::checkEmitLoadEvent):
        * khtml/khtml_part.h:
        * khtml/khtmlpart_p.h:
        (KHTMLPartPrivate::KHTMLPartPrivate):
        * khtml/xml/dom_docimpl.cpp:
        (DocumentImpl::open):
        (DocumentImpl::implicitOpen):
        (DocumentImpl::close):
        (DocumentImpl::implicitClose):
        * khtml/xml/dom_docimpl.h:

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/khtml_part.cpp
WebCore/khtml/khtml_part.h
WebCore/khtml/khtmlpart_p.h
WebCore/khtml/xml/dom_docimpl.cpp
WebCore/khtml/xml/dom_docimpl.h

index 61f1cab82e64231f2a5617710f0c7b70b876e3c5..929cd63057c881f24e9974e052b0efe7bc823601 100644 (file)
@@ -1,3 +1,33 @@
+2005-02-15  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+
+       <rdar://problem/3942428> reproducible crash loading cbs.sportsline.com
+       
+       This change reverts the fix for <rdar://problem/3805311>, and
+       re-fixes it in a different (better) way. Instead of preventing
+       programmatic open from setting the parsing flag, instead make sure
+       that programmatic close resets it.
+
+        * khtml/khtml_part.cpp:
+        (KHTMLPart::openURL):
+        (KHTMLPart::didExplicitOpen):
+        (KHTMLPart::closeURL):
+        (KHTMLPart::begin):
+        (KHTMLPart::end):
+        (KHTMLPart::endIfNotLoading):
+        (KHTMLPart::slotFinishedParsing):
+        (KHTMLPart::checkEmitLoadEvent):
+        * khtml/khtml_part.h:
+        * khtml/khtmlpart_p.h:
+        (KHTMLPartPrivate::KHTMLPartPrivate):
+        * khtml/xml/dom_docimpl.cpp:
+        (DocumentImpl::open):
+        (DocumentImpl::implicitOpen):
+        (DocumentImpl::close):
+        (DocumentImpl::implicitClose):
+        * khtml/xml/dom_docimpl.h:
+
 2005-02-15  David Harrison  <harrison@apple.com>
 
         Reviewed by Maciej.
index deb9d83703d82e5ed37b51e61617b9b9ff3b91d0..0414f5273689f173915d02bdc19031b36e8790cf 100644 (file)
@@ -525,6 +525,7 @@ bool KHTMLPart::openURL( const KURL &url )
            SLOT( slotRedirection(KIO::Job*,const KURL&) ) );
 
   d->m_bComplete = false;
+  d->m_bLoadingMainResource = true;
   d->m_bLoadEventEmitted = false;
 
   // delete old status bar msg's from kjs (if it _was_ activated on last URL)
@@ -569,6 +570,13 @@ bool KHTMLPart::openURL( const KURL &url )
   return true;
 }
 
+void KHTMLPart::didExplicitOpen()
+{
+  d->m_bComplete = false;
+  d->m_bLoadEventEmitted = false;
+}
+
+
 bool KHTMLPart::closeURL()
 {
   if ( d->m_job )
@@ -590,6 +598,7 @@ bool KHTMLPart::closeURL()
   }
 
   d->m_bComplete = true; // to avoid emitting completed() in slotFinishedParsing() (David)
+  d->m_bLoadingMainResource = false;
   d->m_bLoadEventEmitted = true; // don't want that one either
   d->m_cachePolicy = KIO::CC_Verify; // Why here?
 
@@ -1470,6 +1479,7 @@ void KHTMLPart::begin( const KURL &url, int xOffset, int yOffset )
   d->m_cacheId = 0;
   d->m_bComplete = false;
   d->m_bLoadEventEmitted = false;
+  d->m_bLoadingMainResource = true;
 
   if(url.isValid()) {
 #if APPLE_CHANGES
@@ -1571,8 +1581,7 @@ void KHTMLPart::begin( const KURL &url, int xOffset, int yOffset )
   d->m_doc->setRestoreState(args.docState);
 #endif
 
-  d->m_doc->openInternal();
-  d->m_doc->setParsing(true);
+  d->m_doc->implicitOpen();
   // clear widget
   if (d->m_view)
     d->m_view->resizeContents( 0, 0 );
@@ -1581,8 +1590,6 @@ void KHTMLPart::begin( const KURL &url, int xOffset, int yOffset )
 #if !APPLE_CHANGES
   emit d->m_extension->enableAction( "print", true );
 #endif
-
-  d->m_doc->setParsing(true);
 }
 
 void KHTMLPart::write( const char *str, int len )
@@ -1651,6 +1658,15 @@ void KHTMLPart::write( const QString &str )
 
 void KHTMLPart::end()
 {
+    d->m_bLoadingMainResource = false;
+    endIfNotLoading();
+}
+
+void KHTMLPart::endIfNotLoading()
+{
+    if (d->m_bLoadingMainResource)
+        return;
+
     // make sure nothing's left in there...
     if (d->m_decoder)
         write(d->m_decoder->flush());
@@ -1725,7 +1741,6 @@ void KHTMLPart::gotoAnchor()
 void KHTMLPart::slotFinishedParsing()
 {
   d->m_doc->setParsing(false);
-  disconnect(d->m_doc,SIGNAL(finishedParsing()),this,SLOT(slotFinishedParsing()));
 
   if (!d->m_view)
     return; // We are probably being destructed.
@@ -1942,7 +1957,7 @@ void KHTMLPart::checkEmitLoadEvent()
   d->m_bLoadEventEmitted = true;
   d->m_bUnloadEventEmitted = false;
   if (d->m_doc)
-    d->m_doc->close();
+    d->m_doc->implicitClose();
 }
 
 const KHTMLSettings *KHTMLPart::settings() const
index a98aa853105bef41e5cd048a2dba65902f0bb756..0515cafa014fb7974822bfb4172570627d1ae7ef 100644 (file)
@@ -214,6 +214,8 @@ public:
    */
   virtual bool openURL( const KURL &url );
 
+  void didExplicitOpen();
+
   /**
    * Stops loading the document and kill all data requests (for images, etc.)
    */
@@ -464,6 +466,8 @@ public:
    */
   virtual void end();
 
+  void endIfNotLoading();
+
   /**
    * Similar to end, but called to abort a load rather than cleanly end.
    */
index 5b22055f0e8e8c927ab6ff75c14a72770d101182..66cbaece8a5fdbd251369739584624d04fa8d636 100644 (file)
@@ -119,6 +119,7 @@ public:
     m_kjs_lib = 0;
     m_job = 0L;
     m_bComplete = true;
+    m_bLoadingMainResource = false;
     m_bLoadEventEmitted = true;
     m_bUnloadEventEmitted = true;
     m_cachePolicy = KIO::CC_Verify;
@@ -289,6 +290,7 @@ public:
 #endif
 
   bool m_bComplete:1;
+  bool m_bLoadingMainResource:1;
   bool m_bLoadEventEmitted:1;
   bool m_bUnloadEventEmitted:1;
   bool m_haveEncoding:1;
index 446be85eb2383f01c0f3b771eb91cb3ea1360100..66336aa2c7fdf72c3d97354b960d4731a057a3b8 100644 (file)
@@ -1331,16 +1331,20 @@ void DocumentImpl::open(  )
 {
     if (parsing()) return;
 
-    openInternal();
+    implicitOpen();
+
+    if (part()) {
+        part()->didExplicitOpen();
+    }
 
     // This is work that we should probably do in clear(), but we can't have it
-    // happen when openInternal() is called unless we reorganize KHTMLPart code.
+    // happen when implicitOpen() is called unless we reorganize KHTMLPart code.
     setURL(QString());
     DocumentImpl *parent = parentDocument();
     setBaseURL(parent ? parent->baseURL() : QString());
 }
 
-void DocumentImpl::openInternal()
+void DocumentImpl::implicitOpen()
 {
     if (m_tokenizer)
         close();
@@ -1348,6 +1352,7 @@ void DocumentImpl::openInternal()
     clear();
     m_tokenizer = createTokenizer();
     connect(m_tokenizer,SIGNAL(finishedParsing()),this,SIGNAL(finishedParsing()));
+    setParsing(true);
 
     if (m_view && m_view->part()->jScript()) {
         m_view->part()->jScript()->setSourceFile(m_url,""); //fixme
@@ -1373,6 +1378,13 @@ HTMLElementImpl* DocumentImpl::body()
 }
 
 void DocumentImpl::close()
+{
+    if (part())
+        part()->endIfNotLoading();
+    implicitClose();
+}
+
+void DocumentImpl::implicitClose()
 {
     // First fire the onload.
     
@@ -1430,9 +1442,21 @@ void DocumentImpl::close()
        return;
     }
     
-    // The initial layout happens here.
-    DocumentImpl::closeInternal(!doload);
-    
+    if (doload) {
+        // on an explicit document.close(), the tokenizer might still be waiting on scripts,
+        // and in that case we don't want to destroy it because that will prevent the
+        // scripts from getting processed.
+        // FIXME: this check may no longer be necessary, since now it should be impossible
+        // for parsing to be false while stil waiting for scripts
+        if (m_tokenizer && !m_tokenizer->isWaitingForScripts()) {
+            delete m_tokenizer;
+            m_tokenizer = 0;
+        }
+
+        if (m_view)
+            m_view->part()->checkEmitLoadEvent();
+    }
+
     // Now do our painting/layout, but only if we aren't in a subframe or if we're in a subframe
     // that has been sized already.  Otherwise, our view size would be incorrect, so doing any 
     // layout/painting now would be pointless.
@@ -1451,22 +1475,6 @@ void DocumentImpl::close()
     }
 }
 
-void DocumentImpl::closeInternal( bool checkTokenizer )
-{
-    if (parsing() || (checkTokenizer && !m_tokenizer)) return;
-
-    // on an explicit document.close(), the tokenizer might still be waiting on scripts,
-    // and in that case we don't want to destroy it because that will prevent the
-    // scripts from getting processed.
-    if (m_tokenizer && !m_tokenizer->isWaitingForScripts()) {
-       delete m_tokenizer;
-       m_tokenizer = 0;
-    }
-
-    if (m_view)
-        m_view->part()->checkEmitLoadEvent();
-}
-
 void DocumentImpl::setParsing(bool b)
 {
     m_bParsing = b;
index f3d2b17fda31fd95ec651d283f3d121af66f347e..c5ab23c4a5a677be8d0cfc876e9f80f91bfe938a 100644 (file)
@@ -302,9 +302,9 @@ public:
     void updateSelection();
     
     void open();
-    void openInternal();
+    void implicitOpen();
     void close();
-    void closeInternal ( bool checkTokenizer );
+    void implicitClose();
     void write ( const DOMString &text );
     void write ( const QString &text );
     void writeln ( const DOMString &text );