Remove MouseDownThread and its usages

This code comes from an old Be Newsletter and since then the API
received the addition of SetMouseEventMask. In several places the
MouseDownThread was misused: it would spawn a new thread on every mouse
click and not clear the previous one. This could for example lead to
BSpinner skipping values if you clicked it at the right speed.

There are functional changes in BSpinner, before it updated for the
first time 100ms after mouse down, and then as you moved the mouse
around the button, now it activates immediately on first click and then
every 200ms (which may be a bit short). In other places, no functional
changes intended.

Change-Id: Ie600dc68cbb87d1e237633953e5189918bf36575
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2599
Reviewed-by: waddlesplash <[email protected]>

headers/os/interface/MenuField.h

@@ -141,9 +141,6 @@ private:
 			float				_MenuBarOffset() const;
 			float				_MenuBarWidth() const;
 
-			void				_DoneTracking(BPoint point);
-			void				_Track(BPoint point, uint32);
-
 private:
 			char*				fLabel;
 			BMenu*				fMenu;

headers/private/shared/Thread.h

@@ -45,46 +45,6 @@ All rights reserved.
 
 namespace BPrivate {
 
-class MessengerAutoLocker {
-	// move this into AutoLock.h
-	public:
-		MessengerAutoLocker(BMessenger* messenger)
-			:	fMessenger(messenger),
-				fHasLock(messenger->LockTarget())
-		{}
-
-		~MessengerAutoLocker()
-		{
-			Unlock();
-		}
-
-		bool operator!() const
-		{
-			return !fHasLock;
-		}
-
-		bool IsLocked() const
-		{
-			return fHasLock;
-		}
-
-		void Unlock()
-		{
-			if (fHasLock) {
-				BLooper* looper;
-				fMessenger->Target(&looper);
-				if (looper)
-					looper->Unlock();
-				fHasLock = false;
-			}
-		}
-
-	private:
-		BMessenger* fMessenger;
-		bool fHasLock;
-};
-
-
 class SimpleThread {
 	// this should only be used as a base class,
 	// subclass needs to add proper locking mechanism
@@ -313,117 +273,6 @@ LaunchInNewThread(const char* name, int32 priority,
 }
 
 
-template<class View>
-class MouseDownThread {
-public:
-	static void TrackMouse(View* view, void (View::*)(BPoint),
-		void (View::*)(BPoint, uint32) = 0,
-		bigtime_t pressingPeriod = 100000);
-
-protected:
-	MouseDownThread(View* view, void (View::*)(BPoint),
-		void (View::*)(BPoint, uint32), bigtime_t pressingPeriod);
-
-	virtual ~MouseDownThread();
-
-	void Go();
-	virtual void Track();
-
-	static status_t TrackBinder(void*);
-
-private:
-	BMessenger fOwner;
-	void (View::*fDonePressing)(BPoint);
-	void (View::*fPressing)(BPoint, uint32);
-	bigtime_t fPressingPeriod;
-	volatile thread_id fThreadID;
-};
-
-
-template<class View>
-void
-MouseDownThread<View>::TrackMouse(View* view,
-	void(View::*donePressing)(BPoint),
-	void(View::*pressing)(BPoint, uint32), bigtime_t pressingPeriod)
-{
-	(new MouseDownThread(view, donePressing, pressing, pressingPeriod))->Go();
-}
-
-
-template<class View>
-MouseDownThread<View>::MouseDownThread(View* view,
-	void (View::*donePressing)(BPoint),
-	void (View::*pressing)(BPoint, uint32), bigtime_t pressingPeriod)
-	:	fOwner(view, view->Window()),
-		fDonePressing(donePressing),
-		fPressing(pressing),
-		fPressingPeriod(pressingPeriod)
-{
-}
-
-
-template<class View>
-MouseDownThread<View>::~MouseDownThread()
-{
-}
-
-
-template<class View>
-void
-MouseDownThread<View>::Go()
-{
-	fThreadID = spawn_thread(&MouseDownThread::TrackBinder,
-		"MouseTrackingThread", B_NORMAL_PRIORITY, this);
-
-	if (fThreadID <= 0 || resume_thread(fThreadID) != B_OK)
-		// didn't start, don't leak self
-		delete this;
-}
-
-
-template<class View>
-status_t
-MouseDownThread<View>::TrackBinder(void* castToThis)
-{
-	MouseDownThread* self = static_cast<MouseDownThread*>(castToThis);
-	self->Track();
-	// dead at this point
-	return B_OK;
-}
-
-
-template<class View>
-void
-MouseDownThread<View>::Track()
-{
-	for (;;) {
-		MessengerAutoLocker lock(&fOwner);
-		if (!lock)
-			break;
-
-		BLooper* looper;
-		View* view = dynamic_cast<View*>(fOwner.Target(&looper));
-		if (!view)
-			break;
-
-		uint32 buttons;
-		BPoint location;
-		view->GetMouse(&location, &buttons, false);
-		if (!buttons) {
-			(view->*fDonePressing)(location);
-			break;
-		}
-		if (fPressing)
-			(view->*fPressing)(location, buttons);
-
-		lock.Unlock();
-		snooze(fPressingPeriod);
-	}
-
-	delete this;
-}
-
-
 } // namespace BPrivate
 
 using namespace BPrivate;

src/apps/deskbar/ExpandoMenuBar.cpp

@@ -324,8 +324,8 @@ TExpandoMenuBar::MouseDown(BPoint where)
 		&& item->ExpanderBounds().Contains(where)) {
 		// start the animation here, finish on mouse up
 		fLastClickedItem = item;
-		MouseDownThread<TExpandoMenuBar>::TrackMouse(this,
+		item->SetArrowDirection(BControlLook::B_RIGHT_DOWN_ARROW);
-			&TExpandoMenuBar::_DoneTracking, &TExpandoMenuBar::_Track);
+		SetMouseEventMask(B_POINTER_EVENTS, B_NO_POINTER_HISTORY);
 		Invalidate(item->ExpanderBounds());
 		return;
 			// absorb the message
@@ -360,6 +360,23 @@ TExpandoMenuBar::MouseMoved(BPoint where, uint32 code, const BMessage* message)
 		// force a cleanup
 		_FinishedDrag();
 
+		if (buttons != 0) {
+			TTeamMenuItem* lastItem
+				= dynamic_cast<TTeamMenuItem*>(fLastClickedItem);
+			if (lastItem != NULL) {
+				if (lastItem->ExpanderBounds().Contains(where))
+					lastItem->SetArrowDirection(
+						BControlLook::B_RIGHT_DOWN_ARROW);
+				else {
+					lastItem->SetArrowDirection(lastItem->IsExpanded()
+						? BControlLook::B_DOWN_ARROW
+						: BControlLook::B_RIGHT_ARROW);
+				}
+			}
+
+			Invalidate(lastItem->ExpanderBounds());
+		}
+
 		switch (code) {
 			case B_INSIDE_VIEW:
 			{
@@ -471,6 +488,15 @@ TExpandoMenuBar::MouseUp(BPoint where)
 			// absorb the message
 	}
 
+	TTeamMenuItem* lastItem = dynamic_cast<TTeamMenuItem*>(fLastClickedItem);
+	if (lastItem != NULL && lastItem->ExpanderBounds().Contains(where)) {
+		lastItem->ToggleExpandState(true);
+		lastItem->SetArrowDirection(lastItem->IsExpanded()
+			? BControlLook::B_DOWN_ARROW
+			: BControlLook::B_RIGHT_ARROW);
+
+		Invalidate(lastItem->ExpanderBounds());
+	}
 	BMenuBar::MouseUp(where);
 }
 
@@ -1101,41 +1127,3 @@ TExpandoMenuBar::_FinishedDrag(bool invoke)
 	if (!invoke && fBarView != NULL && fBarView->Dragging())
 		fBarView->DragStop(true);
 }
