Changeset 112325 in webkit

Timestamp:

Mar 27, 2012 3:24:36 PM (12 years ago)

Author:

commit-queue@webkit.org

Message:

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):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/FrameView.cpp (modified) (2 diffs)
• WebCore/page/scrolling/ScrollingCoordinator.cpp (modified) (2 diffs)
• WebCore/page/scrolling/ScrollingCoordinator.h (modified) (1 diff)
• WebCore/plugins/PluginViewBase.h (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/ScrollbarGroup.cpp (modified) (5 diffs)
• WebKit/chromium/src/ScrollbarGroup.h (modified) (3 diffs)
• WebKit/chromium/src/WebPluginContainerImpl.cpp (modified) (2 diffs)
• WebKit/chromium/src/WebPluginContainerImpl.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

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

trunk/Source/WebCore/page/FrameView.cpp

@@ -3386 +3386 @@
         m_scrollableAreas = adoptPtr(new ScrollableAreaSet);
     m_scrollableAreas->add(scrollableArea);
+
+    if (Page* page = m_frame->page()) {
+        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
+            scrollingCoordinator->frameViewScrollableAreasDidChange(this);
+    }
 }
 
@@ -3393 +3398 @@
         return;
     m_scrollableAreas->remove(scrollableArea);
+
+    if (Page* page = m_frame->page()) {
+        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
+            scrollingCoordinator->frameViewScrollableAreasDidChange(this);
+    }
 }
 

trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp

@@ -113 +113 @@
         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());
-        }
     }
 
@@ -160 +156 @@
 }
 
+void ScrollingCoordinator::frameViewScrollableAreasDidChange(FrameView*)
+{
+    ASSERT(isMainThread());
+    ASSERT(m_page);
+
+    Region nonFastScrollableRegion = computeNonFastScrollableRegion(m_page->mainFrame()->view());
+    setNonFastScrollableRegion(nonFastScrollableRegion);
+}
+
 void ScrollingCoordinator::frameViewWheelEventHandlerCountChanged(FrameView*)
 {

trunk/Source/WebCore/page/scrolling/ScrollingCoordinator.h

@@ -72 +72 @@
     // 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 

trunk/Source/WebCore/plugins/PluginViewBase.h

@@ -54 +54 @@
     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; }

trunk/Source/WebKit/chromium/ChangeLog

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

trunk/Source/WebKit/chromium/src/ScrollbarGroup.cpp

@@ -37 +37 @@
 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);
 }
 
@@ -49 +49 @@
     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);
@@ -63 +63 @@
         didAddVerticalScrollbar(scrollbar->scrollbar());
     }
+
+    if (!hadScrollbars)
+        m_frameView->addScrollableArea(this);
 }
 
@@ -75 +78 @@
         m_verticalScrollbar = 0;
     }
+
+    if (!m_horizontalScrollbar && !m_verticalScrollbar)
+        m_frameView->removeScrollableArea(this);
 }
 
@@ -128 +134 @@
     // FIXME: Return a parent scrollable area that can be scrolled.
     return 0;
+}
+
+void ScrollbarGroup::setFrameRect(const IntRect& frameRect)
+{
+    m_frameRect = frameRect;
+}
+
+IntRect ScrollbarGroup::scrollableAreaBoundingBox() const
+{
+    return m_frameRect;
 }
 

trunk/Source/WebKit/chromium/src/ScrollbarGroup.h

@@ -41 +41 @@
 class ScrollbarGroup : public WebCore::ScrollableArea {
 public:
-    explicit ScrollbarGroup(WebCore::FrameView*);
+    ScrollbarGroup(WebCore::FrameView*, const WebCore::IntRect& frameRect);
     ~ScrollbarGroup();
 
@@ -47 +47 @@
     void scrollbarDestroyed(WebScrollbarImpl*);
     void setLastMousePosition(const WebCore::IntPoint&);
+    void setFrameRect(const WebCore::IntRect&);
 
     // WebCore::ScrollableArea methods
@@ -73 +74 @@
     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;

trunk/Source/WebKit/chromium/src/WebPluginContainerImpl.cpp

@@ -329 +329 @@
     m_webPlugin->updateGeometry(windowRect, clipRect, cutOutRects, isVisible());
 
-    if (m_scrollbarGroup)
+    if (m_scrollbarGroup) {
         m_scrollbarGroup->scrollAnimator()->contentsResized();
+        m_scrollbarGroup->setFrameRect(frameRect());
+    }
 }
 
@@ -519 +521 @@
 #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();
 }

trunk/Source/WebKit/chromium/src/WebPluginContainerImpl.h

@@ -74 +74 @@
     // PluginViewBase methods
     virtual bool getFormValue(String&);
-    virtual bool wantWheelEvents();
 
     // Widget methods