ScrollableAreaSet should be moved from Page to FrameView
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Feb 2012 20:45:52 +0000 (20:45 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Feb 2012 20:45:52 +0000 (20:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=62762

Reviewed by Beth Dakin.

Source/WebCore:

It makes more sense for the set of scrollable areas to be per frame view instead of per page;
scrollable areas are associated with a containing frame view and their lifecycle follows the lifecycle of the
frame view much more closely. This could even fix a bunch of crashes where a scrollable area outlived its containing page.

* WebCore.exp.in:
Replace the Page member functions with FrameView member functions instead.

* page/EventHandler.cpp:
(WebCore::EventHandler::mouseMoved):
Check if the frame view contains the given layer.

(WebCore::EventHandler::updateMouseEventTargetNode):
Ditto.

* page/FocusController.cpp:
(WebCore::contentAreaDidShowOrHide):
Add helper function.

(WebCore::FocusController::setContainingWindowIsVisible):
Call contentAreaDidShowOrHide for the main frame view, and for all scrollable areas
inside all subframe views.

* page/FrameView.cpp:
(WebCore::FrameView::FrameView):
Use early returns to make the code more clear. Also, don't add the scrollable area to the set here.

(WebCore::FrameView::~FrameView):
Don't remove the scrollable area here.

(WebCore::FrameView::zoomAnimatorTransformChanged):
m_page is gone so use m_frame->page() instead.

(WebCore::FrameView::setAnimatorsAreActive):
Call ScrollAnimator::setIsActive for all the scrollable areas in this frame view. Previously we used to do
this for all scrollable areas on the page, but since setAnimatorsAreActive will be called for each document,
this will be done implicitly.

(WebCore::FrameView::notifyPageThatContentAreaWillPaint):
Call ScrollableArea::contentDidPaint for this frame view and all its immediate scrollable areas. Previously, we used
to do this for all scrollable areas on the page, but we only need to do it for this frame view.

(WebCore::FrameView::scrollAnimatorEnabled):
Get the page from m_frame since m_page is gone.

(WebCore::FrameView::addScrollableArea):
(WebCore::FrameView::removeScrollableArea):
(WebCore::FrameView::containsScrollableArea):
Move these member functions here from Page.

(WebCore::FrameView::addChild):
If we are adding a frame view, add it to the scrollable area set.

(WebCore::FrameView::removeChild):
If we are removing a frame view, remove it from the scrollable area set.

* page/FrameView.h:
Move the member function declarations and the scrollable area set member variable here from Page.

* page/Page.cpp:
(WebCore::Page::~Page):
Don't call disconnectPage on the scrollable areas anymore.

* platform/ScrollView.h:
(ScrollView):
Make addChild and removeChild virtual.

* platform/ScrollableArea.h:
Remove disconnectFromPage.

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::RenderLayer):
(WebCore::RenderLayer::~RenderLayer):
(WebCore::RenderLayer::styleChanged):
The frame view now keeps track of the scrollable areas.

* rendering/RenderLayer.h:
Remove the page member variable and disconnectFromPage.

* rendering/RenderListBox.cpp:
(WebCore::RenderListBox::RenderListBox):
(WebCore::RenderListBox::~RenderListBox):
The frame view now keeps track of the scrollable areas.

* rendering/RenderListBox.h:
Remove the page member variable and disconnectFromPage.

Source/WebKit/chromium:

Update for changes to WebCore now that the scrollable area set is kept per frame view.

* src/ScrollbarGroup.cpp:
(WebKit::ScrollbarGroup::ScrollbarGroup):
(WebKit::ScrollbarGroup::~ScrollbarGroup):
* src/ScrollbarGroup.h:
(WebCore):
(ScrollbarGroup):
* src/WebPluginContainerImpl.cpp:
(WebKit::WebPluginContainerImpl::scrollbarGroup):

Source/WebKit2:

* WebProcess/Plugins/PDF/BuiltInPDFView.cpp:
(WebKit::BuiltInPDFView::initialize):
Call FrameView::addScrollableArea instead.

(WebKit::BuiltInPDFView::destroy):
Call FrameView::removeScrollableArea instead.

* WebProcess/Plugins/PDF/BuiltInPDFView.h:
Remove disconnectFromPage since it no longer exists on ScrollableArea.

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

21 files changed:
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/page/EventHandler.cpp
Source/WebCore/page/FocusController.cpp
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/page/Page.cpp
Source/WebCore/page/Page.h
Source/WebCore/platform/ScrollView.h
Source/WebCore/platform/ScrollableArea.h
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/rendering/RenderListBox.cpp
Source/WebCore/rendering/RenderListBox.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/src/ScrollbarGroup.cpp
Source/WebKit/chromium/src/ScrollbarGroup.h
Source/WebKit/chromium/src/WebPluginContainerImpl.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp
Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.h

index 3f1615e..70dad7e 100644 (file)
@@ -1,3 +1,96 @@
+2012-02-06  Anders Carlsson  <andersca@apple.com>
+
+        ScrollableAreaSet should be moved from Page to FrameView
+        https://bugs.webkit.org/show_bug.cgi?id=62762
+
+        Reviewed by Beth Dakin.
+
+        It makes more sense for the set of scrollable areas to be per frame view instead of per page;
+        scrollable areas are associated with a containing frame view and their lifecycle follows the lifecycle of the
+        frame view much more closely. This could even fix a bunch of crashes where a scrollable area outlived its containing page.
+
+        * WebCore.exp.in:
+        Replace the Page member functions with FrameView member functions instead.
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::mouseMoved):
+        Check if the frame view contains the given layer.
+
+        (WebCore::EventHandler::updateMouseEventTargetNode):
+        Ditto.
+
+        * page/FocusController.cpp:
+        (WebCore::contentAreaDidShowOrHide):
+        Add helper function.
+
+        (WebCore::FocusController::setContainingWindowIsVisible):
+        Call contentAreaDidShowOrHide for the main frame view, and for all scrollable areas
+        inside all subframe views.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::FrameView):
+        Use early returns to make the code more clear. Also, don't add the scrollable area to the set here.
+
+        (WebCore::FrameView::~FrameView):
+        Don't remove the scrollable area here.
+
+        (WebCore::FrameView::zoomAnimatorTransformChanged):
+        m_page is gone so use m_frame->page() instead.
+
+        (WebCore::FrameView::setAnimatorsAreActive):
+        Call ScrollAnimator::setIsActive for all the scrollable areas in this frame view. Previously we used to do
+        this for all scrollable areas on the page, but since setAnimatorsAreActive will be called for each document,
+        this will be done implicitly.
+
+        (WebCore::FrameView::notifyPageThatContentAreaWillPaint):
+        Call ScrollableArea::contentDidPaint for this frame view and all its immediate scrollable areas. Previously, we used
+        to do this for all scrollable areas on the page, but we only need to do it for this frame view.
+
+        (WebCore::FrameView::scrollAnimatorEnabled):
+        Get the page from m_frame since m_page is gone.
+
+        (WebCore::FrameView::addScrollableArea):
+        (WebCore::FrameView::removeScrollableArea):
+        (WebCore::FrameView::containsScrollableArea):
+        Move these member functions here from Page.
+
+        (WebCore::FrameView::addChild):
+        If we are adding a frame view, add it to the scrollable area set.
+
+        (WebCore::FrameView::removeChild):
+        If we are removing a frame view, remove it from the scrollable area set.
+
+        * page/FrameView.h:
+        Move the member function declarations and the scrollable area set member variable here from Page.
+
+        * page/Page.cpp:
+        (WebCore::Page::~Page):
+        Don't call disconnectPage on the scrollable areas anymore.
+
+        * platform/ScrollView.h:
+        (ScrollView):
+        Make addChild and removeChild virtual.
+
+        * platform/ScrollableArea.h:
+        Remove disconnectFromPage.
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::RenderLayer):
+        (WebCore::RenderLayer::~RenderLayer):
+        (WebCore::RenderLayer::styleChanged):
+        The frame view now keeps track of the scrollable areas.
+
+        * rendering/RenderLayer.h:
+        Remove the page member variable and disconnectFromPage.
+
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::RenderListBox):
+        (WebCore::RenderListBox::~RenderListBox):
+        The frame view now keeps track of the scrollable areas.
+
+        * rendering/RenderListBox.h:
+        Remove the page member variable and disconnectFromPage.
+
 2012-02-07  Matthew Delaney  <mdelaney@apple.com>
 
         Remove redundant checks in CanvasRenderingContext2D.cpp
