- Revision
- 107168
- Author
- [email protected]
- Date
- 2012-02-08 19:02:25 -0800 (Wed, 08 Feb 2012)
Log Message
Don't use the wheel event handler count to track if a page has horizontal scrollbars
https://bugs.webkit.org/show_bug.cgi?id=78192
Reviewed by Andreas Kling.
Source/WebCore:
Stop calling Document::didAddWheelEventHandler and Document::didRemoveWheelEventHandler when
adding and removing scrollbars.
* page/FrameView.cpp:
* page/FrameView.h:
(FrameView):
* rendering/RenderLayer.cpp:
* rendering/RenderLayer.h:
Source/WebKit2:
Prior to this change, we were incrementing and decrementing the wheel event handler count
whenever a scrollable area gained or lost a horizontal scrollbar, so we could use the count
to determine if Safari can handle horizontal wheel events directly or whether they have to be sent
to the web process first.
What this meant was that whenever a page had horizontal scrollbars we'd have to send all scroll wheel events
to the main thread instead of the scrolling thread, regardless of whether there were any wheel event handlers.
After this change, we traverse the tree of scrollable areas after every layout and check if any of them
have a horizontal scrollbar. (We still also check if there are wheel event handlers).
If traversing the tree after every layout is deemed to slow we can go back to caching the number of horizontal scrollbars
in a page, but the number of subframes in a page is usually very small and the number of other scrollable areas is even smaller.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::WebPageProxy):
(WebKit::WebPageProxy::willHandleHorizontalScrollEvents):
* UIProcess/WebPageProxy.h:
(WebKit::WebPageProxy::setCanShortCircuitHorizontalWheelEvents):
(WebPageProxy):
* UIProcess/WebPageProxy.messages.in:
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::numWheelEventHandlersChanged):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDidLayout):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::WebPage):
(WebKit::WebPage::numWheelEventHandlersChanged):
(WebKit):
(WebKit::hasEnabledHorizontalScrollbar):
(WebKit::pageContainsAnyHorizontalScrollbars):
(WebKit::WebPage::recomputeShortCircuitHorizontalWheelEventsState):
* WebProcess/WebPage/WebPage.h:
(WebPage):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (107167 => 107168)
--- trunk/Source/WebCore/ChangeLog 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebCore/ChangeLog 2012-02-09 03:02:25 UTC (rev 107168)
@@ -1,3 +1,19 @@
+2012-02-08 Anders Carlsson <[email protected]>
+
+ Don't use the wheel event handler count to track if a page has horizontal scrollbars
+ https://bugs.webkit.org/show_bug.cgi?id=78192
+
+ Reviewed by Andreas Kling.
+
+ Stop calling Document::didAddWheelEventHandler and Document::didRemoveWheelEventHandler when
+ adding and removing scrollbars.
+
+ * page/FrameView.cpp:
+ * page/FrameView.h:
+ (FrameView):
+ * rendering/RenderLayer.cpp:
+ * rendering/RenderLayer.h:
+
2012-02-08 Igor Oliveira <[email protected]>
Implement reverse animation direction
Modified: trunk/Source/WebCore/page/FrameView.cpp (107167 => 107168)
--- trunk/Source/WebCore/page/FrameView.cpp 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebCore/page/FrameView.cpp 2012-02-09 03:02:25 UTC (rev 107168)
@@ -321,21 +321,6 @@
}
}
-void FrameView::didAddHorizontalScrollbar(Scrollbar* scrollbar)
-{
- if (m_frame && m_frame->document())
- m_frame->document()->didAddWheelEventHandler();
- ScrollView::didAddHorizontalScrollbar(scrollbar);
-}
-
-void FrameView::willRemoveHorizontalScrollbar(Scrollbar* scrollbar)
-{
- ScrollView::willRemoveHorizontalScrollbar(scrollbar);
- // FIXME: maybe need a separate ScrollableArea::didRemoveHorizontalScrollbar callback?
- if (m_frame && m_frame->document())
- m_frame->document()->didRemoveWheelEventHandler();
-}
-
void FrameView::recalculateScrollbarOverlayStyle()
{
ScrollbarOverlayStyle oldOverlayStyle = scrollbarOverlayStyle();
Modified: trunk/Source/WebCore/page/FrameView.h (107167 => 107168)
--- trunk/Source/WebCore/page/FrameView.h 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebCore/page/FrameView.h 2012-02-09 03:02:25 UTC (rev 107168)
@@ -86,8 +86,6 @@
virtual PassRefPtr<Scrollbar> createScrollbar(ScrollbarOrientation);
virtual bool avoidScrollbarCreation() const;
- virtual void didAddHorizontalScrollbar(Scrollbar*);
- virtual void willRemoveHorizontalScrollbar(Scrollbar*);
virtual void setContentsSize(const IntSize&);
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (107167 => 107168)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-02-09 03:02:25 UTC (rev 107168)
@@ -2061,19 +2061,6 @@
return (m_hBar && m_hBar->enabled()) || (m_vBar && m_vBar->enabled());
}
-void RenderLayer::didAddHorizontalScrollbar(Scrollbar* scrollbar)
-{
- m_renderer->document()->didAddWheelEventHandler();
- ScrollableArea::didAddHorizontalScrollbar(scrollbar);
-}
-
-void RenderLayer::willRemoveHorizontalScrollbar(Scrollbar* scrollbar)
-{
- ScrollableArea::willRemoveHorizontalScrollbar(scrollbar);
- // FIXME: maybe need a separate ScrollableArea::didRemoveHorizontalScrollbar callback?
- m_renderer->document()->didRemoveWheelEventHandler();
-}
-
void RenderLayer::setHasHorizontalScrollbar(bool hasScrollbar)
{
if (hasScrollbar == (m_hBar != 0))
Modified: trunk/Source/WebCore/rendering/RenderLayer.h (107167 => 107168)
--- trunk/Source/WebCore/rendering/RenderLayer.h 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebCore/rendering/RenderLayer.h 2012-02-09 03:02:25 UTC (rev 107168)
@@ -312,8 +312,6 @@
bool scrollsOverflow() const;
bool allowsScrolling() const; // Returns true if at least one scrollbar is visible and enabled.
bool hasScrollbars() const { return m_hBar || m_vBar; }
- virtual void didAddHorizontalScrollbar(Scrollbar*);
- virtual void willRemoveHorizontalScrollbar(Scrollbar*);
void setHasHorizontalScrollbar(bool);
void setHasVerticalScrollbar(bool);
Modified: trunk/Source/WebKit2/ChangeLog (107167 => 107168)
--- trunk/Source/WebKit2/ChangeLog 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebKit2/ChangeLog 2012-02-09 03:02:25 UTC (rev 107168)
@@ -1,3 +1,45 @@
+2012-02-08 Anders Carlsson <[email protected]>
+
+ Don't use the wheel event handler count to track if a page has horizontal scrollbars
+ https://bugs.webkit.org/show_bug.cgi?id=78192
+
+ Reviewed by Andreas Kling.
+
+ Prior to this change, we were incrementing and decrementing the wheel event handler count
+ whenever a scrollable area gained or lost a horizontal scrollbar, so we could use the count
+ to determine if Safari can handle horizontal wheel events directly or whether they have to be sent
+ to the web process first.
+
+ What this meant was that whenever a page had horizontal scrollbars we'd have to send all scroll wheel events
+ to the main thread instead of the scrolling thread, regardless of whether there were any wheel event handlers.
+
+ After this change, we traverse the tree of scrollable areas after every layout and check if any of them
+ have a horizontal scrollbar. (We still also check if there are wheel event handlers).
+
+ If traversing the tree after every layout is deemed to slow we can go back to caching the number of horizontal scrollbars
+ in a page, but the number of subframes in a page is usually very small and the number of other scrollable areas is even smaller.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::WebPageProxy):
+ (WebKit::WebPageProxy::willHandleHorizontalScrollEvents):
+ * UIProcess/WebPageProxy.h:
+ (WebKit::WebPageProxy::setCanShortCircuitHorizontalWheelEvents):
+ (WebPageProxy):
+ * UIProcess/WebPageProxy.messages.in:
+ * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+ (WebKit::WebChromeClient::numWheelEventHandlersChanged):
+ * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+ (WebKit::WebFrameLoaderClient::dispatchDidLayout):
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::WebPage):
+ (WebKit::WebPage::numWheelEventHandlersChanged):
+ (WebKit):
+ (WebKit::hasEnabledHorizontalScrollbar):
+ (WebKit::pageContainsAnyHorizontalScrollbars):
+ (WebKit::WebPage::recomputeShortCircuitHorizontalWheelEventsState):
+ * WebProcess/WebPage/WebPage.h:
+ (WebPage):
+
2012-02-08 Sheriff Bot <[email protected]>
Unreviewed, rolling out r106920, r106924, r106933, r106939,
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (107167 => 107168)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2012-02-09 03:02:25 UTC (rev 107168)
@@ -195,6 +195,7 @@
, m_mainFrameHasCustomRepresentation(false)
, m_mainFrameHasHorizontalScrollbar(false)
, m_mainFrameHasVerticalScrollbar(false)
+ , m_canShortCircuitHorizontalWheelEvents(true)
, m_mainFrameIsPinnedToLeftSide(false)
, m_mainFrameIsPinnedToRightSide(false)
, m_pageCount(0)
@@ -3456,7 +3457,7 @@
bool WebPageProxy::willHandleHorizontalScrollEvents() const
{
- return m_wheelEventHandlerCount > 0;
+ return !m_canShortCircuitHorizontalWheelEvents;
}
void WebPageProxy::didFinishLoadingDataForCustomRepresentation(const String& suggestedFilename, const CoreIPC::DataReference& dataReference)
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (107167 => 107168)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h 2012-02-09 03:02:25 UTC (rev 107168)
@@ -719,7 +719,7 @@
void didChangeScrollOffsetPinningForMainFrame(bool pinnedToLeftSide, bool pinnedToRightSide);
void didChangePageCount(unsigned);
void didFailToInitializePlugin(const String& mimeType);
- void numWheelEventHandlersChanged(unsigned count) { m_wheelEventHandlerCount = count; }
+ void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
void reattachToWebProcess();
void reattachToWebProcessWithItem(WebBackForwardListItem*);
@@ -1009,8 +1009,10 @@
bool m_mainFrameHasHorizontalScrollbar;
bool m_mainFrameHasVerticalScrollbar;
- int m_wheelEventHandlerCount;
+ // Whether horizontal wheel events can be handled directly for swiping purposes.
+ bool m_canShortCircuitHorizontalWheelEvents;
+
bool m_mainFrameIsPinnedToLeftSide;
bool m_mainFrameIsPinnedToRightSide;
Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in (107167 => 107168)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in 2012-02-09 03:02:25 UTC (rev 107168)
@@ -66,7 +66,7 @@
DidChangeScrollOffsetPinningForMainFrame(bool hasHorizontalScrollbar, bool hasVerticalScrollbar)
DidChangePageCount(unsigned pageCount);
DidFailToInitializePlugin(WTF::String mimeType)
- NumWheelEventHandlersChanged(unsigned count)
+ SetCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents)
#if USE(TILED_BACKING_STORE)
PageDidRequestScroll(WebCore::IntPoint point)
Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp (107167 => 107168)
--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp 2012-02-09 03:02:25 UTC (rev 107168)
@@ -780,7 +780,7 @@
void WebChromeClient::numWheelEventHandlersChanged(unsigned count)
{
- m_page->send(Messages::WebPageProxy::NumWheelEventHandlersChanged(count));
+ m_page->numWheelEventHandlersChanged(count);
}
} // namespace WebKit
Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (107167 => 107168)
--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp 2012-02-09 03:02:25 UTC (rev 107168)
@@ -577,6 +577,8 @@
// Notify the bundle client.
webPage->injectedBundleLoaderClient().didLayoutForFrame(webPage, m_frame);
+ webPage->recomputeShortCircuitHorizontalWheelEventsState();
+
// NOTE: Unlike the other layout notifications, this does not notify the
// the UIProcess for every call.
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (107167 => 107168)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2012-02-09 03:02:25 UTC (rev 107168)
@@ -206,6 +206,8 @@
, m_isRunningModal(false)
, m_cachedMainFrameIsPinnedToLeftSide(false)
, m_cachedMainFrameIsPinnedToRightSide(false)
+ , m_canShortCircuitHorizontalWheelEvents(false)
+ , m_numWheelEventHandlers(0)
, m_cachedPageCount(0)
, m_isShowingContextMenu(false)
#if PLATFORM(WIN)
@@ -3000,6 +3002,68 @@
frame->editor()->confirmComposition(compositionString);
}
+void WebPage::numWheelEventHandlersChanged(unsigned numWheelEventHandlers)
+{
+ if (m_numWheelEventHandlers == numWheelEventHandlers)
+ return;
+
+ m_numWheelEventHandlers = numWheelEventHandlers;
+ recomputeShortCircuitHorizontalWheelEventsState();
+}
+
+static bool hasEnabledHorizontalScrollbar(ScrollableArea* scrollableArea)
+{
+ if (Scrollbar* scrollbar = scrollableArea->horizontalScrollbar())
+ return scrollbar->enabled();
+
+ return false;
+}
+
+static bool pageContainsAnyHorizontalScrollbars(Frame* mainFrame)
+{
+ if (FrameView* frameView = mainFrame->view()) {
+ if (hasEnabledHorizontalScrollbar(frameView))
+ return true;
+ }
+
+ for (Frame* frame = 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());
+
+ if (hasEnabledHorizontalScrollbar(scrollableArea))
+ return true;
+ }
+ }
+
+ return false;
+}
+
+void WebPage::recomputeShortCircuitHorizontalWheelEventsState()
+{
+ bool canShortCircuitHorizontalWheelEvents = !m_numWheelEventHandlers;
+
+ if (canShortCircuitHorizontalWheelEvents) {
+ // Check if we have any horizontal scroll bars on the page.
+ if (pageContainsAnyHorizontalScrollbars(mainFrame()))
+ canShortCircuitHorizontalWheelEvents = false;
+ }
+
+ if (m_canShortCircuitHorizontalWheelEvents == canShortCircuitHorizontalWheelEvents)
+ return;
+
+ m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents;
+ send(Messages::WebPageProxy::SetCanShortCircuitHorizontalWheelEvents(m_canShortCircuitHorizontalWheelEvents));
+}
+
Frame* WebPage::mainFrame() const
{
return m_page ? m_page->mainFrame() : 0;
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h (107167 => 107168)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h 2012-02-09 02:55:41 UTC (rev 107167)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h 2012-02-09 03:02:25 UTC (rev 107168)
@@ -500,6 +500,9 @@
void gestureEvent(const WebGestureEvent&);
#endif
+ void numWheelEventHandlersChanged(unsigned);
+ void recomputeShortCircuitHorizontalWheelEventsState();
+
private:
WebPage(uint64_t pageID, const WebPageCreationParameters&);
@@ -756,6 +759,8 @@
bool m_cachedMainFrameIsPinnedToLeftSide;
bool m_cachedMainFrameIsPinnedToRightSide;
+ bool m_canShortCircuitHorizontalWheelEvents;
+ unsigned m_numWheelEventHandlers;
unsigned m_cachedPageCount;
