Changeset 29369 in webkit

Timestamp:

Jan 10, 2008 1:23:34 PM (16 years ago)

Author:

Adam Roben

Message:

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.

Location:

trunk

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/FrameView.cpp (modified) (2 diffs)
• WebCore/page/FrameView.h (modified) (1 diff)
• WebCore/platform/gtk/WidgetGtk.cpp (modified) (1 diff)
• WebCore/platform/qt/WidgetQt.cpp (modified) (1 diff)
• WebCore/rendering/RenderWidget.cpp (modified) (1 diff)
• WebKit/win/ChangeLog (modified) (1 diff)
• WebKit/win/WebFrame.cpp (modified) (2 diffs)
• WebKit/win/WebView.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/WebCore/page/FrameView.cpp

@@ -144 +144 @@
     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()
@@ -700 +712 @@
 void FrameView::scheduleRelayout()
 {
+    ASSERT(!m_frame->document() || !m_frame->document()->inPageCache());
     ASSERT(m_frame->view() == this);
 

trunk/WebCore/page/FrameView.h

@@ -51 +51 @@
 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();
 

trunk/WebCore/platform/gtk/WidgetGtk.cpp

@@ -80 +80 @@
 Widget::~Widget()
 {
+    ASSERT(!parent());
     delete data;
 }

trunk/WebCore/platform/qt/WidgetQt.cpp

@@ -80 +80 @@
 Widget::~Widget()
 {
+    Q_ASSERT(!parent());
     delete data;
     data = 0;

trunk/WebCore/rendering/RenderWidget.cpp

@@ -126 +126 @@
     if (widget != m_widget) {
         if (m_widget) {
+            // removeFromParent is a no-op on Mac.
+            m_widget->removeFromParent();
             widgetRendererMap().remove(m_widget);
             deleteWidget();

trunk/WebKit/win/ChangeLog

@@ +1 @@
+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>
 

trunk/WebKit/win/WebFrame.cpp

@@ -1070 +1070 @@
     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);
 }
 
@@ -1839 +1832 @@
 void WebFrame::transitionToCommittedFromCachedPage(CachedPage*)
 {
-    transitionToCommittedForNewPage();
 }
 
 void WebFrame::transitionToCommittedForNewPage()
 {
-    ASSERT(core(this));
-
-    // 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();
+    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();
+
+    if (frame->ownerRenderer())
+        frame->ownerRenderer()->setWidget(frameView);
 }
 

trunk/WebKit/win/WebView.cpp

@@ -2061 +2061 @@
     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)));