index 5e0d197..ec27aec 100644 (file)
@@ -764,12 +764,10 @@ __ZN7WebCore4Page15addSchedulePairEN3WTF10PassRefPtrINS_12SchedulePairEEE
 __ZN7WebCore4Page15didMoveOnscreenEv
 __ZN7WebCore4Page16setCanStartMediaEb
 __ZN7WebCore4Page16setDefersLoadingEb
-__ZN7WebCore4Page17addScrollableAreaEPNS_14ScrollableAreaE
 __ZN7WebCore4Page17willMoveOffscreenEv
 __ZN7WebCore4Page18removeSchedulePairEN3WTF10PassRefPtrINS_12SchedulePairEEE
 __ZN7WebCore4Page18setPageScaleFactorEfRKNS_8IntPointE
 __ZN7WebCore4Page19visitedStateChangedEPNS_9PageGroupEy
-__ZN7WebCore4Page20removeScrollableAreaEPNS_14ScrollableAreaE
 __ZN7WebCore4Page20setDeviceScaleFactorEf
 __ZN7WebCore4Page20unmarkAllTextMatchesEv
 __ZN7WebCore4Page21markAllMatchesForTextERKN3WTF6StringEjbj
@@ -1046,12 +1044,14 @@ __ZN7WebCore9FrameView14setNeedsLayoutEv
 __ZN7WebCore9FrameView14setTransparentEb
 __ZN7WebCore9FrameView15setMarginHeightEi
 __ZN7WebCore9FrameView16setPaintBehaviorEj
