Use one throttle for scrollbar updates, without per-line allocations

Scroll position changes reached the XAML scrollbar through two stacked
8 ms ThrottledFuncs: ControlCore's (added for out-of-proc content in
#10187) and TermControl's (#4608). ControlCore also allocated a
ScrollPositionChangedArgs for every notification, which is one per line
feed while the buffer fills.

ControlCore's throttle is now the only one. It queues three ints and
creates the event args once per callback, on the UI thread, where
TermControl applies them directly.

UserScrollViewport queues its update through the same throttle, under
the terminal lock like output does, so the newest position wins.
ScrollToMark relies on that instead of queuing a second update, which
carried the requested row rather than the viewport it scrolled to
(clamped on the last page). ControlInteractivity no longer raises its
own ScrollPositionChanged. That also stops auto-scroll, which runs on a
render timer since #20710, from calling TermControl's handler (and
XAML) off the UI thread.

User input still takes priority over output: instead of ModifyPending,
TermControl leaves the scrollbar value alone when it already rounds to
the target row, so a dragged thumb doesn't snap.

In a 100k-line burst (Debug ARM64), ScrollPositionChangedArgs
allocations went from 9002 to about 5, and the scrollbar now trails the
oldest unshown scroll notification by about 16 ms on average, down from
about 24 ms.

Add tests that drive the real throttle and that check ScrollToMark
raises one ScrollPositionChanged, for the viewport it scrolled to. The
interactivity tests now check that wheel scrolls arrive through the
core's event.

src/cascadia/TerminalControl/ControlCore.cpp

@@ -242,18 +242,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             });
 
         // Scrollbar updates are also expensive (XAML), so we'll throttle them as well.
-        shared->updateScrollBar = std::make_shared<ThrottledFunc<Control::ScrollPositionChangedArgs>>(
+        // The callback runs on _dispatcher (the UI thread), so handlers may update XAML directly.
+        shared->updateScrollBar = std::make_shared<ThrottledFunc<int, int, int>>(
             _dispatcher,
             til::throttled_func_options{
                 .delay = std::chrono::milliseconds{ 8 },
                 .trailing = true,
             },
-            [weakThis = get_weak()](const auto& update) {
+            [weakThis = get_weak()](const int viewTop, const int viewHeight, const int bufferSize) {
                 if (auto core{ weakThis.get() }; core && !core->_IsClosing())
                 {
                     // GH#20219: re-evaluate if we're hovering over a hyperlink after scrolling
                     core->_refreshHoveredCell();
-                    core->ScrollPositionChanged.raise(*core, update);
+                    core->ScrollPositionChanged.raise(*core, winrt::make<ScrollPositionChangedArgs>(viewTop, viewHeight, bufferSize));
                 }
             });
     }
@@ -758,6 +759,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             //      itself - it was initiated by the mouse wheel, or the scrollbar.
             const auto lock = _terminal->LockForWriting();
             _terminal->UserScrollViewport(viewTop);
+
+            // Terminal::UserScrollViewport doesn't notify us, so update the scrollbar here.
+            // Do it under the lock, like output does, so that the newest position wins.
+            const auto visible = _terminal->GetViewport();
+            _terminalScrollPositionChanged(visible.Top(), visible.Height(), _terminal->GetBufferHeight());
         }
 
         // GH#20219: re-evaluate if we're hovering over a hyperlink after scrolling
@@ -1678,21 +1684,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             return;
         }
 
-        // Start the throttled update of our scrollbar.
-        auto update{ winrt::make<ScrollPositionChangedArgs>(viewTop,
-                                                            viewHeight,
-                                                            bufferSize) };
-
         if (_inUnitTests) [[unlikely]]
         {
-            ScrollPositionChanged.raise(*this, update);
+            ScrollPositionChanged.raise(*this, winrt::make<ScrollPositionChangedArgs>(viewTop, viewHeight, bufferSize));
         }
         else
         {
+            // This runs for every line of output: queue plain values, not event args.
             const auto shared = _shared.lock_shared();
             if (shared->updateScrollBar)
             {
-                shared->updateScrollBar->Run(update);
+                shared->updateScrollBar->Run(viewTop, viewHeight, bufferSize);
             }
         }
     }
@@ -2721,27 +2723,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         }
 
