Scrollable plugins not registered properly in ScrollingCoordinator

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

Patch by James Robinson <jamesr@chromium.org> on 2012-03-27
Reviewed by Anders Carlsson.

Source/WebCore:

Whenever a ScrollableArea is added or removed from a FrameView's ScrollableAreaSet, we have to recalculate the
nonFastScrollableRegion. This can happen for certain types of plugins that are scrollable.

This also reverts 112142 which was a not quite right way to handle these plugins.

* page/FrameView.cpp:
(WebCore::FrameView::addScrollableArea):
(WebCore::FrameView::removeScrollableArea):
* page/scrolling/ScrollingCoordinator.cpp:
(WebCore::computeNonFastScrollableRegion):
(WebCore::ScrollingCoordinator::frameViewScrollableAreasDidChange):
(WebCore):
* page/scrolling/ScrollingCoordinator.h:
(ScrollingCoordinator):
* plugins/PluginViewBase.h:

Source/WebKit/chromium:

Since ScrollbarGroups are ScrollableAreas, they need to be able to report their bounds for the
ScrollingCoordinator's calculateNonFastScrollableRegion. This also changes ScrollbarGroups to only be registered
as ScrollableAreas on the FrameView's set when they actually have Scrollbars.

* src/ScrollbarGroup.cpp:
(WebKit::ScrollbarGroup::ScrollbarGroup):
(WebKit::ScrollbarGroup::~ScrollbarGroup):
(WebKit::ScrollbarGroup::scrollbarCreated):
(WebKit::ScrollbarGroup::scrollbarDestroyed):
(WebKit::ScrollbarGroup::setFrameRect):
(WebKit):
(WebKit::ScrollbarGroup::scrollableAreaBoundingBox):
* src/ScrollbarGroup.h:
(ScrollbarGroup):
* src/WebPluginContainerImpl.cpp:
(WebKit::WebPluginContainerImpl::reportGeometry):
(WebKit):
(WebKit::WebPluginContainerImpl::scrollbarGroup):
* src/WebPluginContainerImpl.h:
(WebPluginContainerImpl):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 72c728f..c086f24 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+2012-03-27  James Robinson  <jamesr@chromium.org>
+
+        Scrollable plugins not registered properly in ScrollingCoordinator
+        https://bugs.webkit.org/show_bug.cgi?id=82163
+
+        Reviewed by Anders Carlsson.
+
+        Whenever a ScrollableArea is added or removed from a FrameView's ScrollableAreaSet, we have to recalculate the
+        nonFastScrollableRegion. This can happen for certain types of plugins that are scrollable.
+
+        This also reverts 112142 which was a not quite right way to handle these plugins.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::addScrollableArea):
+        (WebCore::FrameView::removeScrollableArea):
+        * page/scrolling/ScrollingCoordinator.cpp:
+        (WebCore::computeNonFastScrollableRegion):
+        (WebCore::ScrollingCoordinator::frameViewScrollableAreasDidChange):
+        (WebCore):
+        * page/scrolling/ScrollingCoordinator.h:
+        (ScrollingCoordinator):
+        * plugins/PluginViewBase.h:
+
 2012-03-27  Adam Klein  <adamk@chromium.org>
 
         Hold a reference to refChild in insertBefore before calling collectChildrenAndRemoveFromOldParent

diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index d8c74b6..eddab75 100644 (file)
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -3385,6 +3385,11 @@ void FrameView::addScrollableArea(ScrollableArea* scrollableArea)
     if (!m_scrollableAreas)
         m_scrollableAreas = adoptPtr(new ScrollableAreaSet);
     m_scrollableAreas->add(scrollableArea);
+
+    if (Page* page = m_frame->page()) {
+        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
+            scrollingCoordinator->frameViewScrollableAreasDidChange(this);
+    }
 }
 
 void FrameView::removeScrollableArea(ScrollableArea* scrollableArea)
@@ -3392,6 +3397,11 @@ void FrameView::removeScrollableArea(ScrollableArea* scrollableArea)
     if (!m_scrollableAreas)
         return;
     m_scrollableAreas->remove(scrollableArea);
