Changeset 286973 in webkit

Timestamp:

Dec 13, 2021, 12:56:48 PM (5 years ago)

Author:

Alan Coon

Message:

Cherry-pick r286900. rdar://problem/86344954

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.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@286900 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:

branches/safari-612-branch/Source/WebKit

Files:

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

branches/safari-612-branch/Source/WebKit/ChangeLog

@@ +1 @@
+2021-12-13  Alan Coon  <alancoon@apple.com>
+
+        Cherry-pick r286900. rdar://problem/86344954
+
+    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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@286900 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-09  Alan Coon  <alancoon@apple.com>
 

branches/safari-612-branch/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);
 }
 

branches/safari-612-branch/Source/WebKit/UIProcess/WebPageProxy.cpp

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