+__ZN7WebCore9FrameView17addScrollableAreaEPNS_14ScrollableAreaE
 __ZN7WebCore9FrameView17paintControlTintsEv
 __ZN7WebCore9FrameView17setScrollPositionERKNS_8IntPointE
 __ZN7WebCore9FrameView17setTracksRepaintsEb
 __ZN7WebCore9FrameView18updateControlTintsEv
 __ZN7WebCore9FrameView19scrollElementToRectEPNS_7ElementERKNS_7IntRectE
 __ZN7WebCore9FrameView20enterCompositingModeEv
+__ZN7WebCore9FrameView20removeScrollableAreaEPNS_14ScrollableAreaE
 __ZN7WebCore9FrameView21flushDeferredRepaintsEv
 __ZN7WebCore9FrameView22setBaseBackgroundColorERKNS_5ColorE
 __ZN7WebCore9FrameView23updateCanHaveScrollbarsEv
index 088b05c..e30b42d 100644 (file)
@@ -1656,8 +1656,10 @@ bool EventHandler::mouseMoved(const PlatformMouseEvent& event)
         return result;
 
     if (RenderLayer* layer = layerForNode(hoveredNode.innerNode())) {
-        if (page->containsScrollableArea(layer))
-            layer->mouseMovedInContentArea();
+        if (FrameView* frameView = m_frame->view()) {
+            if (frameView->containsScrollableArea(layer))
+                layer->mouseMovedInContentArea();
+        }
     }
 
     if (FrameView* frameView = m_frame->view())
@@ -2117,10 +2119,14 @@ void EventHandler::updateMouseEventTargetNode(Node* targetNode, const PlatformMo
             }
         } else if (page && (layerForLastNode && (!layerForNodeUnderMouse || layerForNodeUnderMouse != layerForLastNode))) {
             // The mouse has moved between layers.
-            if (page->containsScrollableArea(layerForLastNode))
-                layerForLastNode->mouseExitedContentArea();
+            if (Frame* frame = m_lastNodeUnderMouse->document()->frame()) {
+                if (FrameView* frameView = frame->view()) {
+                    if (frameView->containsScrollableArea(layerForLastNode))
+                        layerForLastNode->mouseExitedContentArea();
+                }
+            }
         }
