Skip to content

Commit

Permalink
Fix keyboard scrolling after 269941@main
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=265396
rdar://118453548

Reviewed by Simon Fraser.

I didn't quite get the KeyboardScrollData hooked up correctly in that PR.
I thought it might need a structural refactoring, but I was wrong.
This restores behavior to how it was before that PR.

* Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.cpp:
(WebCore::ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode):
* Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.h:
* Source/WebCore/page/scrolling/ScrollingStateNode.h:
* Source/WebCore/page/scrolling/ScrollingStateOverflowScrollingNode.cpp:
(WebCore::ScrollingStateOverflowScrollingNode::ScrollingStateOverflowScrollingNode):
* Source/WebCore/page/scrolling/ScrollingStateOverflowScrollingNode.h:
* Source/WebCore/page/scrolling/ScrollingStatePluginScrollingNode.cpp:
(WebCore::ScrollingStatePluginScrollingNode::ScrollingStatePluginScrollingNode):
* Source/WebCore/page/scrolling/ScrollingStatePluginScrollingNode.h:
* Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.serialization.in:

Canonical link: https://commits.webkit.org/271160@main
  • Loading branch information
achristensen07 committed Nov 27, 2023
1 parent 89ae804 commit 07f2e6a
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 24 deletions.
26 changes: 13 additions & 13 deletions Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode(
MouseLocationState&& mouseLocationState,
ScrollbarHoverState&& scrollbarHoverState,
ScrollbarEnabledState&& scrollbarEnabledState,
RequestedKeyboardScrollData&& keyboardScrollData,
float frameScaleFactor,
EventTrackingRegions&& eventTrackingRegions,
std::optional<PlatformLayerIdentifier> rootContentsLayer,
Expand All @@ -80,8 +81,7 @@ ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode(
FloatPoint minLayoutViewportOrigin,
FloatPoint maxLayoutViewportOrigin,
std::optional<FloatSize> overrideVisualViewportSize,
bool overlayScrollbarsEnabled,
RequestedKeyboardScrollData&& keyboardScrollData
bool overlayScrollbarsEnabled
) : ScrollingStateScrollingNode(
isMainFrame ? ScrollingNodeType::MainFrame : ScrollingNodeType::Subframe,
scrollingNodeID,
Expand Down Expand Up @@ -111,17 +111,17 @@ ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode(
WTFMove(scrollbarHoverState),
WTFMove(scrollbarEnabledState),
WTFMove(keyboardScrollData))
, m_rootContentsLayer(rootContentsLayer.value_or(PlatformLayerIdentifier()))
, m_counterScrollingLayer(counterScrollingLayer.value_or(PlatformLayerIdentifier()))
, m_insetClipLayer(insetClipLayer.value_or(PlatformLayerIdentifier()))
, m_contentShadowLayer(contentShadowLayer.value_or(PlatformLayerIdentifier()))
, m_eventTrackingRegions(WTFMove(eventTrackingRegions))
, m_layoutViewport(layoutViewport)
, m_minLayoutViewportOrigin(minLayoutViewportOrigin)
, m_maxLayoutViewportOrigin(maxLayoutViewportOrigin)
, m_overrideVisualViewportSize(overrideVisualViewportSize)
, m_frameScaleFactor(frameScaleFactor)
, m_topContentInset(topContentInset)
, m_rootContentsLayer(rootContentsLayer.value_or(PlatformLayerIdentifier()))
, m_counterScrollingLayer(counterScrollingLayer.value_or(PlatformLayerIdentifier()))
, m_insetClipLayer(insetClipLayer.value_or(PlatformLayerIdentifier()))
, m_contentShadowLayer(contentShadowLayer.value_or(PlatformLayerIdentifier()))
, m_eventTrackingRegions(WTFMove(eventTrackingRegions))
, m_layoutViewport(layoutViewport)
, m_minLayoutViewportOrigin(minLayoutViewportOrigin)
, m_maxLayoutViewportOrigin(maxLayoutViewportOrigin)
, m_overrideVisualViewportSize(overrideVisualViewportSize)
, m_frameScaleFactor(frameScaleFactor)
, m_topContentInset(topContentInset)
, m_headerHeight(headerHeight)
, m_footerHeight(footerHeight)
, m_behaviorForFixed(WTFMove(scrollBehaviorForFixedElements))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ class ScrollingStateFrameScrollingNode final : public ScrollingStateScrollingNod
MouseLocationState&&,
ScrollbarHoverState&&,
ScrollbarEnabledState&&,
RequestedKeyboardScrollData&&,
float frameScaleFactor,
EventTrackingRegions&&,
std::optional<PlatformLayerIdentifier> rootContentsLayer,
Expand All @@ -172,8 +173,7 @@ class ScrollingStateFrameScrollingNode final : public ScrollingStateScrollingNod
FloatPoint minLayoutViewportOrigin,
FloatPoint maxLayoutViewportOrigin,
std::optional<FloatSize> overrideVisualViewportSize,
bool overlayScrollbarsEnabled,
RequestedKeyboardScrollData&&
bool overlayScrollbarsEnabled
);

ScrollingStateFrameScrollingNode(ScrollingStateTree&, ScrollingNodeType, ScrollingNodeID);
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/page/scrolling/ScrollingStateNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ enum class ScrollingStateNodeProperty : uint64_t {
ScrollbarHoverState = MouseActivityState << 1,
ScrollbarEnabledState = ScrollbarHoverState << 1,
// ScrollingStateFrameScrollingNode
FrameScaleFactor = ScrollbarEnabledState << 1,
KeyboardScrollData = ScrollbarEnabledState << 1,
FrameScaleFactor = KeyboardScrollData << 1,
EventTrackingRegion = FrameScaleFactor << 1,
RootContentsLayer = EventTrackingRegion << 1,
CounterScrollingLayer = RootContentsLayer << 1,
Expand All @@ -257,7 +258,6 @@ enum class ScrollingStateNodeProperty : uint64_t {
MaxLayoutViewportOrigin = MinLayoutViewportOrigin << 1,
OverrideVisualViewportSize = MaxLayoutViewportOrigin << 1,
OverlayScrollbarsEnabled = OverrideVisualViewportSize << 1,
KeyboardScrollData = OverlayScrollbarsEnabled << 1,
// ScrollingStatePositionedNode
RelatedOverflowScrollingNodes = 1LLU << 1, // Same value as ScrollableAreaSize, ViewportConstraints and OverflowScrollingNode
LayoutConstraintData = 1LLU << 2, // Same value as TotalContentsSize
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ ScrollingStateOverflowScrollingNode::ScrollingStateOverflowScrollingNode(
bool mouseIsOverContentArea,
MouseLocationState&& mouseLocationState,
ScrollbarHoverState&& scrollbarHoverState,
ScrollbarEnabledState&& scrollbarEnabledState
ScrollbarEnabledState&& scrollbarEnabledState,
RequestedKeyboardScrollData&& scrollData
) : ScrollingStateScrollingNode(
ScrollingNodeType::Overflow,
scrollingNodeID,
Expand Down Expand Up @@ -88,7 +89,7 @@ ScrollingStateOverflowScrollingNode::ScrollingStateOverflowScrollingNode(
WTFMove(mouseLocationState),
WTFMove(scrollbarHoverState),
WTFMove(scrollbarEnabledState),
RequestedKeyboardScrollData { } // FIXME: This probably needs to be refactored so that this is a member of ScrollingStateFrameScrollingNode instead of ScrollingStateScrollingNode.
WTFMove(scrollData)
) { }

ScrollingStateOverflowScrollingNode::ScrollingStateOverflowScrollingNode(ScrollingStateTree& stateTree, ScrollingNodeID nodeID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ class ScrollingStateOverflowScrollingNode : public ScrollingStateScrollingNode {
bool mouseIsOverContentArea,
MouseLocationState&&,
ScrollbarHoverState&&,
ScrollbarEnabledState&&
ScrollbarEnabledState&&,
RequestedKeyboardScrollData&&
);
ScrollingStateOverflowScrollingNode(ScrollingStateTree&, ScrollingNodeID);
ScrollingStateOverflowScrollingNode(const ScrollingStateOverflowScrollingNode&, ScrollingStateTree&);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ ScrollingStatePluginScrollingNode::ScrollingStatePluginScrollingNode(
bool mouseIsOverContentArea,
MouseLocationState&& mouseLocationState,
ScrollbarHoverState&& scrollbarHoverState,
ScrollbarEnabledState&& scrollbarEnabledState
ScrollbarEnabledState&& scrollbarEnabledState,
RequestedKeyboardScrollData&& scrollData
) : ScrollingStateScrollingNode(
ScrollingNodeType::PluginScrolling,
scrollingNodeID,
Expand Down Expand Up @@ -88,7 +89,7 @@ ScrollingStatePluginScrollingNode::ScrollingStatePluginScrollingNode(
WTFMove(mouseLocationState),
WTFMove(scrollbarHoverState),
WTFMove(scrollbarEnabledState),
RequestedKeyboardScrollData { } // FIXME: This probably needs to be refactored so that this is a member of ScrollingStateFrameScrollingNode instead of ScrollingStateScrollingNode.
WTFMove(scrollData)
)
{
ASSERT(isPluginScrollingNode());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class ScrollingStatePluginScrollingNode final : public ScrollingStateScrollingNo
bool mouseIsOverContentArea,
MouseLocationState&&,
ScrollbarHoverState&&,
ScrollbarEnabledState&&
ScrollbarEnabledState&&,
RequestedKeyboardScrollData&&
);

ScrollingStatePluginScrollingNode(ScrollingStateTree&, ScrollingNodeType, ScrollingNodeID);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ header: <WebCore/ScrollingStatePositionedNode.h>
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::MouseActivityState] WebCore::MouseLocationState mouseLocationState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::ScrollbarHoverState] WebCore::ScrollbarHoverState scrollbarHoverState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::ScrollbarEnabledState] WebCore::ScrollbarEnabledState scrollbarEnabledState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::KeyboardScrollData] WebCore::RequestedKeyboardScrollData keyboardScrollData()
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::FrameScaleFactor] float frameScaleFactor()
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::EventTrackingRegion] WebCore::EventTrackingRegions eventTrackingRegions()
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::RootContentsLayer] std::optional<WebCore::PlatformLayerIdentifier> rootContentsLayer().layerIDForEncoding();
Expand All @@ -110,7 +111,6 @@ header: <WebCore/ScrollingStatePositionedNode.h>
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::MaxLayoutViewportOrigin] WebCore::FloatPoint maxLayoutViewportOrigin();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::OverrideVisualViewportSize] std::optional<WebCore::FloatSize> overrideVisualViewportSize();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::OverlayScrollbarsEnabled] bool overlayScrollbarsEnabled();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::KeyboardScrollData] WebCore::RequestedKeyboardScrollData keyboardScrollData()
}