-        const auto viewHeight = ViewportSize().Height;
-        const auto bufferSize = BufferHeight();
-
-        // UserScrollViewport, to update the Terminal about where the viewport should be
-        // then raise a _terminalScrollPositionChanged to inform the control to update the scrollbar.
+        // UserScrollViewport also updates the scrollbar.
         if (tgt.has_value())
         {
             UserScrollViewport(tgt->start.y);
-            _terminalScrollPositionChanged(tgt->start.y, viewHeight, bufferSize);
         }
         else
         {
             if (direction == ScrollToMarkDirection::Last || direction == ScrollToMarkDirection::Next)
             {
                 UserScrollViewport(BufferHeight());
-                _terminalScrollPositionChanged(BufferHeight(), viewHeight, bufferSize);
             }
             else if (direction == ScrollToMarkDirection::First || direction == ScrollToMarkDirection::Previous)
             {
                 UserScrollViewport(0);
-                _terminalScrollPositionChanged(0, viewHeight, bufferSize);
             }
         }
     }

src/cascadia/TerminalControl/ControlCore.h

@@ -313,7 +313,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         {
             std::unique_ptr<til::throttled_func<>> outputIdle;
             std::unique_ptr<til::throttled_func<bool>> focusChanged;
-            std::shared_ptr<ThrottledFunc<Control::ScrollPositionChangedArgs>> updateScrollBar;
+            // viewTop, viewHeight, bufferSize
+            std::shared_ptr<ThrottledFunc<int, int, int>> updateScrollBar;
         };
 
         void _setupDispatcherAndCallbacks();

src/cascadia/TerminalControl/ControlInteractivity.cpp

@@ -513,8 +513,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 const auto currentOffset = _core->ScrollOffset();
                 const auto newValue = numRows + currentOffset;
 
-                // Update the Core's viewport position, and raise a
-                // ScrollPositionChanged event to update the scrollbar
+                // Update the Core's viewport position, which also
+                // updates the scrollbar
                 UpdateScrollbar(newValue);
 
                 // Use this point as our new scroll anchor.
@@ -690,8 +690,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         const auto rowsToScroll{ _rowsToScroll == WHEEL_PAGESCROLL ? _core->ViewportSize().Height : _rowsToScroll };
         const auto newValue = rowsToScroll * rowDelta + currentOffset;
 
-        // Update the Core's viewport position, and raise a
-        // ScrollPositionChanged event to update the scrollbar
+        // Update the Core's viewport position, which also
+        // updates the scrollbar
         UpdateScrollbar(newValue);
 
         if (isLeftButtonPressed)
@@ -705,15 +705,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // Method Description:
     // - Update the scroll position in such a way that should update the
     //   scrollbar. For example, when scrolling the buffer with the mouse or
-    //   touch input. This will both update the Core's Terminal's buffer
-    //   location, then also raise our own ScrollPositionChanged event.
-    //   UserScrollViewport _won't_ raise the core's ScrollPositionChanged
-    //   event, because it's assumed that's already being called from a context
-    //   that knows about the change to the scrollbar. So we need to raise the
-    //   event on our own.
-    // - The hosting control should make sure to listen to our own
-    //   ScrollPositionChanged event and use that as an opportunity to update
-    //   the location of the scrollbar.
+    //   touch input. This scrolls the Core, which updates the scrollbar through
+    //   its throttled ScrollPositionChanged event.
+    // - May be called off the UI thread (auto-scroll runs on a render timer).
     // Arguments:
     // - newValue: The new top of the viewport
     // Return Value:
@@ -726,18 +720,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _internalScrollbarPosition = std::clamp(newValue, 0.0f, static_cast<float>(_core->BufferHeight()));
 
         // If the new scrollbar position, rounded to an int, is at a different
-        // row, then actually update the scroll position in the core, and raise
-        // a ScrollPositionChanged to inform the control.
+        // row, then actually update the scroll position in the core.
         const auto viewTop = std::lround(_internalScrollbarPosition);
         if (viewTop != _core->ScrollOffset())
         {
             _core->UserScrollViewport(viewTop);
-
-            // _core->ScrollOffset() is now set to newValue
-            ScrollPositionChanged.raise(*this,
-                                        winrt::make<ScrollPositionChangedArgs>(_core->ScrollOffset(),
-                                                                               _core->ViewportSize().Height,
-                                                                               _core->BufferHeight()));
         }
     }
 