-        
+
         if (m_nodeUnderMouse && (!m_lastNodeUnderMouse || m_lastNodeUnderMouse->document() != m_frame->document())) {
             // The mouse has moved between frames.
             if (Frame* frame = m_nodeUnderMouse->document()->frame()) {
@@ -2129,10 +2135,14 @@ void EventHandler::updateMouseEventTargetNode(Node* targetNode, const PlatformMo
             }
         } else if (page && (layerForNodeUnderMouse && (!layerForLastNode || layerForNodeUnderMouse != layerForLastNode))) {
             // The mouse has moved between layers.
-            if (page->containsScrollableArea(layerForNodeUnderMouse))
-                layerForNodeUnderMouse->mouseEnteredContentArea();
+            if (Frame* frame = m_nodeUnderMouse->document()->frame()) {
+                if (FrameView* frameView = frame->view()) {
+                    if (frameView->containsScrollableArea(layerForNodeUnderMouse))
+                        layerForNodeUnderMouse->mouseEnteredContentArea();
+                }
+            }
         }
-        
+
         if (m_lastNodeUnderMouse && m_lastNodeUnderMouse->document() != m_frame->document()) {
             m_lastNodeUnderMouse = 0;
             m_lastScrollbarUnderMouse = 0;
index c2c03e5..51426a3 100644 (file)
@@ -582,6 +582,14 @@ void FocusController::setActive(bool active)
         dispatchEventsOnWindowAndFocusedNode(m_focusedFrame->document(), active);
 }
 
+static void contentAreaDidShowOrHide(ScrollableArea* scrollableArea, bool didShow)
+{
+    if (didShow)
+        scrollableArea->contentAreaDidShow();
+    else
+        scrollableArea->contentAreaDidHide();
+}
+
 void FocusController::setContainingWindowIsVisible(bool containingWindowIsVisible)
 {
     if (m_containingWindowIsVisible == containingWindowIsVisible)
@@ -593,13 +601,22 @@ void FocusController::setContainingWindowIsVisible(bool containingWindowIsVisibl
     if (!view)
         return;
 
-    if (const HashSet<ScrollableArea*>* scrollableAreas = m_page->scrollableAreaSet()) {
-        HashSet<ScrollableArea*>::const_iterator end = scrollableAreas->end(); 
-        for (HashSet<ScrollableArea*>::const_iterator it = scrollableAreas->begin(); it != end; ++it) {
-            if (!containingWindowIsVisible)
-                (*it)->contentAreaDidHide();
-            else
-                (*it)->contentAreaDidShow();
+    contentAreaDidShowOrHide(view, containingWindowIsVisible);
+
+    for (Frame* frame = m_page->mainFrame(); frame; frame = frame->tree()->traverseNext()) {
+        FrameView* frameView = frame->view();
+        if (!frameView)
+            continue;
+
+        const HashSet<ScrollableArea*>* scrollableAreas = frameView->scrollableAreas();
+        if (!scrollableAreas)
+            continue;
+
+        for (HashSet<ScrollableArea*>::const_iterator it = scrollableAreas->begin(), end = scrollableAreas->end(); it != end; ++it) {
+            ScrollableArea* scrollableArea = *it;
+            ASSERT(scrollableArea->isOnActivePage());
+
+            contentAreaDidShowOrHide(scrollableArea, containingWindowIsVisible);
         }
     }
 }
index 89e6286..d0a10c8 100644 (file)
@@ -152,16 +152,17 @@ FrameView::FrameView(Frame* frame)
 {
     init();
 
-    if (m_frame) {
-        if (Page* page = m_frame->page()) {
-            m_page = page;
-            m_page->addScrollableArea(this);
+    // FIXME: Can m_frame ever be null here?
+    if (!m_frame)
+        return;
 
-            if (m_frame == m_page->mainFrame()) {
-                ScrollableArea::setVerticalScrollElasticity(ScrollElasticityAllowed);
-                ScrollableArea::setHorizontalScrollElasticity(ScrollElasticityAllowed);
-            }
-        }
+    Page* page = m_frame->page();
+    if (!page)
+        return;
+
+    if (m_frame == page->mainFrame()) {
+        ScrollableArea::setVerticalScrollElasticity(ScrollElasticityAllowed);
+        ScrollableArea::setHorizontalScrollElasticity(ScrollElasticityAllowed);
     }
 }
 
@@ -202,9 +203,6 @@ FrameView::~FrameView()
     ASSERT(!m_scrollCorner);
     ASSERT(m_actionScheduler->isEmpty());
 
-    if (m_page)
-        m_page->removeScrollableArea(this);
-
     if (m_frame) {
         ASSERT(m_frame->view() != this || !m_frame->contentRenderer());
         RenderPart* renderer = m_frame->ownerRenderer();
@@ -1242,8 +1240,8 @@ void FrameView::removeWidgetToUpdate(RenderEmbeddedObject* object)
 void FrameView::zoomAnimatorTransformChanged(float scale, float x, float y, ZoomAnimationState state)
 {
     if (state == ZoomAnimationFinishing) {
-        m_page->setPageScaleFactor(m_page->pageScaleFactor() * scale,
-                                   IntPoint(scale * scrollX() - x, scale * scrollY() - y));
+        if (Page* page = m_frame->page())
+            page->setPageScaleFactor(page->pageScaleFactor() * scale, IntPoint(scale * scrollX() - x, scale * scrollY() - y));
         scrollAnimator()->resetZoom();
     }
 
@@ -2575,18 +2573,16 @@ void FrameView::setAnimatorsAreActive()
     if (!page)
         return;
 
-    const HashSet<ScrollableArea*>* scrollableAreas = page->scrollableAreaSet();
-    if (!scrollableAreas)
+    scrollAnimator()->setIsActive();
+
+    if (!m_scrollableAreas)
         return;
 
-    HashSet<ScrollableArea*>::const_iterator end = scrollableAreas->end(); 
-    for (HashSet<ScrollableArea*>::const_iterator it = scrollableAreas->begin(); it != end; ++it) {
-        // FIXME: This extra check to make sure the ScrollableArea is on an active page needs 
-        // to be here as long as the ScrollableArea HashSet lives on Page. But it should really be
-        // moved to the top-level Document or a similar class that really represents a single 
-        // web page. https://bugs.webkit.org/show_bug.cgi?id=62762
-        if ((*it)->isOnActivePage())
-            (*it)->scrollAnimator()->setIsActive();
+    for (HashSet<ScrollableArea*>::const_iterator it = m_scrollableAreas->begin(), end = m_scrollableAreas->end(); it != end; ++it) {
+        ScrollableArea* scrollableArea = *it;
+
+        ASSERT(scrollableArea->isOnActivePage());
+        scrollableArea->scrollAnimator()->setIsActive();
     }
 }
 
@@ -2596,21 +2592,26 @@ void FrameView::notifyPageThatContentAreaWillPaint() const
     if (!page)
         return;
 
-    const HashSet<ScrollableArea*>* scrollableAreas = page->scrollableAreaSet();
-    if (!scrollableAreas)
+    contentAreaWillPaint();
+
+    if (!m_scrollableAreas)
         return;
 
-    HashSet<ScrollableArea*>::const_iterator end = scrollableAreas->end(); 
-    for (HashSet<ScrollableArea*>::const_iterator it = scrollableAreas->begin(); it != end; ++it)
-        (*it)->contentAreaWillPaint();
+    for (HashSet<ScrollableArea*>::const_iterator it = m_scrollableAreas->begin(), end = m_scrollableAreas->end(); it != end; ++it) {
+        ScrollableArea* scrollableArea = *it;
+
+        ASSERT(scrollableArea->isOnActivePage());
+        scrollableArea->contentAreaWillPaint();
+    }
 }
 
 bool FrameView::scrollAnimatorEnabled() const
 {
 #if ENABLE(SMOOTH_SCROLLING)
-    if (m_page && m_page->settings())
-        return m_page->settings()->scrollAnimatorEnabled();
+    if (Page* page = m_frame->page())
+        return page->settings()->scrollAnimatorEnabled();
 #endif
+
     return false;
 }
 
@@ -3262,6 +3263,43 @@ void FrameView::setTracksRepaints(bool trackRepaints)
     m_isTrackingRepaints = trackRepaints;
 }
 
+void FrameView::addScrollableArea(ScrollableArea* scrollableArea)
+{
+    if (!m_scrollableAreas)
+        m_scrollableAreas = adoptPtr(new ScrollableAreaSet);
+    m_scrollableAreas->add(scrollableArea);
+}
+
+void FrameView::removeScrollableArea(ScrollableArea* scrollableArea)
+{
+    if (!m_scrollableAreas)
+        return;
+    m_scrollableAreas->remove(scrollableArea);
+}
+
+bool FrameView::containsScrollableArea(ScrollableArea* scrollableArea) const
+{
+    if (!m_scrollableAreas)
+        return false;
+    return m_scrollableAreas->contains(scrollableArea);
+}
+
+void FrameView::addChild(PassRefPtr<Widget> widget)
+{
+    if (widget->isFrameView())
+        addScrollableArea(static_cast<FrameView*>(widget.get()));
+
+    ScrollView::addChild(widget);
+}
+
+void FrameView::removeChild(Widget* widget)
+{
+    if (widget->isFrameView())
+        removeScrollableArea(static_cast<FrameView*>(widget));
+
+    ScrollView::removeChild(widget);
+}
+
 bool FrameView::isVerticalDocument() const
 {
     RenderView* root = rootRenderer(this);
index 466a889..cd8eaa1 100644 (file)
@@ -308,6 +308,15 @@ public:
     void resetTrackedRepaints() { m_trackedRepaintRects.clear(); }
     const Vector<IntRect>& trackedRepaintRects() const { return m_trackedRepaintRects; }
 
+    typedef HashSet<ScrollableArea*> ScrollableAreaSet;
+    void addScrollableArea(ScrollableArea*);
+    void removeScrollableArea(ScrollableArea*);
+    bool containsScrollableArea(ScrollableArea*) const;
+    const ScrollableAreaSet* scrollableAreas() const { return m_scrollableAreas.get(); }
+
+    virtual void addChild(PassRefPtr<Widget>) OVERRIDE;
+    virtual void removeChild(Widget*) OVERRIDE;
+
 protected:
     virtual bool scrollContentsFastPath(const IntSize& scrollDelta, const IntRect& rectToScroll, const IntRect& clipRect);
     virtual void scrollContentsSlowPath(const IntRect& updateRect);
@@ -373,7 +382,6 @@ private:
 #endif
 
     virtual void notifyPageThatContentAreaWillPaint() const;
-    virtual void disconnectFromPage() { m_page = 0; }
 
     virtual bool scrollAnimatorEnabled() const;
 
@@ -476,8 +484,6 @@ private:
     // Renderer to hold our custom scroll corner.
     RenderScrollbarPart* m_scrollCorner;
 
-    Page* m_page;
-
     // If true, automatically resize the frame view around its content.
     bool m_shouldAutoSize;
     bool m_inAutoSize;
@@ -486,6 +492,8 @@ private:
     // The upper bound on the size when autosizing.
     IntSize m_maxAutoSize;
 
+    OwnPtr<ScrollableAreaSet> m_scrollableAreas;
+
     static double s_deferredRepaintDelay;
     static double s_initialDeferredRepaintDelayDuringLoading;
     static double s_maxDeferredRepaintDelayDuringLoading;
index e8c05e5..492e949 100644 (file)
@@ -211,12 +211,6 @@ Page::~Page()
     for (Frame* frame = mainFrame(); frame; frame = frame->tree()->traverseNext())
         frame->pageDestroyed();
 
-    if (m_scrollableAreaSet) {
-        ScrollableAreaSet::const_iterator end = m_scrollableAreaSet->end(); 
-        for (ScrollableAreaSet::const_iterator it = m_scrollableAreaSet->begin(); it != end; ++it)
-            (*it)->disconnectFromPage();
-    }
-
     m_editorClient->pageDestroyed();
 
 #if ENABLE(INSPECTOR)
@@ -1026,27 +1020,6 @@ void Page::privateBrowsingStateChanged()
         pluginViewBases[i]->privateBrowsingStateChanged(privateBrowsingEnabled);
 }
 
-void Page::addScrollableArea(ScrollableArea* scrollableArea)
-{
-    if (!m_scrollableAreaSet)
-        m_scrollableAreaSet = adoptPtr(new ScrollableAreaSet);
-    m_scrollableAreaSet->add(scrollableArea);
-}
-
-void Page::removeScrollableArea(ScrollableArea* scrollableArea)
-{
-    if (!m_scrollableAreaSet)
-        return;
-    m_scrollableAreaSet->remove(scrollableArea);
-}
-
-bool Page::containsScrollableArea(ScrollableArea* scrollableArea) const
-{
-    if (!m_scrollableAreaSet)
-        return false;
-    return m_scrollableAreaSet->contains(scrollableArea);
-}
-
 #if !ASSERT_DISABLED
 void Page::checkFrameCountConsistency() const
 {
index a12432d..5f6e206 100644 (file)
@@ -333,12 +333,6 @@ namespace WebCore {
         void setJavaScriptURLsAreAllowed(bool);
         bool javaScriptURLsAreAllowed() const;
 
-        typedef HashSet<ScrollableArea*> ScrollableAreaSet;
-        void addScrollableArea(ScrollableArea*);
-        void removeScrollableArea(ScrollableArea*);
-        bool containsScrollableArea(ScrollableArea*) const;
-        const ScrollableAreaSet* scrollableAreaSet() const { return m_scrollableAreaSet.get(); }
-
         // Don't allow more than a certain number of frames in a page.
         // This seems like a reasonable upper bound, and otherwise mutually
         // recursive frameset pages can quickly bring the program to its knees
@@ -465,8 +459,6 @@ namespace WebCore {
 
         double m_minimumTimerInterval;
 
-        OwnPtr<ScrollableAreaSet> m_scrollableAreaSet;
-
         bool m_isEditable;
 
 #if ENABLE(PAGE_VISIBILITY_API)
index ba586a4..cb6ab4e 100644 (file)
@@ -72,8 +72,8 @@ public:
 
     // Functions for child manipulation and inspection.
     const HashSet<RefPtr<Widget> >* children() const { return &m_children; }
-    void addChild(PassRefPtr<Widget>);
-    void removeChild(Widget*);
+    virtual void addChild(PassRefPtr<Widget>);
+    virtual void removeChild(Widget*);
     
     // If the scroll view does not use a native widget, then it will have cross-platform Scrollbars. These functions
     // can be used to obtain those scrollbars.
index 0b4583b..7b049dc 100644 (file)
@@ -161,8 +161,6 @@ public:
 
     virtual bool shouldRubberBandInDirection(ScrollDirection) const { return true; }
 
-    virtual void disconnectFromPage() { }
-
     virtual bool scrollAnimatorEnabled() const { return false; }
 
     // NOTE: Only called from Internals for testing.
index bdec5f9..fee27e9 100644 (file)
@@ -185,7 +185,6 @@ RenderLayer::RenderLayer(RenderBoxModelObject* renderer)
     , m_reflection(0)
     , m_scrollCorner(0)
     , m_resizer(0)
-    , m_scrollableAreaPage(0)
 {
     m_isNormalFlowOnly = shouldBeNormalFlowOnly();
 
@@ -204,8 +203,10 @@ RenderLayer::~RenderLayer()
             frame->eventHandler()->resizeLayerDestroyed();
     }
 
-    if (m_scrollableAreaPage)
-        m_scrollableAreaPage->removeScrollableArea(this);
+    if (Frame* frame = renderer()->frame()) {
+        if (FrameView* frameView = frame->view())
+            frameView->removeScrollableArea(this);
+    }
 
     destroyScrollbar(HorizontalScrollbar);
     destroyScrollbar(VerticalScrollbar);
@@ -4346,19 +4347,14 @@ void RenderLayer::styleChanged(StyleDifference, const RenderStyle* oldStyle)
 #if ENABLE(CSS_FILTERS)
     updateOrRemoveFilterEffect();
 #endif
-    
-    if (scrollsOverflow()) {
-        if (!m_scrollableAreaPage) {
-            if (Frame* frame = renderer()->frame()) {
-                if (Page* page = frame->page()) {
-                    m_scrollableAreaPage = page;
-                    m_scrollableAreaPage->addScrollableArea(this);
-                }
-            }
+
+    if (Frame* frame = renderer()->frame()) {
+        if (FrameView* frameView = frame->view()) {
+            if (scrollsOverflow())
+                frameView->addScrollableArea(this);
+            else
+                frameView->removeScrollableArea(this);
         }
-    } else if (m_scrollableAreaPage) {
-        m_scrollableAreaPage->removeScrollableArea(this);
-        m_scrollableAreaPage = 0;
     }
     
     // FIXME: Need to detect a swap from custom to native scrollbars (and vice versa).
index de75f24..df99bec 100644 (file)
@@ -668,8 +668,6 @@ private:
     // Rectangle encompassing the scroll corner and resizer rect.
     IntRect scrollCornerAndResizerRect() const;
 
-    virtual void disconnectFromPage() { m_scrollableAreaPage = 0; }
-
     // NOTE: This should only be called by the overriden setScrollOffset from ScrollableArea.
     void scrollTo(int, int);
 
@@ -868,8 +866,6 @@ private:
 #if USE(ACCELERATED_COMPOSITING)
     OwnPtr<RenderLayerBacking> m_backing;
 #endif
-
-    Page* m_scrollableAreaPage; // Page on which this is registered as a scrollable area.
 };
 
 inline void RenderLayer::updateZOrderLists()
index 1c001ed..9b19c79 100644 (file)
@@ -92,17 +92,17 @@ RenderListBox::RenderListBox(Element* element)
     ASSERT(element);
     ASSERT(element->isHTMLElement());
     ASSERT(element->hasTagName(HTMLNames::selectTag));
-    if (Page* page = frame()->page()) {
-        m_page = page;
-        m_page->addScrollableArea(this);
-    }
+
+    if (FrameView* frameView = frame()->view())
+        frameView->addScrollableArea(this);
 }
 
 RenderListBox::~RenderListBox()
 {
     setHasVerticalScrollbar(false);
-    if (m_page)
-        m_page->removeScrollableArea(this);
+
+    if (FrameView* frameView = frame()->view())
+        frameView->removeScrollableArea(this);
 }
 
 void RenderListBox::updateFromElement()
index 4d8b35f..ef9472e 100644 (file)
@@ -121,8 +121,6 @@ private:
 
     virtual ScrollableArea* enclosingScrollableArea() const;
 
-    virtual void disconnectFromPage() { m_page = 0; }
-
     // NOTE: This should only be called by the overriden setScrollOffset from ScrollableArea.
     void scrollTo(int newOffset);
 
@@ -147,8 +145,6 @@ private:
     int m_indexOffset;
 
     RefPtr<Scrollbar> m_vBar;
-
-    Page* m_page;
 };
 
 inline RenderListBox* toRenderListBox(RenderObject* object)
index bbd3016..06d81b8 100644 (file)
@@ -1,3 +1,21 @@
+2012-02-07  Anders Carlsson  <andersca@apple.com>
+
+        ScrollableAreaSet should be moved from Page to FrameView
+        https://bugs.webkit.org/show_bug.cgi?id=62762
+
+        Reviewed by Beth Dakin.
+
+        Update for changes to WebCore now that the scrollable area set is kept per frame view.
+
+        * src/ScrollbarGroup.cpp:
+        (WebKit::ScrollbarGroup::ScrollbarGroup):
+        (WebKit::ScrollbarGroup::~ScrollbarGroup):
+        * src/ScrollbarGroup.h:
+        (WebCore):
+        (ScrollbarGroup):
+        * src/WebPluginContainerImpl.cpp:
+        (WebKit::WebPluginContainerImpl::scrollbarGroup):
+
 2012-02-07  Hans Wennborg  <hans@chromium.org>
 
         Chromium: remove WebSpeechInputResult::set
index ad4ef83..579dc7a 100644 (file)
@@ -26,7 +26,7 @@
 #include "config.h"
 #include "ScrollbarGroup.h"
 
-#include "Page.h"
+#include "FrameView.h"
 #include "Scrollbar.h"
 #include "ScrollbarTheme.h"
 #include "platform/WebRect.h"
@@ -36,20 +36,19 @@ using namespace WebCore;
 
 namespace WebKit {
 
-ScrollbarGroup::ScrollbarGroup(Page* page)
-    : m_page(page)
+ScrollbarGroup::ScrollbarGroup(FrameView* frameView)
+    : m_frameView(frameView)
     , m_horizontalScrollbar(0)
     , m_verticalScrollbar(0)
 {
-    m_page->addScrollableArea(this);
+    m_frameView->addScrollableArea(this);
 }
 
 ScrollbarGroup::~ScrollbarGroup()
 {
     ASSERT(!m_horizontalScrollbar);
     ASSERT(!m_verticalScrollbar);
-    if (m_page)
-        m_page->removeScrollableArea(this);
+    m_frameView->removeScrollableArea(this);
 }
 
 void ScrollbarGroup::scrollbarCreated(WebScrollbarImpl* scrollbar)
@@ -249,9 +248,4 @@ bool ScrollbarGroup::isOnActivePage() const
     return true;
 }
 
-void ScrollbarGroup::disconnectFromPage()
-{
-    m_page = 0;
-}
-
 } // namespace WebKit
index 9093f33..44be2cf 100644 (file)
@@ -31,7 +31,7 @@
 #include <wtf/RefPtr.h>
 
 namespace WebCore {
-class Page;
+class FrameView;
 }
 
 namespace WebKit {
@@ -40,7 +40,7 @@ class WebScrollbarImpl;
 
 class ScrollbarGroup : public WebCore::ScrollableArea {
 public:
-    explicit ScrollbarGroup(WebCore::Page*);
+    explicit ScrollbarGroup(WebCore::FrameView*);
     ~ScrollbarGroup();
 
     void scrollbarCreated(WebScrollbarImpl*);
@@ -72,10 +72,9 @@ public:
     virtual bool shouldSuspendScrollAnimations() const;
     virtual void scrollbarStyleChanged(int newStyle, bool forceUpdate);
     virtual bool isOnActivePage() const;
-    virtual void disconnectFromPage();
 
 private:
-    WebCore::Page* m_page;
+    WebCore::FrameView* m_frameView;
     WebCore::IntPoint m_lastMousePosition;
     WebScrollbarImpl* m_horizontalScrollbar;
     WebScrollbarImpl* m_verticalScrollbar;
index a8f6d12..129088e 100644 (file)
@@ -493,7 +493,7 @@ WebCore::LayerChromium* WebPluginContainerImpl::platformLayer() const
 ScrollbarGroup* WebPluginContainerImpl::scrollbarGroup()
 {
     if (!m_scrollbarGroup)
-        m_scrollbarGroup = adoptPtr(new ScrollbarGroup(m_element->document()->frame()->page()));
+        m_scrollbarGroup = adoptPtr(new ScrollbarGroup(m_element->document()->frame()->view()));
     return m_scrollbarGroup.get();
 }
 
index bfcd366..f786386 100644 (file)
@@ -1,3 +1,20 @@
+2012-02-06  Anders Carlsson  <andersca@apple.com>
+
+        ScrollableAreaSet should be moved from Page to FrameView
+        https://bugs.webkit.org/show_bug.cgi?id=62762
+
+        Reviewed by Beth Dakin.
+
+        * WebProcess/Plugins/PDF/BuiltInPDFView.cpp:
+        (WebKit::BuiltInPDFView::initialize):
+        Call FrameView::addScrollableArea instead.
+
+        (WebKit::BuiltInPDFView::destroy):
+        Call FrameView::removeScrollableArea instead.
+
+        * WebProcess/Plugins/PDF/BuiltInPDFView.h:
+        Remove disconnectFromPage since it no longer exists on ScrollableArea.
+
 2012-02-07  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Add cut, copy and paste methods to WebKit2 GTK+ API
index 13f97d9..904a83d 100644 (file)
@@ -348,7 +348,7 @@ void BuiltInPDFView::calculateSizes()
 
 bool BuiltInPDFView::initialize(const Parameters& parameters)
 {
-    m_frame->coreFrame()->page()->addScrollableArea(this);
+    m_frame->coreFrame()->view()->addScrollableArea(this);
 
     // Load the src URL if needed.
     m_sourceURL = parameters.url;
@@ -361,8 +361,8 @@ bool BuiltInPDFView::initialize(const Parameters& parameters)
 void BuiltInPDFView::destroy()
 {
     if (m_frame) {
-        if (Page* page = m_frame->coreFrame()->page())
-            page->removeScrollableArea(this);
+        if (FrameView* frameView = m_frame->coreFrame()->view())
+            frameView->removeScrollableArea(this);
     }
 
     destroyScrollbar(HorizontalScrollbar);
index 6ee4bf3..c185c18 100644 (file)
@@ -138,7 +138,6 @@ private:
     virtual WebCore::Scrollbar* horizontalScrollbar() const  { return m_horizontalScrollbar.get(); }
     virtual WebCore::Scrollbar* verticalScrollbar() const { return m_verticalScrollbar.get(); }
     virtual bool isOnActivePage() const;
-    virtual void disconnectFromPage() { m_frame = 0; }
     virtual bool shouldSuspendScrollAnimations() const { return false; } // If we return true, ScrollAnimatorMac will keep cycling a timer forever, waiting for a good time to animate.
     virtual void scrollbarStyleChanged(int newStyle, bool forceUpdate);
     virtual void zoomAnimatorTransformChanged(float, float, float, ZoomAnimationState) { }