[RefCounted] class WebCore::ScrollingStateOverflowScrollingNode {
Expand Down Expand Up @@ -141,6 +141,7 @@ header: <WebCore/ScrollingStatePositionedNode.h>
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::MouseActivityState] WebCore::MouseLocationState mouseLocationState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::ScrollbarHoverState] WebCore::ScrollbarHoverState scrollbarHoverState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::ScrollbarEnabledState] WebCore::ScrollbarEnabledState scrollbarEnabledState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::KeyboardScrollData] WebCore::RequestedKeyboardScrollData keyboardScrollData()
};

[RefCounted] class WebCore::ScrollingStatePluginHostingNode {
Expand Down Expand Up @@ -178,6 +179,7 @@ header: <WebCore/ScrollingStatePositionedNode.h>
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::MouseActivityState] WebCore::MouseLocationState mouseLocationState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::ScrollbarHoverState] WebCore::ScrollbarHoverState scrollbarHoverState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::ScrollbarEnabledState] WebCore::ScrollbarEnabledState scrollbarEnabledState();
[OptionalTupleBit=WebCore::ScrollingStateNode::Property::KeyboardScrollData] WebCore::RequestedKeyboardScrollData keyboardScrollData()
}

[CustomHeader] struct WebCore::ScrollbarHoverState {
Expand Down

0 comments on commit 07f2e6a

Please sign in to comment.