src/cascadia/TerminalControl/ControlInteractivity.h

@@ -93,7 +93,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         til::typed_event<IInspectable, Control::OpenHyperlinkEventArgs> OpenHyperlink;
         til::typed_event<IInspectable, Control::PasteFromClipboardEventArgs> PasteFromClipboard;
-        til::typed_event<IInspectable, Control::ScrollPositionChangedArgs> ScrollPositionChanged;
         til::typed_event<IInspectable, Control::ContextMenuRequestedEventArgs> ContextMenuRequested;
 
         til::typed_event<IInspectable, IInspectable> Closed;

src/cascadia/TerminalControl/TermControl.cpp

@@ -27,10 +27,6 @@ using namespace winrt::Windows::System;
 using namespace winrt::Windows::ApplicationModel::DataTransfer;
 using namespace winrt::Windows::Storage::Streams;
 
-// The minimum delay between updates to the scroll bar's values.
-// The updates are throttled to limit power usage.
-constexpr const auto ScrollBarUpdateInterval = std::chrono::milliseconds(8);
-
 // The minimum delay between updating the TSF input control.
 // This is already throttled primarily in the ControlCore, with a timeout of 100ms. We're adding another smaller one here, as the (potentially x-proc) call will come in off the UI thread
 constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(8);
@@ -334,7 +330,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _revokers.UpdateSelectionMarkers = _core->UpdateSelectionMarkers(winrt::auto_revoke, { get_weak(), &TermControl::_updateSelectionMarkers });
         _revokers.coreOpenHyperlink = _core->OpenHyperlink(winrt::auto_revoke, { get_weak(), &TermControl::_HyperlinkHandler });
         _revokers.interactivityOpenHyperlink = _interactivity->OpenHyperlink(winrt::auto_revoke, { get_weak(), &TermControl::_HyperlinkHandler });
-        _revokers.interactivityScrollPositionChanged = _interactivity->ScrollPositionChanged(winrt::auto_revoke, { get_weak(), &TermControl::_ScrollPositionChanged });
         _revokers.ContextMenuRequested = _interactivity->ContextMenuRequested(winrt::auto_revoke, { get_weak(), &TermControl::_contextMenuHandler });
 
         // "Bubbled" events - ones we want to handle, by raising our own event.
@@ -375,7 +370,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // TermControl::Dispatcher().
         auto dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread();
 
-        // These three throttled functions are triggered by terminal output and interact with the UI.
+        // This throttled function is triggered by terminal output and interacts with the UI.
         // Since Close() is the point after which we are removed from the UI, but before the
         // destructor has run, we MUST check control->_IsClosing() before actually doing anything.
         _playWarningBell = std::make_shared<ThrottledFunc<>>(
@@ -391,25 +386,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 }
             });
 
-        _updateScrollBar = std::make_shared<ThrottledFunc<ScrollBarUpdate>>(
-            dispatcher,
-            til::throttled_func_options{
-                .delay = ScrollBarUpdateInterval,
-                .trailing = true,
-            },
-            [weakThis = get_weak()](const auto& update) {
-                if (auto control{ weakThis.get() }; control && !control->_IsClosing())
-                {
-                    control->_throttledUpdateScrollbar(update);
-                }
-            });
-
         // These events might all be triggered by the connection, but that
         // should be drained and closed before we complete destruction. So these
         // are safe.
         //
-        // NOTE: _ScrollPositionChanged has to be registered after we set up the
-        // _updateScrollBar func. Otherwise, we could get a callback from an
+        // NOTE: WarningBell has to be registered after we set up the
+        // _playWarningBell func. Otherwise, we could get a callback from an
         // attached content before we set up the throttled func, and that'll A/V
         _revokers.coreScrollPositionChanged = _core->ScrollPositionChanged(winrt::auto_revoke, { get_weak(), &TermControl::_ScrollPositionChanged });
         _revokers.WarningBell = _core->WarningBell(winrt::auto_revoke, { get_weak(), &TermControl::_coreWarningBell });
@@ -553,7 +535,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _core->HardResetWithoutErase();
     }
 