+
+    if (Page* page = m_frame->page()) {
+        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
+            scrollingCoordinator->frameViewScrollableAreasDidChange(this);
+    }
 }
 
 bool FrameView::containsScrollableArea(ScrollableArea* scrollableArea) const

diff --git a/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp b/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
index 93e1da4..e5d954f 100644 (file)
--- a/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
+++ b/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
@@ -112,10 +112,6 @@ static Region computeNonFastScrollableRegion(FrameView* frameView)
     for (HashSet<RefPtr<Widget> >::const_iterator it = frameView->children()->begin(), end = frameView->children()->end(); it != end; ++it) {
         if ((*it)->isFrameView())
             childFrameViews.add(static_cast<FrameView*>(it->get()));
-        else if ((*it)->isPluginViewBase()) {
-            if (static_cast<PluginViewBase*>(it->get())->wantWheelEvents())
-                nonFastScrollableRegion.unite((*it)->frameRect());
-        }
     }
 
     if (const FrameView::ScrollableAreaSet* scrollableAreas = frameView->scrollableAreas()) {
@@ -159,6 +155,15 @@ void ScrollingCoordinator::frameViewLayoutUpdated(FrameView* frameView)
 
 }
 
+void ScrollingCoordinator::frameViewScrollableAreasDidChange(FrameView*)
+{
+    ASSERT(isMainThread());
+    ASSERT(m_page);
+
+    Region nonFastScrollableRegion = computeNonFastScrollableRegion(m_page->mainFrame()->view());
+    setNonFastScrollableRegion(nonFastScrollableRegion);
+}
+
 void ScrollingCoordinator::frameViewWheelEventHandlerCountChanged(FrameView*)
 {
     ASSERT(isMainThread());

diff --git a/Source/WebCore/page/scrolling/ScrollingCoordinator.h b/Source/WebCore/page/scrolling/ScrollingCoordinator.h
index ff54400..839235b 100644 (file)
--- a/Source/WebCore/page/scrolling/ScrollingCoordinator.h
+++ b/Source/WebCore/page/scrolling/ScrollingCoordinator.h
@@ -72,6 +72,9 @@ public:
     // Should be called whenever the given frame view has been laid out.
     void frameViewLayoutUpdated(FrameView*);
 
+    // Should be called whenever the set of ScrollableAreas inside a FrameView changes.
+    void frameViewScrollableAreasDidChange(FrameView*);
+
     // Should be called whenever a wheel event handler is added or removed in the 
     // frame view's underlying document.
     void frameViewWheelEventHandlerCountChanged(FrameView*);

diff --git a/Source/WebCore/plugins/PluginViewBase.h b/Source/WebCore/plugins/PluginViewBase.h
index a50061b..fa839d4 100644 (file)
--- a/Source/WebCore/plugins/PluginViewBase.h
+++ b/Source/WebCore/plugins/PluginViewBase.h
@@ -53,8 +53,6 @@ public:
     virtual bool getFormValue(String&) { return false; }
     virtual bool scroll(ScrollDirection, ScrollGranularity) { return false; }
 
-    virtual bool wantWheelEvents() { return false; }
-
     // A plug-in can ask WebKit to handle scrollbars for it.
     virtual Scrollbar* horizontalScrollbar() { return 0; }
     virtual Scrollbar* verticalScrollbar() { return 0; }

diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
index 0fde05b..411e463 100644 (file)
--- a/Source/WebKit/chromium/ChangeLog
+++ b/Source/WebKit/chromium/ChangeLog
@@ -1,3 +1,31 @@
+2012-03-27  James Robinson  <jamesr@chromium.org>
+
+        Scrollable plugins not registered properly in ScrollingCoordinator
+        https://bugs.webkit.org/show_bug.cgi?id=82163
+
+        Reviewed by Anders Carlsson.
+
+        Since ScrollbarGroups are ScrollableAreas, they need to be able to report their bounds for the
+        ScrollingCoordinator's calculateNonFastScrollableRegion. This also changes ScrollbarGroups to only be registered
+        as ScrollableAreas on the FrameView's set when they actually have Scrollbars.
+
+        * src/ScrollbarGroup.cpp:
+        (WebKit::ScrollbarGroup::ScrollbarGroup):
+        (WebKit::ScrollbarGroup::~ScrollbarGroup):
+        (WebKit::ScrollbarGroup::scrollbarCreated):
+        (WebKit::ScrollbarGroup::scrollbarDestroyed):
+        (WebKit::ScrollbarGroup::setFrameRect):
+        (WebKit):
+        (WebKit::ScrollbarGroup::scrollableAreaBoundingBox):
+        * src/ScrollbarGroup.h:
+        (ScrollbarGroup):
+        * src/WebPluginContainerImpl.cpp:
+        (WebKit::WebPluginContainerImpl::reportGeometry):
+        (WebKit):
+        (WebKit::WebPluginContainerImpl::scrollbarGroup):
+        * src/WebPluginContainerImpl.h:
+        (WebPluginContainerImpl):
+
 2012-03-27  Dana Jansens  <danakj@chromium.org>
 
         [chromium] Make use of common animation unit test methods

diff --git a/Source/WebKit/chromium/src/ScrollbarGroup.cpp b/Source/WebKit/chromium/src/ScrollbarGroup.cpp
index 579dc7a..c48db3d 100644 (file)
--- a/Source/WebKit/chromium/src/ScrollbarGroup.cpp
+++ b/Source/WebKit/chromium/src/ScrollbarGroup.cpp
@@ -36,23 +36,23 @@ using namespace WebCore;
 
 namespace WebKit {
 
-ScrollbarGroup::ScrollbarGroup(FrameView* frameView)
+ScrollbarGroup::ScrollbarGroup(FrameView* frameView, const IntRect& frameRect)
     : m_frameView(frameView)
+    , m_frameRect(frameRect)
     , m_horizontalScrollbar(0)
     , m_verticalScrollbar(0)
 {
-    m_frameView->addScrollableArea(this);
 }
 
 ScrollbarGroup::~ScrollbarGroup()
 {
     ASSERT(!m_horizontalScrollbar);
     ASSERT(!m_verticalScrollbar);
-    m_frameView->removeScrollableArea(this);
 }
 
 void ScrollbarGroup::scrollbarCreated(WebScrollbarImpl* scrollbar)
 {
+    bool hadScrollbars = m_horizontalScrollbar || m_verticalScrollbar;
     if (scrollbar->scrollbar()->orientation() == HorizontalScrollbar) {
         ASSERT(!m_horizontalScrollbar);
         m_horizontalScrollbar = scrollbar;
@@ -62,6 +62,9 @@ void ScrollbarGroup::scrollbarCreated(WebScrollbarImpl* scrollbar)
         m_verticalScrollbar = scrollbar;
         didAddVerticalScrollbar(scrollbar->scrollbar());
     }
+
+    if (!hadScrollbars)
+        m_frameView->addScrollableArea(this);
 }
 
 void ScrollbarGroup::scrollbarDestroyed(WebScrollbarImpl* scrollbar)
@@ -74,6 +77,9 @@ void ScrollbarGroup::scrollbarDestroyed(WebScrollbarImpl* scrollbar)
         willRemoveVerticalScrollbar(scrollbar->scrollbar());
         m_verticalScrollbar = 0;
     }
+
+    if (!m_horizontalScrollbar && !m_verticalScrollbar)
+        m_frameView->removeScrollableArea(this);
 }
 
 void ScrollbarGroup::setLastMousePosition(const IntPoint& point)
