[Qt] Crash in WebCore::CachedFrame::destroy

https://bugs.webkit.org/show_bug.cgi?id=104525

Reviewed by Adam Barth.

Source/WebCore:

Add an assert to increase the chances of catching this crash
early on in the future.

* dom/Document.cpp:
(WebCore::Document::takeDOMWindowFrom):

Source/WebKit/qt:

Remove the call to HistoryController::setCurrentItem which is ultimately
causing the initial empty document of a page to be added to the page cache.

This re-introduce the bug that was fixed by this line, which will be
properly fixed in a follow-up patch.

* Api/qwebhistory.cpp:
(operator>>):
* tests/qwebhistory/tst_qwebhistory.cpp:
(tst_QWebHistory::saveAndRestore_crash_4): Cover the crash.

Change-Id: Ia96ba570231557f8b53bc586410d50875880e1d1
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@139876 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Reviewed-by: Jocelyn Turcotte <jocelyn.turcotte@digia.com>

5 files changed, 57 insertions, 5 deletions

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index e8af73ebf..3157b7a39 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2013-01-16  Jocelyn Turcotte  <jocelyn.turcotte@digia.com>
+
+        [Qt] Crash in WebCore::CachedFrame::destroy
+        https://bugs.webkit.org/show_bug.cgi?id=104525
+
+        Reviewed by Adam Barth.
+
+        Add an assert to increase the chances of catching this crash
+        early on in the future.
+
+        * dom/Document.cpp:
+        (WebCore::Document::takeDOMWindowFrom):
+
 2012-12-12  Allan Sandfeld Jensen  <allan.jensen@digia.com>
 
         [Qt] Animation fails on large layers
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index 0bb8cc019..70e271c8c 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -3612,6 +3612,8 @@ void Document::takeDOMWindowFrom(Document* document)
     ASSERT(m_frame);
     ASSERT(!m_domWindow);
     ASSERT(document->domWindow());
+    // A valid DOMWindow is needed by CachedFrame for its documents.
+    ASSERT(!document->inPageCache());
 
     m_domWindow = document->m_domWindow.release();
     m_domWindow->didSecureTransitionTo(this);
diff --git a/Source/WebKit/qt/Api/qwebhistory.cpp b/Source/WebKit/qt/Api/qwebhistory.cpp
index a945b592b..a02083307 100644
--- a/Source/WebKit/qt/Api/qwebhistory.cpp
+++ b/Source/WebKit/qt/Api/qwebhistory.cpp
@@ -542,8 +542,6 @@ QDataStream& operator>>(QDataStream& source, QWebHistory& history)
                 d->lst->addItem(item);
             }
             d->lst->removeItem(nullItem);
-            // Update the HistoryController.
-            static_cast<WebCore::BackForwardListImpl*>(history.d->lst)->page()->mainFrame()->loader()->history()->setCurrentItem(history.d->lst->entries()[currentIndex].get());
             history.goToItem(history.itemAt(currentIndex));
         }
     }
diff --git a/Source/WebKit/qt/ChangeLog b/Source/WebKit/qt/ChangeLog
index 77eea541b..291c43a24 100644
--- a/Source/WebKit/qt/ChangeLog
+++ b/Source/WebKit/qt/ChangeLog
@@ -1,3 +1,21 @@
+2013-01-16  Jocelyn Turcotte  <jocelyn.turcotte@digia.com>
+
+        [Qt] Crash in WebCore::CachedFrame::destroy
+        https://bugs.webkit.org/show_bug.cgi?id=104525
+
+        Reviewed by Adam Barth.
+
+        Remove the call to HistoryController::setCurrentItem which is ultimately
+        causing the initial empty document of a page to be added to the page cache.
+
+        This re-introduce the bug that was fixed by this line, which will be
+        properly fixed in a follow-up patch.
+
+        * Api/qwebhistory.cpp:
+        (operator>>):
+        * tests/qwebhistory/tst_qwebhistory.cpp:
+        (tst_QWebHistory::saveAndRestore_crash_4): Cover the crash.
+
 2013-01-07  Michael Brüning  <michael.bruning@digia.com>
 
         [Qt] Apply correct patch for the scrolling issue from bug 105014
diff --git a/Source/WebKit/qt/tests/qwebhistory/tst_qwebhistory.cpp b/Source/WebKit/qt/tests/qwebhistory/tst_qwebhistory.cpp
index c0d9d8ec2..06c292908 100644
--- a/Source/WebKit/qt/tests/qwebhistory/tst_qwebhistory.cpp
+++ b/Source/WebKit/qt/tests/qwebhistory/tst_qwebhistory.cpp
@@ -56,9 +56,12 @@ private Q_SLOTS:
     void serialize_1(); //QWebHistory countity
     void serialize_2(); //QWebHistory index
     void serialize_3(); //QWebHistoryItem
+    // Those tests shouldn't crash
     void saveAndRestore_crash_1();
     void saveAndRestore_crash_2();
     void saveAndRestore_crash_3();
+    void saveAndRestore_crash_4();
+
     void popPushState_data();
     void popPushState();
     void clear();
@@ -308,7 +311,6 @@ static void restoreHistory(QWebHistory* history, QByteArray* out)
     load >> *history;
 }
 
-/** The test shouldn't crash */
 void tst_QWebHistory::saveAndRestore_crash_1()
 {
     QByteArray buffer;
@@ -319,7 +321,6 @@ void tst_QWebHistory::saveAndRestore_crash_1()
     }
 }
 
-/** The test shouldn't crash */
 void tst_QWebHistory::saveAndRestore_crash_2()
 {
     QByteArray buffer;
@@ -333,7 +334,6 @@ void tst_QWebHistory::saveAndRestore_crash_2()
     delete page2;
 }
 
-/** The test shouldn't crash */
 void tst_QWebHistory::saveAndRestore_crash_3()
 {
     QByteArray buffer;
@@ -353,6 +353,27 @@ void tst_QWebHistory::saveAndRestore_crash_3()
     delete page2;
 }
 
+void tst_QWebHistory::saveAndRestore_crash_4()
+{
+    QByteArray buffer;
+    saveHistory(hist, &buffer);
+
+    QWebPage* page2 = new QWebPage(this);
+    // The initial crash was in PageCache.
+    page2->settings()->setMaximumPagesInCache(3);
+
+    // Load the history in a new page, waiting for the load to finish.
+    QEventLoop waitForLoadFinished;
+    QObject::connect(page2, SIGNAL(loadFinished(bool)), &waitForLoadFinished, SLOT(quit()), Qt::QueuedConnection);
+    QDataStream load(&buffer, QIODevice::ReadOnly);
+    load >> *page2->history();
+    waitForLoadFinished.exec();
+
+    delete page2;
+    // Give some time for the PageCache cleanup 0-timer to fire.
+    QTest::qWait(50);
+}
+
 void tst_QWebHistory::popPushState_data()
 {
     QTest::addColumn<QString>("script");