-    void TermControl::_throttledUpdateScrollbar(const ScrollBarUpdate& update)
+    void TermControl::_applyScrollBarUpdate(const ScrollBarUpdate& update)
     {
         if (!_initializedTerminal)
         {
@@ -567,7 +549,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _isInternalScrollBarUpdate = true;
 
         auto scrollBar = ScrollBar();
-        if (update.newValue)
+        // Don't snap a dragged thumb onto the row it's already on: its value is
+        // fractional, and the core reports the row that value rounds to.
+        if (update.newValue && std::lround(scrollBar.Value()) != std::lround(*update.newValue))
         {
             scrollBar.Value(*update.newValue);
         }
@@ -839,7 +823,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                 .newMinimum = scrollBar.Minimum(),
                 .newViewportSize = scrollBar.ViewportSize(),
             };
-            _updateScrollBar->Run(update);
+            _applyScrollBarUpdate(update);
         }
 
         // Set focus back to terminal control
@@ -2187,12 +2171,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         const auto newValue = args.NewValue();
         _interactivity->UpdateScrollbar(static_cast<float>(newValue));
-
-        // User input takes priority over terminal events so cancel
-        // any pending scroll bar update if the user scrolls.
-        _updateScrollBar->ModifyPending([](auto& update) {
-            update.newValue.reset();
-        });
     }
 
     // Method Description:
@@ -2423,6 +2401,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     void TermControl::_ScrollPositionChanged(const IInspectable& /*sender*/,
                                              const Control::ScrollPositionChangedArgs& args)
     {
+        // ControlCore raises this from its scrollbar throttle, on our UI thread.
+        if (_IsClosing())
+        {
+            return;
+        }
+
         ScrollBarUpdate update;
         const auto hiddenContent = args.BufferSize() - args.ViewHeight();
         update.newMaximum = hiddenContent;
@@ -2430,7 +2414,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         update.newViewportSize = args.ViewHeight();
         update.newValue = args.ViewTop();
 
-        _updateScrollBar->Run(update);
+        _applyScrollBarUpdate(update);
 
         // If we have a selection with markers (exposed via selection mode),
         //   update the position of the markers
@@ -3725,7 +3709,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                     .newMinimum = scrollBar.Minimum(),
                     .newViewportSize = scrollBar.ViewportSize(),
                 };
-                _updateScrollBar->Run(update);
+                _applyScrollBarUpdate(update);
             }
         }
 

src/cascadia/TerminalControl/TermControl.h

@@ -305,8 +305,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             double newViewportSize;
         };
 
-        std::shared_ptr<ThrottledFunc<ScrollBarUpdate>> _updateScrollBar;
-
         bool _isInternalScrollBarUpdate;
 
         winrt::Windows::UI::Composition::ScalarKeyFrameAnimation _bellLightAnimation{ nullptr };
@@ -424,7 +422,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void _coreOutputIdle(const IInspectable& sender, const IInspectable& args);
 
         winrt::Windows::Foundation::Point _toPosInDips(const Core::Point terminalCellPos);
-        void _throttledUpdateScrollbar(const ScrollBarUpdate& update);
+        void _applyScrollBarUpdate(const ScrollBarUpdate& update);
 
         void _pasteTextWithBroadcast(const winrt::hstring& text);
 
@@ -475,7 +473,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             Windows::UI::ViewManagement::AccessibilitySettings::HighContrastChanged_revoker HighContrastChanged;
 
             til::event_revoker interactivityOpenHyperlink;
-            til::event_revoker interactivityScrollPositionChanged;
             til::event_revoker PasteFromClipboard;
             til::event_revoker ContextMenuRequested;
         } _revokers{};

src/cascadia/UnitTests_Control/ControlCoreTests.cpp

@@ -48,6 +48,9 @@ namespace ControlUnitTests
 
         TEST_METHOD(TestSimpleClickSelection);
 
+        TEST_METHOD(TestScrollPositionChangedIsThrottled);
+        TEST_METHOD(TestScrollToMarkRaisesVisibleViewport);
+
         TEST_CLASS_SETUP(ModuleSetup)
         {
             winrt::init_apartment(winrt::apartment_type::single_threaded);
@@ -810,4 +813,117 @@ namespace ControlUnitTests
         }
         VERIFY_IS_TRUE(gotSelectionUpdate);
     }
