Fix many bugs by giving Windows one FrameView per page load

WebCore:

        Fixes to allow multiple FrameViews on Windows

        Reviewed by Hyatt.

        * page/FrameView.cpp:
        (WebCore::FrameView::FrameView): Added a new constructor that takes an
        IntSize to specify the FrameView's initial size.
        (WebCore::FrameView::scheduleRelayout): Added an assertion that our
        Document is not in the page cache.
        * page/FrameView.h:
        * platform/gtk/WidgetGtk.cpp:
        (WebCore::Widget::~Widget): Add a warm, fuzzy ASSERT.
        * platform/qt/WidgetQt.cpp:
        (WebCore::Widget::~Widget): Ditto.
        * rendering/RenderWidget.cpp:
        (WebCore::RenderWidget::setWidget): Make sure to remove any existing
        Widget from the Widget hierarchy before deleting it. One instance
        where this is needed is when setWidget is called during FrameView
        creation on Windows.

WebKit/win:

        Fix many bugs by giving Windows one FrameView per page load

        Bugs include:
            <rdar://5659200>
                Windows back/forward cache causes crashes in the layout tests
            <rdar://5659355>
            <http://bugs.webkit.org/show_bug.cgi?id=16808>
                REGRESSION: PLT broken on Windows due to back/forward cache
            <rdar://5663654>
            <http://bugs.webkit.org/show_bug.cgi?id=16607>
                Random crashes in FrameView::scheduleRelayout while surfing
                Thinkgeek

        On Windows until now we've only had one FrameView per Frame. Once the
        back/forward cache was turned on this started causing assertions to
        fail and crashes to occur due to a single FrameView being both in the
        back/forward cache (possibly multiple times!) and used by a live
        document. We now create a new FrameView for each page load, just as
        Mac does.

        This has the side-effect of plugging some of the world leaks seen at
        the end of the PLT.

        Reviewed by Hyatt.

        * WebFrame.cpp:
        (WebFrame::initWithWebFrameView): Don't create the FrameView right
        away -- it'll be created when the load is committed.
        (WebFrame::transitionToCommittedFromCachedPage): Match the Mac by no
        longer calling resetMultipleFormSubmissionProtection here.
        (WebFrame::transitionToCommittedForNewPage): Ported code from
        -[WebCoreFrameBridge
        createFrameViewWithNSView:marginWidth:marginHeight:],
        -[WebCoreFrameBridge installInFrame:], and moved code here from
        WebFrame::initWithWebFrameView and WebView::initWithFrame. WebCore
        takes care of resetMultipleFormSubmissionProtection, just like it does
        on the Mac.
        * WebView.cpp:
        (WebView::initWithFrame): Moved FrameView initialization code to
        WebFrame::transitionToCommittedForNewPage.


Canonical link: https://commits.webkit.org/23202@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@29369 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog

@@ -1,3 +1,25 @@
+2008-01-10  Adam Roben  <aroben@apple.com>
+
+        Fixes to allow multiple FrameViews on Windows
+
+        Reviewed by Hyatt.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::FrameView): Added a new constructor that takes an
+        IntSize to specify the FrameView's initial size.
+        (WebCore::FrameView::scheduleRelayout): Added an assertion that our
+        Document is not in the page cache.
+        * page/FrameView.h:
+        * platform/gtk/WidgetGtk.cpp:
+        (WebCore::Widget::~Widget): Add a warm, fuzzy ASSERT.
+        * platform/qt/WidgetQt.cpp:
+        (WebCore::Widget::~Widget): Ditto.
+        * rendering/RenderWidget.cpp:
+        (WebCore::RenderWidget::setWidget): Make sure to remove any existing
+        Widget from the Widget hierarchy before deleting it. One instance
+        where this is needed is when setWidget is called during FrameView
+        creation on Windows.
+
 2008-01-10  Alp Toker  <alp@atoker.com>
 
         Include math.h to get ceilf(). Part of the SVG font GTK+ build fix.