-
-
-void
-TExpandoMenuBar::_DoneTracking(BPoint where)
-{
-	TTeamMenuItem* lastItem = dynamic_cast<TTeamMenuItem*>(fLastClickedItem);
-	if (lastItem == NULL)
-		return;
-
-	if (!lastItem->ExpanderBounds().Contains(where))
-		return;
-
-	lastItem->ToggleExpandState(true);
-	lastItem->SetArrowDirection(lastItem->IsExpanded()
-		? BControlLook::B_DOWN_ARROW
-		: BControlLook::B_RIGHT_ARROW);
-
-	Invalidate(lastItem->ExpanderBounds());
-}
-
-
-void
-TExpandoMenuBar::_Track(BPoint where, uint32)
-{
-	TTeamMenuItem* lastItem = dynamic_cast<TTeamMenuItem*>(fLastClickedItem);
-	if (lastItem == NULL)
-		return;
-
-	if (lastItem->ExpanderBounds().Contains(where))
-		lastItem->SetArrowDirection(BControlLook::B_RIGHT_DOWN_ARROW);
-	else {
-		lastItem->SetArrowDirection(lastItem->IsExpanded()
-			? BControlLook::B_DOWN_ARROW
-			: BControlLook::B_RIGHT_ARROW);
-	}
-
-	Invalidate(lastItem->ExpanderBounds());
-}

src/apps/deskbar/ExpandoMenuBar.h

@@ -113,9 +113,6 @@ private:
 
 			void			_FinishedDrag(bool invoke = false);
 
