Diff
Modified: trunk/Source/WebCore/ChangeLog (264133 => 264134)
--- trunk/Source/WebCore/ChangeLog 2020-07-08 21:33:54 UTC (rev 264133)
+++ trunk/Source/WebCore/ChangeLog 2020-07-08 21:40:44 UTC (rev 264134)
@@ -1,3 +1,33 @@
+2020-07-08 Simon Fraser <[email protected]>
+
+ Make a logging channel for ScrollSnap and improve its logging
+ https://bugs.webkit.org/show_bug.cgi?id=214096
+
+ Reviewed by Wenson Hsieh.
+
+ Pull existing scroll snap logging out of the Scrolling log channel into a new ScrollSnap channel,
+ add more logging, and make better use of TextStream-based logging.
+
+ * page/scrolling/AxisScrollSnapOffsets.cpp:
+ (WebCore::operator<<):
+ (WebCore::updateSnapOffsetsForScrollableArea):
+ (WebCore::snapOffsetsToString): Deleted.
+ (WebCore::snapOffsetRangesToString): Deleted.
+ (WebCore::snapPortOrAreaToString): Deleted.
+ * platform/Logging.h:
+ * platform/ScrollableArea.cpp:
+ (WebCore::ScrollableArea::updateScrollSnapState):
+ * platform/cocoa/ScrollController.mm:
+ (WebCore::operator<<):
+ (WebCore::ScrollController::processWheelEventForScrollSnap):
+ (WebCore::ScrollController::startScrollSnapTimer):
+ (WebCore::ScrollController::stopScrollSnapTimer):
+ (WebCore::ScrollController::scrollSnapTimerFired):
+ (WebCore::ScrollController::updateScrollSnapState):
+ * platform/cocoa/ScrollSnapAnimatorState.h:
+ * platform/cocoa/ScrollSnapAnimatorState.mm:
+ (WebCore::operator<<):
+
2020-07-08 Per Arne Vollan <[email protected]>
[Cocoa] Make it possible to establish direct XPC connections between WebKit processes
Modified: trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp (264133 => 264134)
--- trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp 2020-07-08 21:33:54 UTC (rev 264133)
+++ trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp 2020-07-08 21:40:44 UTC (rev 264134)
@@ -71,35 +71,13 @@
}
}
-#if !LOG_DISABLED
-
-static String snapOffsetsToString(const Vector<LayoutUnit>& snapOffsets)
+template<typename T>
+TextStream& operator<<(TextStream& ts, const ScrollOffsetRange<T>& range)
{
- StringBuilder builder;
- builder.appendLiteral("[ ");
- for (auto& offset : snapOffsets)
- builder.append(offset.toFloat(), ' ');
- builder.append(']');
- return builder.toString();
+ ts << "start: " << range.start << " end: " << range.end;
+ return ts;
}
-static String snapOffsetRangesToString(const Vector<ScrollOffsetRange<LayoutUnit>>& ranges)
-{
- StringBuilder builder;
- builder.appendLiteral("[ ");
- for (auto& range : ranges)
- builder.append('(', range.start.toFloat(), ", ", range.end.toFloat(), ") ");
- builder.append(']');
- return builder.toString();
-}
-
-static String snapPortOrAreaToString(const LayoutRect& rect)
-{
- return makeString("{{", rect.x().toFloat(), ", ", rect.y().toFloat(), "} {", rect.width().toFloat(), ", ", rect.height().toFloat(), "}}");
-}
-
-#endif
-
template <typename LayoutType>
static void indicesOfNearestSnapOffsetRanges(LayoutType offset, const Vector<ScrollOffsetRange<LayoutType>>& snapOffsetRanges, unsigned& lowerIndex, unsigned& upperIndex)
{
@@ -218,9 +196,7 @@
// The bounds of the scrolling container's snap port, where the top left of the scrolling container's border box is the origin.
auto scrollSnapPort = computeScrollSnapPortOrAreaRect(scrollingElementBox.paddingBoxRect(), scrollingElementStyle.scrollPadding(), InsetOrOutset::Inset);
-#if !LOG_DISABLED
- LOG(Scrolling, "Computing scroll snap offsets in snap port: %s", snapPortOrAreaToString(scrollSnapPort).utf8().data());
-#endif
+ LOG_WITH_STREAM(ScrollSnap, stream << "Computing scroll snap offsets for " << scrollableArea << " in snap port " << scrollSnapPort);
for (auto* child : scrollContainer->view().boxesWithScrollSnapPositions()) {
if (child->enclosingScrollableContainerForSnapping() != scrollContainer)
continue;
@@ -230,9 +206,7 @@
auto scrollSnapArea = LayoutRect(child->localToContainerQuad(FloatQuad(child->borderBoundingBox()), scrollingElement.renderBox()).boundingBox());
scrollSnapArea.moveBy(containerScrollOffset);
scrollSnapArea = computeScrollSnapPortOrAreaRect(scrollSnapArea, child->style().scrollSnapMargin(), InsetOrOutset::Outset);
-#if !LOG_DISABLED
- LOG(Scrolling, " Considering scroll snap area: %s", snapPortOrAreaToString(scrollSnapArea).utf8().data());
-#endif
+ LOG_WITH_STREAM(ScrollSnap, stream << " Considering scroll snap target area " << scrollSnapArea);
auto alignment = child->style().scrollSnapAlign();
if (hasHorizontalSnapOffsets && alignment.x != ScrollSnapAxisAlignType::None) {
auto absoluteScrollOffset = clampTo<LayoutUnit>(computeScrollSnapAlignOffset(scrollSnapArea.x(), scrollSnapArea.width(), alignment.x) - computeScrollSnapAlignOffset(scrollSnapPort.x(), scrollSnapPort.width(), alignment.x), 0, maxScrollLeft);
@@ -252,10 +226,8 @@
if (!horizontalSnapOffsets.isEmpty()) {
adjustAxisSnapOffsetsForScrollExtent(horizontalSnapOffsets, maxScrollLeft);
-#if !LOG_DISABLED
- LOG(Scrolling, " => Computed horizontal scroll snap offsets: %s", snapOffsetsToString(horizontalSnapOffsets).utf8().data());
- LOG(Scrolling, " => Computed horizontal scroll snap offset ranges: %s", snapOffsetRangesToString(horizontalSnapOffsetRanges).utf8().data());
-#endif
+ LOG_WITH_STREAM(ScrollSnap, stream << " => Computed horizontal scroll snap offsets: " << horizontalSnapOffsets);
+ LOG_WITH_STREAM(ScrollSnap, stream << " => Computed horizontal scroll snap offset ranges: " << horizontalSnapOffsetRanges);
if (scrollSnapType.strictness == ScrollSnapStrictness::Proximity)
computeAxisProximitySnapOffsetRanges(horizontalSnapOffsets, horizontalSnapOffsetRanges, scrollSnapPort.width());
@@ -266,10 +238,8 @@
if (!verticalSnapOffsets.isEmpty()) {
adjustAxisSnapOffsetsForScrollExtent(verticalSnapOffsets, maxScrollTop);
-#if !LOG_DISABLED
- LOG(Scrolling, " => Computed vertical scroll snap offsets: %s", snapOffsetsToString(verticalSnapOffsets).utf8().data());
- LOG(Scrolling, " => Computed vertical scroll snap offset ranges: %s", snapOffsetRangesToString(verticalSnapOffsetRanges).utf8().data());
-#endif
+ LOG_WITH_STREAM(ScrollSnap, stream << " => Computed vertical scroll snap offsets: " << verticalSnapOffsets);
+ LOG_WITH_STREAM(ScrollSnap, stream << " => Computed vertical scroll snap offset ranges: " << verticalSnapOffsetRanges);
if (scrollSnapType.strictness == ScrollSnapStrictness::Proximity)
computeAxisProximitySnapOffsetRanges(verticalSnapOffsets, verticalSnapOffsetRanges, scrollSnapPort.height());
Modified: trunk/Source/WebCore/platform/Logging.h (264133 => 264134)
--- trunk/Source/WebCore/platform/Logging.h 2020-07-08 21:33:54 UTC (rev 264133)
+++ trunk/Source/WebCore/platform/Logging.h 2020-07-08 21:40:44 UTC (rev 264134)
@@ -94,6 +94,7 @@
M(ResourceLoading) \
M(ResourceLoadObserver) \
M(ResourceLoadStatistics) \
+ M(ScrollSnap) \
M(Scrolling) \
M(ScrollingTree) \
M(ScrollLatching) \
Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (264133 => 264134)
--- trunk/Source/WebCore/platform/ScrollableArea.cpp 2020-07-08 21:33:54 UTC (rev 264133)
+++ trunk/Source/WebCore/platform/ScrollableArea.cpp 2020-07-08 21:40:44 UTC (rev 264134)
@@ -570,14 +570,18 @@
if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
scrollAnimator->updateScrollSnapState();
+ LOG_WITH_STREAM(ScrollSnap, stream << *this << " updateScrollSnapState: isScrollSnapInProgress " << isScrollSnapInProgress());
+
if (isScrollSnapInProgress())
return;
IntPoint currentPosition = scrollPosition();
IntPoint correctedPosition = nearestActiveSnapPoint(currentPosition);
-
- if (correctedPosition != currentPosition)
+
+ if (correctedPosition != currentPosition) {
+ LOG_WITH_STREAM(ScrollSnap, stream << " adjusting position from " << currentPosition << " to " << correctedPosition);
scrollToOffsetWithoutAnimation(correctedPosition);
+ }
}
#else
void ScrollableArea::updateScrollSnapState()
Modified: trunk/Source/WebCore/platform/cocoa/ScrollController.mm (264133 => 264134)
--- trunk/Source/WebCore/platform/cocoa/ScrollController.mm 2020-07-08 21:33:54 UTC (rev 264133)
+++ trunk/Source/WebCore/platform/cocoa/ScrollController.mm 2020-07-08 21:40:44 UTC (rev 264134)
@@ -27,10 +27,12 @@
#import "ScrollController.h"
#import "LayoutSize.h"
+#import "Logging.h"
#import "PlatformWheelEvent.h"
#import "WheelEventTestMonitor.h"
#import <sys/sysctl.h>
#import <sys/time.h>
+#import <wtf/text/TextStream.h>
#if ENABLE(CSS_SCROLL_SNAP)
#import "ScrollSnapAnimatorState.h"
@@ -577,6 +579,23 @@
return WheelEventStatus::Unknown;
}
+#if !LOG_DISABLED
+static TextStream& operator<<(TextStream& ts, WheelEventStatus status)
+{
+ switch (status) {
+ case WheelEventStatus::UserScrollBegin: ts << "UserScrollBegin"; break;
+ case WheelEventStatus::UserScrolling: ts << "UserScrolling"; break;
+ case WheelEventStatus::UserScrollEnd: ts << "UserScrollEnd"; break;
+ case WheelEventStatus::InertialScrollBegin: ts << "InertialScrollBegin"; break;
+ case WheelEventStatus::InertialScrolling: ts << "InertialScrolling"; break;
+ case WheelEventStatus::InertialScrollEnd: ts << "InertialScrollEnd"; break;
+ case WheelEventStatus::StatelessScrollEvent: ts << "StatelessScrollEvent"; break;
+ case WheelEventStatus::Unknown: ts << "Unknown"; break;
+ }
+ return ts;
+}
+#endif
+
bool ScrollController::shouldOverrideInertialScrolling() const
{
if (!m_scrollSnapState)
@@ -634,6 +653,9 @@
return true;
WheelEventStatus status = toWheelEventStatus(wheelEvent.phase(), wheelEvent.momentumPhase());
+
+ LOG_WITH_STREAM(ScrollSnap, stream << "ScrollController " << this << " processWheelEventForScrollSnap: status " << status);
+
bool isInertialScrolling = false;
switch (status) {
case WheelEventStatus::UserScrollBegin:
@@ -672,6 +694,8 @@
if (m_scrollSnapTimer)
return;
+ LOG_WITH_STREAM(ScrollSnap, stream << "ScrollController " << this << " startScrollSnapTimer (main thread " << isMainThread() << ")");
+
startDeferringWheelEventTestCompletionDueToScrollSnapping();
m_client.willStartScrollSnapAnimation();
m_scrollSnapTimer = m_client.createTimer([this] {
@@ -685,6 +709,8 @@
if (!m_scrollSnapTimer)
return;
+ LOG_WITH_STREAM(ScrollSnap, stream << "ScrollController " << this << " stopScrollSnapTimer (main thread " << isMainThread() << ")");
+
stopDeferringWheelEventTestCompletionDueToScrollSnapping();
m_client.didStopScrollSnapAnimation();
@@ -702,6 +728,9 @@
bool isAnimationComplete;
auto animationOffset = m_scrollSnapState->currentAnimatedScrollOffset(isAnimationComplete);
auto currentOffset = m_client.scrollOffset();
+
+ LOG_WITH_STREAM(ScrollSnap, stream << "ScrollController " << this << " scrollSnapTimerFired - isAnimationComplete " << isAnimationComplete << " currentOffset " << currentOffset << " (main thread " << isMainThread() << ")");
+
m_client.immediateScrollByWithoutContentEdgeConstraints(FloatSize(animationOffset.x() - currentOffset.x(), animationOffset.y() - currentOffset.y()));
if (isAnimationComplete) {
m_scrollSnapState->transitionToDestinationReachedState();
@@ -727,6 +756,8 @@
updateScrollSnapPoints(ScrollEventAxis::Vertical, *snapOffsets, { });
} else
updateScrollSnapPoints(ScrollEventAxis::Vertical, { }, { });
+
+ LOG_WITH_STREAM(ScrollSnap, stream << "ScrollController " << this << " updateScrollSnapState for " << scrollableArea << " new state " << ValueOrNull(m_scrollSnapState.get()));
}
void ScrollController::updateScrollSnapPoints(ScrollEventAxis axis, const Vector<LayoutUnit>& snapPoints, const Vector<ScrollOffsetRange<LayoutUnit>>& snapRanges)
Modified: trunk/Source/WebCore/platform/cocoa/ScrollSnapAnimatorState.h (264133 => 264134)
--- trunk/Source/WebCore/platform/cocoa/ScrollSnapAnimatorState.h 2020-07-08 21:33:54 UTC (rev 264133)
+++ trunk/Source/WebCore/platform/cocoa/ScrollSnapAnimatorState.h 2020-07-08 21:40:44 UTC (rev 264134)
@@ -23,8 +23,7 @@
* THE POSSIBILITY OF SUCH DAMAGE.
*/
-#ifndef ScrollSnapAnimatorState_h
-#define ScrollSnapAnimatorState_h
+#pragma once
#if ENABLE(CSS_SCROLL_SNAP)
@@ -38,6 +37,10 @@
#include "ScrollingMomentumCalculator.h"
#include <wtf/MonotonicTime.h>
+namespace WTF {
+class TextStream;
+}
+
namespace WebCore {
enum class ScrollSnapState {
@@ -112,8 +115,8 @@
std::unique_ptr<ScrollingMomentumCalculator> m_momentumCalculator;
};
+WTF::TextStream& operator<<(WTF::TextStream&, const ScrollSnapAnimatorState&);
+
} // namespace WebCore
#endif // ENABLE(CSS_SCROLL_SNAP)
-
-#endif // ScrollSnapAnimatorState_h
Modified: trunk/Source/WebCore/platform/cocoa/ScrollSnapAnimatorState.mm (264133 => 264134)
--- trunk/Source/WebCore/platform/cocoa/ScrollSnapAnimatorState.mm 2020-07-08 21:33:54 UTC (rev 264133)
+++ trunk/Source/WebCore/platform/cocoa/ScrollSnapAnimatorState.mm 2020-07-08 21:40:44 UTC (rev 264134)
@@ -29,6 +29,7 @@
#if ENABLE(CSS_SCROLL_SNAP)
#import <wtf/MathExtras.h>
+#import <wtf/text/TextStream.h>
namespace WebCore {
@@ -103,7 +104,30 @@
targetOffset = clampTo<float>(targetOffset, minimumTargetOffset, maximumTargetOffset);
return pageScale * targetOffset;
}
-
+
+template<typename T>
+TextStream& operator<<(TextStream& ts, const ScrollOffsetRange<T>& range)
+{
+ ts << "start: " << range.start << " end: " << range.end;
+ return ts;
+}
+
+TextStream& operator<<(TextStream& ts, const ScrollSnapAnimatorState& state)
+{
+ ts << "ScrollSnapAnimatorState";
+ ts.dumpProperty("snap offsets x", state.snapOffsetsForAxis(ScrollEventAxis::Horizontal));
+ ts.dumpProperty("snap offsets y", state.snapOffsetsForAxis(ScrollEventAxis::Vertical));
+ if (!state.snapOffsetRangesForAxis(ScrollEventAxis::Horizontal).isEmpty())
+ ts.dumpProperty("snap offsets ranges x", state.snapOffsetRangesForAxis(ScrollEventAxis::Horizontal));
+ if (!state.snapOffsetRangesForAxis(ScrollEventAxis::Vertical).isEmpty())
+ ts.dumpProperty("snap offsets ranges y", state.snapOffsetRangesForAxis(ScrollEventAxis::Vertical));
+
+ ts.dumpProperty("active snap index x", state.activeSnapIndexForAxis(ScrollEventAxis::Horizontal));
+ ts.dumpProperty("active snap index y", state.activeSnapIndexForAxis(ScrollEventAxis::Vertical));
+
+ return ts;
+}
+
} // namespace WebCore
#endif // CSS_SCROLL_SNAP