+
+    void ControlCoreTests::TestScrollPositionChangedIsThrottled()
+    {
+        // Declared before the core, so that they outlive it.
+        std::atomic<int> callbacks{ 0 };
+        std::atomic<bool> raisedOffDispatcher{ false };
+        std::atomic<int> lastViewTop{ -1 };
+        std::atomic<int> lastBufferSize{ -1 };
+
+        auto [settings, conn] = _createSettingsAndConnection();
+        auto core = createCore(*settings, *conn);
+        VERIFY_IS_NOT_NULL(core);
+        _standardInit(core);
+
+        core->ScrollPositionChanged([&, dispatcher = core->_dispatcher](auto&&, const Control::ScrollPositionChangedArgs& args) {
+            ++callbacks;
+            if (!dispatcher.HasThreadAccess())
+            {
+                raisedOffDispatcher = true;
+            }
+            lastViewTop = args.ViewTop();
+            // Stored last: once the test thread sees it, it sees the values above too.
+            lastBufferSize = args.BufferSize();
+        });
+
+        // _inUnitTests bypasses the throttle. Turn it off to test the real one.
+        core->_inUnitTests = false;
+
+        Log::Comment(L"Print 100 lines in a single write. The last 81 line feeds scroll the viewport.");
+        std::wstring output;
+        for (auto i = 0; i < 100; ++i)
+        {
+            output.append(L"Foo\r\n");
+        }
+        conn->WriteInput(winrt_wstring_to_array_view(output));
+
+        const auto expectedViewTop = core->ScrollOffset();
+        const auto expectedBufferSize = core->BufferHeight();
+        VERIFY_ARE_EQUAL(81, expectedViewTop);
+        VERIFY_ARE_EQUAL(101, expectedBufferSize);
+
+        Log::Comment(L"Wait for the throttled update with the final position");
+        const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds{ 5 };
+        while (lastBufferSize.load() != expectedBufferSize && std::chrono::steady_clock::now() < deadline)
+        {
+            Sleep(1);
+        }
+
+        // That was the last update, so no callback is pending anymore.
+        // Restore the flag; ~ControlCore relies on it.
+        core->_inUnitTests = true;
+
+        VERIFY_ARE_EQUAL(expectedBufferSize, lastBufferSize.load());
+        VERIFY_ARE_EQUAL(expectedViewTop, lastViewTop.load());
+        VERIFY_IS_FALSE(raisedOffDispatcher.load(), L"ScrollPositionChanged must be raised on the core's dispatcher");
+
+        Log::Comment(NoThrowString().Format(L"81 scroll notifications resulted in %d ScrollPositionChanged events", callbacks.load()));
+        VERIFY_IS_GREATER_THAN_OR_EQUAL(callbacks.load(), 1);
+        VERIFY_IS_LESS_THAN(callbacks.load(), 81);
+    }
+
+    void ControlCoreTests::TestScrollToMarkRaisesVisibleViewport()
+    {
+        // Declared before the core, so that they outlive it.
+        auto events = 0;
+        auto lastViewTop = -1;
+        auto lastViewHeight = -1;
+        auto lastBufferSize = -1;
+
+        auto [settings, conn] = _createSettingsAndConnection();
+        auto core = createCore(*settings, *conn);
+        VERIFY_IS_NOT_NULL(core);
+        _standardInit(core);
+
+        Log::Comment(L"Print 100 lines, with prompt marks on rows 10 and 95");
+        for (auto i = 0; i < 100; ++i)
+        {
+            if (i == 10 || i == 95)
+            {
+                conn->WriteInput(winrt_wstring_to_array_view(L"\x1b]133;A\x7"));
+            }
+            conn->WriteInput(winrt_wstring_to_array_view(L"Foo\r\n"));
+        }
+        VERIFY_ARE_EQUAL(81, core->ScrollOffset());
+        VERIFY_ARE_EQUAL(101, core->BufferHeight());
+
+        core->ScrollPositionChanged([&](auto&&, const Control::ScrollPositionChangedArgs& args) {
+            ++events;
+            lastViewTop = args.ViewTop();
+            lastViewHeight = args.ViewHeight();
+            lastBufferSize = args.BufferSize();
+        });
+
+        // Expect one ScrollPositionChanged per call, for the (possibly clamped) viewport it scrolled to.
+        const auto scrollToMark = [&](const Control::ScrollToMarkDirection direction, const int expectedTop) {
+            events = 0;
+            core->ScrollToMark(direction);
+            VERIFY_ARE_EQUAL(expectedTop, core->ScrollOffset());
+            VERIFY_ARE_EQUAL(1, events);
+            VERIFY_ARE_EQUAL(expectedTop, lastViewTop);
+            VERIFY_ARE_EQUAL(20, lastViewHeight);
+            VERIFY_ARE_EQUAL(101, lastBufferSize);
+        };
+
+        Log::Comment(L"Previous: from the bottom (row 81) to the mark on row 10");
+        scrollToMark(Control::ScrollToMarkDirection::Previous, 10);
+        Log::Comment(L"Previous: there's no mark above row 10, so scroll to the top");
+        scrollToMark(Control::ScrollToMarkDirection::Previous, 0);
+        Log::Comment(L"Next: back to the mark on row 10");
+        scrollToMark(Control::ScrollToMarkDirection::Next, 10);
+        Log::Comment(L"Next: the mark on row 95 is on the last page, so the viewport stops at row 81");
+        scrollToMark(Control::ScrollToMarkDirection::Next, 81);
+    }
 }