-			void			_DoneTracking(BPoint where);
-			void			_Track(BPoint where, uint32);
-
 			bool			CheckForSizeOverrunVertical();
 			bool			CheckForSizeOverrunHorizontal();
 

src/kits/interface/AbstractSpinner.cpp

@@ -28,14 +28,13 @@
 #include <LayoutUtils.h>
 #include <Message.h>
 #include <MessageFilter.h>
+#include <MessageRunner.h>
 #include <Point.h>
 #include <PropertyInfo.h>
 #include <TextView.h>
 #include <View.h>
 #include <Window.h>
 
-#include "Thread.h"
-
 
 static const float kFrameMargin			= 2.0f;
 
@@ -170,20 +169,18 @@ public:
 	virtual	void				MouseUp(BPoint where);
 	virtual	void				MouseMoved(BPoint where, uint32 transit,
 									const BMessage* message);
+	virtual void				MessageReceived(BMessage* message);
 
 			bool				IsEnabled() const { return fIsEnabled; }
 	virtual	void				SetEnabled(bool enable) { fIsEnabled = enable; };
 
 private:
-			void				_DoneTracking(BPoint where);
-			void				_Track(BPoint where, uint32);
-
 			spinner_direction	fSpinnerDirection;
 			BAbstractSpinner*	fParent;
 			bool				fIsEnabled;
 			bool				fIsMouseDown;
 			bool				fIsMouseOver;
-			bigtime_t			fRepeatDelay;
+			BMessageRunner*		fRepeater;
 };
 
 
@@ -309,13 +306,14 @@ SpinnerButton::SpinnerButton(BRect frame, const char* name,
 	fIsEnabled(true),
 	fIsMouseDown(false),
 	fIsMouseOver(false),
-	fRepeatDelay(100000)
+	fRepeater(NULL)
 {
 }
 
 
 SpinnerButton::~SpinnerButton()
 {
+	delete fRepeater;
 }
 
 
@@ -448,10 +446,14 @@ SpinnerButton::MouseDown(BPoint where)
 {
 	if (fIsEnabled) {
 		fIsMouseDown = true;
+		fSpinnerDirection == SPINNER_INCREMENT
+			? fParent->Increment()
+			: fParent->Decrement();
 		Invalidate();
-		fRepeatDelay = 100000;
+		BMessage repeatMessage('rept');
-		MouseDownThread<SpinnerButton>::TrackMouse(this,
+		SetMouseEventMask(B_POINTER_EVENTS, B_NO_POINTER_HISTORY);
-			&SpinnerButton::_DoneTracking, &SpinnerButton::_Track);
+		fRepeater = new BMessageRunner(BMessenger(this), repeatMessage,
+			200000);
 	}
 
 	BView::MouseDown(where);
@@ -470,8 +472,6 @@ SpinnerButton::MouseMoved(BPoint where, uint32 transit,
 			uint32 buttons;
 			GetMouse(&where, &buttons);
 			fIsMouseOver = Bounds().Contains(where) && buttons == 0;
-			if (!fIsMouseDown)
-				Invalidate();
 
 			break;
 		}
@@ -491,38 +491,32 @@ void
 SpinnerButton::MouseUp(BPoint where)
 {
 	fIsMouseDown = false;
+	delete fRepeater;
+	fRepeater = NULL;
 	Invalidate();
 
 	BView::MouseUp(where);
 }
 
 
-//	#pragma mark  - SpinnerButton private methods
-
-
 void
-SpinnerButton::_DoneTracking(BPoint where)
+SpinnerButton::MessageReceived(BMessage* message)
 {
-	if (fIsMouseDown || !Bounds().Contains(where))
+	switch (message->what) {
-		fIsMouseDown = false;
+		case 'rept':
-}
+		{
+			if (fIsMouseDown && fRepeater != NULL) {
+				fSpinnerDirection == SPINNER_INCREMENT
+					? fParent->Increment()
+					: fParent->Decrement();
+			}
 
+			break;
+		}
 
-void
+		default:
-SpinnerButton::_Track(BPoint where, uint32)
+			BView::MessageReceived(message);
-{
-	if (fParent == NULL || !Bounds().Contains(where)) {
-		fIsMouseDown = false;
-		return;
 	}
-	fIsMouseDown = true;
-
-	fSpinnerDirection == SPINNER_INCREMENT
-		? fParent->Increment()
-		: fParent->Decrement();
-
-	snooze(fRepeatDelay);
-	fRepeatDelay = 10000;
 }
 
 

src/kits/interface/MenuField.cpp

@@ -30,7 +30,6 @@
 #include <MenuPrivate.h>
 #include <Message.h>
 #include <MessageFilter.h>
-#include <Thread.h>
 #include <Window.h>
 
 #include <binary_compatibility/Interface.h>
@@ -484,8 +483,7 @@ BMenuField::MouseDown(BPoint where)
 		if (fMouseDownFilter->Looper() == NULL)
 			Window()->AddCommonFilter(fMouseDownFilter);
 
-		MouseDownThread<BMenuField>::TrackMouse(this, &BMenuField::_DoneTracking,
+		SetMouseEventMask(B_POINTER_EVENTS, B_NO_POINTER_HISTORY);
-			&BMenuField::_Track);
 	}
 }
 
@@ -557,6 +555,7 @@ BMenuField::MouseMoved(BPoint point, uint32 code, const BMessage* message)
 void
 BMenuField::MouseUp(BPoint where)
 {
+	Window()->RemoveCommonFilter(fMouseDownFilter);
 	BView::MouseUp(where);
 }
 
@@ -1403,19 +1402,6 @@ BMenuField::_MenuBarWidth() const
 }
 
 
