Changeset 286900 in webkit

Timestamp:

Dec 10, 2021, 7:43:48 PM (5 years ago)

Author:

timothy_horton@apple.com

Message:

Momentum Event Dispatcher: Magic Mouse doesn't use momentum event dispatcher
​https://bugs.webkit.org/show_bug.cgi?id=234189
<rdar://problem/86344954>

Reviewed by Simon Fraser.

• Shared/mac/ScrollingAccelerationCurveMac.mm:

(WebKit::fromIOHIDDevice):
Fix the FIXME here about the additional fallback values; it turns out
Magic Mouse is one device that does not have a value for
kIOHIDScrollAccelerationTypeKey, so we need the full fallback chain to support it.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::sendWheelEvent):
Un-wrapping this optional results in losing the engaged state, and sending
a garbage ScrollingAccelerationCurve across the wire.
The message argument is also an optional, so just pass it along.

The result of this bug was that if you had ever used a device with a curve
for a given page, and then used a device with no curve, MomentumEventDispatcher
would have a garbage curve (from this message trying to "unset" the optional),
and a garbage curve results in chaotic scrolling.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Shared/mac/ScrollingAccelerationCurveMac.mm (modified) (2 diffs)
• UIProcess/WebPageProxy.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-12-10  Tim Horton  <timothy_horton@apple.com>
+
+        Momentum Event Dispatcher: Magic Mouse doesn't use momentum event dispatcher
+        https://bugs.webkit.org/show_bug.cgi?id=234189
+        <rdar://problem/86344954>
+
+        Reviewed by Simon Fraser.
+
+        * Shared/mac/ScrollingAccelerationCurveMac.mm:
+        (WebKit::fromIOHIDDevice):
+        Fix the FIXME here about the additional fallback values; it turns out
+        Magic Mouse is one device that does not have a value for
+        kIOHIDScrollAccelerationTypeKey, so we need the full fallback chain to support it.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::sendWheelEvent):
+        Un-wrapping this optional results in losing the engaged state, and sending
+        a garbage ScrollingAccelerationCurve across the wire.
+        The message argument is also an optional, so just pass it along.
+
+        The result of this bug was that if you had ever used a device with a curve
+        for a given page, and then used a device with no curve, MomentumEventDispatcher
+        would have a garbage curve (from this message trying to "unset" the optional),
+        and a garbage curve results in chaotic scrolling.
+
 2021-12-10  Michael Saboff  <msaboff@apple.com>
 

trunk/Source/WebKit/Shared/mac/ScrollingAccelerationCurveMac.mm

@@ -117 +117 @@
     }
 
-    // FIXME: There is some additional fallback to implement here, though this seems usually sufficient.
-    auto scrollAccelerationType = adoptCF(dynamic_cf_cast<CFStringRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), CFSTR("HIDScrollAccelerationType"))));
-    if (!scrollAccelerationType) {
-        RELEASE_LOG(ScrollAnimations, "ScrollingAccelerationCurve::fromIOHIDDevice failed to look up acceleration type");
+    auto readFixedPointServiceKey = [&] (CFStringRef key) -> std::optional<float> {
+        auto valueCF = adoptCF(dynamic_cf_cast<CFNumberRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), key)));
+        if (!valueCF)
+            return std::nullopt;
+        return fromFixedPoint([(NSNumber *)valueCF.get() floatValue]);
+    };
+
+    auto scrollAcceleration = [&] () -> std::optional<float> {
+        if (auto scrollAccelerationType = adoptCF(dynamic_cf_cast<CFStringRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), CFSTR("HIDScrollAccelerationType"))))) {
+            if (auto acceleration = readFixedPointServiceKey(scrollAccelerationType.get()))
+                return acceleration;
+        }
+
+        if (auto acceleration = readFixedPointServiceKey(CFSTR(kIOHIDMouseScrollAccelerationKey)))
+            return acceleration;
+
+        if (auto acceleration = readFixedPointServiceKey(CFSTR(kIOHIDScrollAccelerationKey)))
+            return acceleration;
+
+        return std::nullopt;
+    }();
+    if (!scrollAcceleration) {
+        RELEASE_LOG(ScrollAnimations, "ScrollingAccelerationCurve::fromIOHIDDevice failed to look up acceleration");
         return std::nullopt;
     }
 
-    auto scrollAccelerationCF = adoptCF(dynamic_cf_cast<CFNumberRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), scrollAccelerationType.get())));
-    if (!scrollAccelerationCF) {
-        RELEASE_LOG(ScrollAnimations, "ScrollingAccelerationCurve::fromIOHIDDevice failed to look up acceleration value");
-        return std::nullopt;
-    }
-    auto scrollAcceleration = fromFixedPoint(fromCFNumber(scrollAccelerationCF.get()));
-
-    auto resolutionCF = adoptCF(dynamic_cf_cast<CFNumberRef>(IOHIDServiceClientCopyProperty(ioHIDService.get(), CFSTR(kIOHIDScrollResolutionKey))));
-    if (!resolutionCF) {
+    auto resolution = readFixedPointServiceKey(CFSTR(kIOHIDScrollResolutionKey));
+    if (!resolution) {
         RELEASE_LOG(ScrollAnimations, "ScrollingAccelerationCurve::fromIOHIDDevice failed to look up resolution");
         return std::nullopt;
     }
-    auto resolution = fromFixedPoint([(NSNumber *)resolutionCF.get() floatValue]);
 
     static CFStringRef dispatchFrameRateKey = CFSTR("ScrollMomentumDispatchRate");
@@ -143 +154 @@
     float frameRate = frameRateCF ? fromCFNumber(frameRateCF.get()) : defaultDispatchFrameRate;
 
-    return fromIOHIDCurveArrayWithAcceleration((NSArray *)curves.get(), scrollAcceleration, resolution, frameRate);
+    return fromIOHIDCurveArrayWithAcceleration((NSArray *)curves.get(), *scrollAcceleration, *resolution, frameRate);
 }
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -2963 +2963 @@
 #if ENABLE(MOMENTUM_EVENT_DISPATCHER)
     if (event.momentumPhase() == WebWheelEvent::PhaseBegan && m_scrollingAccelerationCurve != m_lastSentScrollingAccelerationCurve) {
-        connection->send(Messages::EventDispatcher::SetScrollingAccelerationCurve(m_webPageID, *m_scrollingAccelerationCurve), 0, { }, Thread::QOS::UserInteractive);
+        connection->send(Messages::EventDispatcher::SetScrollingAccelerationCurve(m_webPageID, m_scrollingAccelerationCurve), 0, { }, Thread::QOS::UserInteractive);
         m_lastSentScrollingAccelerationCurve = m_scrollingAccelerationCurve;
     }