@@ -129,6 +135,16 @@ ScrollableArea* ScrollbarGroup::enclosingScrollableArea() const
     return 0;
 }
 
+void ScrollbarGroup::setFrameRect(const IntRect& frameRect)
+{
+    m_frameRect = frameRect;
+}
+
+IntRect ScrollbarGroup::scrollableAreaBoundingBox() const
+{
+    return m_frameRect;
+}
+
 bool ScrollbarGroup::isScrollCornerVisible() const
 {
     return false;

diff --git a/Source/WebKit/chromium/src/ScrollbarGroup.h b/Source/WebKit/chromium/src/ScrollbarGroup.h
index 44be2cf..aeb4405 100644 (file)
--- a/Source/WebKit/chromium/src/ScrollbarGroup.h
+++ b/Source/WebKit/chromium/src/ScrollbarGroup.h
@@ -40,12 +40,13 @@ class WebScrollbarImpl;
 
 class ScrollbarGroup : public WebCore::ScrollableArea {
 public:
-    explicit ScrollbarGroup(WebCore::FrameView*);
+    ScrollbarGroup(WebCore::FrameView*, const WebCore::IntRect& frameRect);
     ~ScrollbarGroup();
 
     void scrollbarCreated(WebScrollbarImpl*);
     void scrollbarDestroyed(WebScrollbarImpl*);
     void setLastMousePosition(const WebCore::IntPoint&);
+    void setFrameRect(const WebCore::IntRect&);
 
     // WebCore::ScrollableArea methods
     virtual int scrollSize(WebCore::ScrollbarOrientation) const;
@@ -72,10 +73,12 @@ public:
     virtual bool shouldSuspendScrollAnimations() const;
     virtual void scrollbarStyleChanged(int newStyle, bool forceUpdate);
     virtual bool isOnActivePage() const;
+    virtual WebCore::IntRect scrollableAreaBoundingBox() const;
 
 private:
     WebCore::FrameView* m_frameView;
     WebCore::IntPoint m_lastMousePosition;
+    WebCore::IntRect m_frameRect;
     WebScrollbarImpl* m_horizontalScrollbar;
     WebScrollbarImpl* m_verticalScrollbar;
 };

diff --git a/Source/WebKit/chromium/src/WebPluginContainerImpl.cpp b/Source/WebKit/chromium/src/WebPluginContainerImpl.cpp
index 4aad9bb..f9e678d 100644 (file)
--- a/Source/WebKit/chromium/src/WebPluginContainerImpl.cpp
+++ b/Source/WebKit/chromium/src/WebPluginContainerImpl.cpp
@@ -328,8 +328,10 @@ void WebPluginContainerImpl::reportGeometry()
 
     m_webPlugin->updateGeometry(windowRect, clipRect, cutOutRects, isVisible());
 
-    if (m_scrollbarGroup)
+    if (m_scrollbarGroup) {
         m_scrollbarGroup->scrollAnimator()->contentsResized();
+        m_scrollbarGroup->setFrameRect(frameRect());
+    }
 }
 
 void WebPluginContainerImpl::setBackingTextureId(unsigned id)