-void
-BMenuField::_DoneTracking(BPoint point)
-{
-	Window()->RemoveCommonFilter(fMouseDownFilter);
-}
-
-
-void
-BMenuField::_Track(BPoint point, uint32)
-{
-}
-
-
 // #pragma mark - BMenuField::LabelLayoutItem
 
 

src/kits/tracker/DialogPane.cpp

@@ -38,7 +38,6 @@ All rights reserved.
 #include <ControlLook.h>
 #include <LayoutUtils.h>
 
-#include "Thread.h"
 #include "Utilities.h"
 #include "Window.h"
 
@@ -139,12 +138,53 @@ PaneSwitch::MouseDown(BPoint)
 		return;
 
 	fPressing = true;
-	MouseDownThread<PaneSwitch>::TrackMouse(this, &PaneSwitch::DoneTracking,
+	SetMouseEventMask(B_POINTER_EVENTS, B_NO_POINTER_HISTORY);
-		&PaneSwitch::Track);
 	Invalidate();
 }
 
 
+void
+PaneSwitch::MouseMoved(BPoint point, uint32 code, const BMessage* message)
+{
+	int32 buttons;
+	BMessage* currentMessage = Window()->CurrentMessage();
+	if (currentMessage == NULL
+		|| currentMessage->FindInt32("buttons", &buttons) != B_OK) {
+		buttons = 0;
+	}
+
+	if (buttons != 0) {
+		BRect bounds(Bounds());
+		bounds.InsetBy(-3, -3);
+
+		bool newPressing = bounds.Contains(point);
+		if (newPressing != fPressing) {
+			fPressing = newPressing;
+			Invalidate();
+		}
+	}
+
+	BControl::MouseMoved(point, code, message);
+}
+
+
+void
+PaneSwitch::MouseUp(BPoint point)
+{
+	BRect bounds(Bounds());
+	bounds.InsetBy(-3, -3);
+
+	fPressing = false;
+	Invalidate();
+	if (bounds.Contains(point)) {
+		SetValue(!Value());
+		Invoke();
+	}
+
+	BControl::MouseUp(point);
+}
+
+
 void
 PaneSwitch::GetPreferredSize(float* _width, float* _height)
 {
@@ -213,35 +253,6 @@ PaneSwitch::SetLabels(const char* labelOn, const char* labelOff)
 }
 
 
-void
-PaneSwitch::DoneTracking(BPoint point)
-{
-	BRect bounds(Bounds());
-	bounds.InsetBy(-3, -3);
-
-	fPressing = false;
-	Invalidate();
-	if (bounds.Contains(point)) {
-		SetValue(!Value());
-		Invoke();
-	}
-}
-
-
-void
-PaneSwitch::Track(BPoint point, uint32)
-{
-	BRect bounds(Bounds());
-	bounds.InsetBy(-3, -3);
-
-	bool newPressing = bounds.Contains(point);
-	if (newPressing != fPressing) {
-		fPressing = newPressing;
-		Invalidate();
-	}
-}
-
-
 void
 PaneSwitch::DrawInState(PaneSwitch::State state)
 {

src/kits/tracker/DialogPane.h

@@ -57,6 +57,8 @@ public:
 
 	virtual	void				Draw(BRect updateRect);
 	virtual	void				MouseDown(BPoint where);
+	virtual	void				MouseMoved(BPoint where, uint32 code, const BMessage*);
+	virtual	void				MouseUp(BPoint where);
 
 	virtual	void				GetPreferredSize(float* _width,
 									float* _height);
@@ -69,9 +71,6 @@ public:
 									const char* labelOff);
 
 protected:
-			void				DoneTracking(BPoint where);
-			void				Track(BPoint where, uint32);
-
 			enum State {
 				kCollapsed,
 				kPressed,