WebCore/page/FrameView.cpp

@@ -144,6 +144,18 @@ FrameView::FrameView(Frame* frame)
     show();
 }
 
+#if !PLATFORM(MAC)
+FrameView::FrameView(Frame* frame, const IntSize& initialSize)
+    : m_refCount(1)
+    , m_frame(frame)
+    , d(new FrameViewPrivate(this))
+{
+    init();
+    Widget::setFrameGeometry(IntRect(x(), y(), initialSize.width(), initialSize.height()));
+    show();
+}
+#endif
+
 FrameView::~FrameView()
 {
     if (d->postLayoutTasksTimer.isActive()) {
@@ -699,6 +711,7 @@ void FrameView::layoutTimerFired(Timer<FrameView>*)
 
 void FrameView::scheduleRelayout()
 {
+    ASSERT(!m_frame->document() || !m_frame->document()->inPageCache());
     ASSERT(m_frame->view() == this);
 
     if (d->layoutRoot) {

WebCore/page/FrameView.h

@@ -50,6 +50,13 @@ template <typename T> class Timer;
 class FrameView : public ScrollView {
 public:
     FrameView(Frame*);
+
+    // On the Mac, FrameViews always get their size from the underlying NSView,
+    // so passing in a size is nonsensical.
+#if !PLATFORM(MAC)
+    FrameView(Frame*, const IntSize& initialSize);
+#endif
+
     virtual ~FrameView();
 
     Frame* frame() const { return m_frame.get(); }

WebCore/platform/gtk/WidgetGtk.cpp

@@ -79,6 +79,7 @@ void Widget::setGtkWidget(GtkWidget* widget)
 
 Widget::~Widget()
 {
+    ASSERT(!parent());
     delete data;
 }
 

WebCore/platform/qt/WidgetQt.cpp

@@ -79,6 +79,7 @@ Widget::Widget()
 
 Widget::~Widget()
 {
+    Q_ASSERT(!parent());
     delete data;
     data = 0;
 }

WebCore/rendering/RenderWidget.cpp

@@ -125,6 +125,8 @@ void RenderWidget::setWidget(Widget* widget)
 {
     if (widget != m_widget) {
         if (m_widget) {
+            // removeFromParent is a no-op on Mac.
+            m_widget->removeFromParent();
             widgetRendererMap().remove(m_widget);
             deleteWidget();
         }

WebKit/win/ChangeLog

@@ -1,3 +1,46 @@
+2008-01-10  Adam Roben  <aroben@apple.com>
+
+        Fix many bugs by giving Windows one FrameView per page load
+
+        Bugs include:
+            <rdar://5659200>
+                Windows back/forward cache causes crashes in the layout tests
+            <rdar://5659355>
+            <http://bugs.webkit.org/show_bug.cgi?id=16808>
+                REGRESSION: PLT broken on Windows due to back/forward cache
+            <rdar://5663654>
+            <http://bugs.webkit.org/show_bug.cgi?id=16607>
+                Random crashes in FrameView::scheduleRelayout while surfing
+                Thinkgeek
+
+        On Windows until now we've only had one FrameView per Frame. Once the
+        back/forward cache was turned on this started causing assertions to
+        fail and crashes to occur due to a single FrameView being both in the
+        back/forward cache (possibly multiple times!) and used by a live
+        document. We now create a new FrameView for each page load, just as
+        Mac does.
+
+        This has the side-effect of plugging some of the world leaks seen at
+        the end of the PLT.
+
+        Reviewed by Hyatt.
+
+        * WebFrame.cpp:
+        (WebFrame::initWithWebFrameView): Don't create the FrameView right
+        away -- it'll be created when the load is committed.
+        (WebFrame::transitionToCommittedFromCachedPage): Match the Mac by no
+        longer calling resetMultipleFormSubmissionProtection here.
+        (WebFrame::transitionToCommittedForNewPage): Ported code from
+        -[WebCoreFrameBridge
+        createFrameViewWithNSView:marginWidth:marginHeight:],
+        -[WebCoreFrameBridge installInFrame:], and moved code here from
+        WebFrame::initWithWebFrameView and WebView::initWithFrame. WebCore
+        takes care of resetMultipleFormSubmissionProtection, just like it does
+        on the Mac.
+        * WebView.cpp:
+        (WebView::initWithFrame): Moved FrameView initialization code to
+        WebFrame::transitionToCommittedForNewPage.
+
 2008-01-09  Ada Chan  <adachan@apple.com>
 
         Fix crash that could happen if the key we are passing to the HashMap is 0 in WebView::interpretKeyEvent().

WebKit/win/WebFrame.cpp

@@ -1069,13 +1069,6 @@ void WebFrame::initWithWebFrameView(IWebFrameView* /*view*/, IWebView* webView,
     this->AddRef(); // We release this ref in frameLoaderDestroyed()
     Frame* frame = new Frame(page, ownerElement, this);
     d->frame = frame;
-
-    FrameView* frameView = new FrameView(frame);
-
-    frame->setView(frameView);
-    frameView->deref(); // FrameViews are created with a ref count of 1. Release this ref since we've assigned it to frame.
-
-    frameView->setContainingWindow(viewWindow);
 }
 
 Frame* WebFrame::impl()
@@ -1838,16 +1831,43 @@ void WebFrame::savePlatformDataToCachedPage(CachedPage* cachedPage)
 
 void WebFrame::transitionToCommittedFromCachedPage(CachedPage*)
 {
-    transitionToCommittedForNewPage();
 }
 
 void WebFrame::transitionToCommittedForNewPage()
 {
-    ASSERT(core(this));
+    Frame* frame = core(this);
+    ASSERT(frame);
+
+    Page* page = frame->page();
+    ASSERT(page);
+
+    bool isMainFrame = frame == page->mainFrame();
+
+    if (isMainFrame && frame->view())
+        frame->view()->detachFromWindow();
+
+    frame->setView(0);
+
+    FrameView* frameView;
+    if (isMainFrame) {
+        RECT rect;
+        d->webView->frameRect(&rect);
+        frameView = new FrameView(frame, IntRect(rect).size());
+    } else
+        frameView = new FrameView(frame);
+
+    frame->setView(frameView);
+    frameView->deref(); // FrameViews are created with a ref count of 1. Release this ref since we've assigned it to frame.
+
+    HWND viewWindow;
+    if (SUCCEEDED(d->webView->viewWindow(reinterpret_cast<OLE_HANDLE*>(&viewWindow))))
+        frameView->setContainingWindow(viewWindow);
+
+    if (isMainFrame)
+        frameView->attachToWindow();
 
-    // On the mac, this is done in Frame::setView, but since we don't have separate
-    // frame views, we'll just do it here instead.
-    core(this)->loader()->resetMultipleFormSubmissionProtection();
+    if (frame->ownerRenderer())
+        frame->ownerRenderer()->setWidget(frameView);
 }
 
 void WebFrame::updateGlobalHistoryForStandardLoad(const KURL& url)

WebKit/win/WebView.cpp

@@ -2060,8 +2060,6 @@ HRESULT STDMETHODCALLTYPE WebView::initWithFrame(
     webFrame->initWithWebFrameView(0 /*FIXME*/, this, m_page, 0);
     m_mainFrame = webFrame;
     webFrame->Release(); // The WebFrame is owned by the Frame, so release our reference to it.
-    m_page->mainFrame()->view()->attachToWindow();
-    m_page->mainFrame()->view()->resize(frame.right - frame.left, frame.bottom - frame.top);
 
     m_page->mainFrame()->tree()->setName(String(frameName, SysStringLen(frameName)));
     m_page->mainFrame()->init();