src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp

@@ -213,13 +213,15 @@ namespace ControlUnitTests
         auto expectedViewHeight = 20;
         auto expectedBufferHeight = 20;
 
+        auto scrollChangedCount = 0;
         auto scrollChangedHandler = [&](auto&&, const Control::ScrollPositionChangedArgs& args) mutable {
+            ++scrollChangedCount;
             VERIFY_ARE_EQUAL(expectedTop, args.ViewTop());
             VERIFY_ARE_EQUAL(expectedViewHeight, args.ViewHeight());
             VERIFY_ARE_EQUAL(expectedBufferHeight, args.BufferSize());
         };
+        // Output and wheel scrolls both raise the core's ScrollPositionChanged.
         core->ScrollPositionChanged(scrollChangedHandler);
-        interactivity->ScrollPositionChanged(scrollChangedHandler);
 
         for (auto i = 0; i < 40; ++i)
         {
@@ -244,11 +246,13 @@ namespace ControlUnitTests
         const auto modifiers = ControlKeyStates();
         expectedBufferHeight = 41;
         expectedTop = 20;
+        scrollChangedCount = 0;
 
         interactivity->MouseWheel(modifiers,
                                   Core::Point{ 0, WHEEL_DELTA },
                                   Core::Point{ 0, 0 },
                                   buttonState);
+        VERIFY_ARE_EQUAL(1, scrollChangedCount);
 
         Log::Comment(L"Scroll up 19 more times, to the top");
         for (auto i = 0; i < 20; ++i)
@@ -259,6 +263,7 @@ namespace ControlUnitTests
                                       Core::Point{ 0, 0 },
                                       buttonState);
         }
+        VERIFY_ARE_EQUAL(21, scrollChangedCount);
         Log::Comment(L"Scrolling up more should do nothing");
         expectedTop = 0;
         interactivity->MouseWheel(modifiers,
@@ -269,6 +274,7 @@ namespace ControlUnitTests
                                   Core::Point{ 0, WHEEL_DELTA },
                                   Core::Point{ 0, 0 },
                                   buttonState);
+        VERIFY_ARE_EQUAL(21, scrollChangedCount);
 
         Log::Comment(L"Scroll down 21 more times, to the bottom");
         for (auto i = 0; i < 21; ++i)
@@ -281,6 +287,7 @@ namespace ControlUnitTests
                                       buttonState);
             Log::Comment(NoThrowString().Format(L"internal scrollbar pos:%f", interactivity->_internalScrollbarPosition));
         }
+        VERIFY_ARE_EQUAL(42, scrollChangedCount);
         Log::Comment(L"Scrolling down more should do nothing");
         expectedTop = 21;
         interactivity->MouseWheel(modifiers,
@@ -700,7 +707,6 @@ namespace ControlUnitTests
             VERIFY_ARE_EQUAL(expectedBufferHeight, args.BufferSize());
         };
         core->ScrollPositionChanged(scrollChangedHandler);
-        interactivity->ScrollPositionChanged(scrollChangedHandler);
 
         for (auto i = 0; i < 40; ++i)
         {