@@ -518,15 +520,10 @@ WebCore::LayerChromium* WebPluginContainerImpl::platformLayer() const
 }
 #endif
 
-bool WebPluginContainerImpl::wantWheelEvents()
-{
-    return m_scrollbarGroup;
-}
-
 ScrollbarGroup* WebPluginContainerImpl::scrollbarGroup()
 {
     if (!m_scrollbarGroup)
-        m_scrollbarGroup = adoptPtr(new ScrollbarGroup(m_element->document()->frame()->view()));
+        m_scrollbarGroup = adoptPtr(new ScrollbarGroup(m_element->document()->frame()->view(), frameRect()));
     return m_scrollbarGroup.get();
 }
 

diff --git a/Source/WebKit/chromium/src/WebPluginContainerImpl.h b/Source/WebKit/chromium/src/WebPluginContainerImpl.h
index cc635c9..9d970f2 100644 (file)
--- a/Source/WebKit/chromium/src/WebPluginContainerImpl.h
+++ b/Source/WebKit/chromium/src/WebPluginContainerImpl.h
@@ -73,7 +73,6 @@ public:
 
     // PluginViewBase methods
     virtual bool getFormValue(String&);
-    virtual bool wantWheelEvents();
 
     // Widget methods
     virtual void setFrameRect(const WebCore::IntRect&);