HaikuDepot: Rework to get actions working properly.

- BPackageManager doesn't (currently) support being reused for multiple
  differing actions, so instantiate it on the fly for each action to be
  performed.

- As such, pull the package action worker out of PackageManager and into
  MainWindow. Define abstract interface PackageActionHandler for other
  parts of Depot to use to instruct it to queue actions.

- Adjust other parts of code to take the above into account.

src/apps/haiku-depot/Jamfile

@@ -40,6 +40,8 @@ Application HaikuDepot :
 	main.cpp
 	MainWindow.cpp
 	Model.cpp
+	PackageAction.cpp
+	PackageActionHandler.cpp
 	PackageInfo.cpp
 	PackageInfoListener.cpp
 	PackageInfoView.cpp

src/apps/haiku-depot/MainWindow.cpp

@@ -29,12 +29,13 @@
 #include <package/solver/SolverPackage.h>
 
 #include "AutoDeleter.h"
+#include "AutoLocker.h"
 #include "DecisionProvider.h"
 #include "FilterView.h"
 #include "JobStateListener.h"
 #include "PackageInfoView.h"
 #include "PackageListView.h"
-
+#include "PackageManager.h"
 
 
 #undef B_TRANSLATION_CONTEXT
@@ -76,7 +77,6 @@ MainWindow::MainWindow(BRect frame)
 	BWindow(frame, B_TRANSLATE_SYSTEM_NAME("HaikuDepot"),
 		B_DOCUMENT_WINDOW_LOOK, B_NORMAL_WINDOW_FEEL,
 		B_ASYNCHRONOUS_CONTROLS | B_AUTO_UPDATE_SIZE_LIMITS),
-	fPackageManager(B_PACKAGE_INSTALLATION_LOCATION_HOME),
 	fTerminating(false),
 	fModelWorker(B_BAD_THREAD_ID)
 {
@@ -85,7 +85,7 @@ MainWindow::MainWindow(BRect frame)
 
 	fFilterView = new FilterView(fModel);
 	fPackageListView = new PackageListView(fModel.Lock());
-	fPackageInfoView = new PackageInfoView(fModel.Lock(), &fPackageManager);
+	fPackageInfoView = new PackageInfoView(fModel.Lock(), this);
 
 	fSplitView = new BSplitView(B_VERTICAL, 5.0f);
 
@@ -107,6 +107,14 @@ MainWindow::MainWindow(BRect frame)
 	fSplitView->SetCollapsible(1, false);
 
 	_StartRefreshWorker();
+
+	fPendingActionsSem = create_sem(0, "PendingPackageActions");
+	if (fPendingActionsSem >= 0) {
+		fPendingActionsWorker = spawn_thread(&_PackageActionWorker,
+			"Planet Express", B_NORMAL_PRIORITY, this);
+		if (fPendingActionsWorker >= 0)
+			resume_thread(fPendingActionsWorker);
+	}
 }
 
 
@@ -117,6 +125,10 @@ MainWindow::~MainWindow()
 		status_t result;
 		wait_for_thread(fModelWorker, &result);
 	}
+
+	delete_sem(fPendingActionsSem);
+	status_t result;
+	wait_for_thread(fPendingActionsWorker, &result);
 }
 
 
@@ -231,6 +243,20 @@ MainWindow::PackageChanged(const PackageInfoEvent& event)
 }
 
 
+status_t
+MainWindow::SchedulePackageActions(PackageActionList& list)
+{
+	AutoLocker<BLocker> lock(&fPendingActionsLock);
+	for (int32 i = 0; i < list.CountItems(); i++) {
+		if (!fPendingActions.Add(list.ItemAtFast(i))) {
+			return B_NO_MEMORY;
+		}
+	}
+
+	return release_sem_etc(fPendingActionsSem, list.CountItems(), 0);
+}
+
+
 void
 MainWindow::_BuildMenu(BMenuBar* menuBar)
 {
@@ -333,8 +359,9 @@ MainWindow::_RefreshPackageList()
 		depots[repoName] = DepotInfo(repoName);
 	}
 
+	PackageManager manager(B_PACKAGE_INSTALLATION_LOCATION_HOME);
 	try {
-		fPackageManager.Init(PackageManager::B_ADD_INSTALLED_REPOSITORIES
+		manager.Init(PackageManager::B_ADD_INSTALLED_REPOSITORIES
 			| PackageManager::B_ADD_REMOTE_REPOSITORIES);
 	} catch (BException ex) {
 		BString message(B_TRANSLATE("An error occurred while "
@@ -345,7 +372,7 @@ MainWindow::_RefreshPackageList()
 	}
 
 	BObjectList<BSolverPackage> packages;
-	result = fPackageManager.Solver()->FindPackages("",
+	result = manager.Solver()->FindPackages("",
 		BSolver::B_FIND_CASE_INSENSITIVE | BSolver::B_FIND_IN_NAME
 			| BSolver::B_FIND_IN_SUMMARY | BSolver::B_FIND_IN_DESCRIPTION
 			| BSolver::B_FIND_IN_PROVIDES,
@@ -401,13 +428,13 @@ MainWindow::_RefreshPackageList()
 		} else {
 			const char* installationLocation = NULL;
 			if (repository == static_cast<const BSolverRepository*>(
-					fPackageManager.SystemRepository())) {
+					manager.SystemRepository())) {
 				installationLocation = "system";
 			} else if (repository == static_cast<const BSolverRepository*>(
-					fPackageManager.CommonRepository())) {
+					manager.CommonRepository())) {
 				installationLocation = "common";
 			} else if (repository == static_cast<const BSolverRepository*>(
-					fPackageManager.HomeRepository())) {
+					manager.HomeRepository())) {
 				installationLocation = "home";
 			}
 
@@ -484,6 +511,28 @@ MainWindow::_RefreshModelThreadWorker(void* arg)
 }
 
 
+status_t
+MainWindow::_PackageActionWorker(void* arg)
+{
+	MainWindow* window = reinterpret_cast<MainWindow*>(arg);
+
+	while (acquire_sem(window->fPendingActionsSem) == B_OK) {
+		PackageActionRef ref;
+		{
+			AutoLocker<BLocker> lock(&window->fPendingActionsLock);
+			ref = window->fPendingActions.ItemAt(0);
+			if (ref.Get() == NULL)
+				break;
+			window->fPendingActions.Remove(0);
+		}
+
+		ref->Perform();
+	}
+
+	return 0;
+}
+
+
 void
 MainWindow::_NotifyUser(const char* title, const char* message)
 {

src/apps/haiku-depot/MainWindow.h

@@ -9,8 +9,9 @@
 #include <Window.h>
 
 #include "Model.h"
+#include "PackageAction.h"
+#include "PackageActionHandler.h"
 #include "PackageInfoListener.h"
-#include "PackageManager.h"
 
 
 class BSplitView;
@@ -19,12 +20,14 @@ class PackageActionsView;
 class PackageInfoView;
 class PackageListView;
 
+
 enum {
 	MSG_MAIN_WINDOW_CLOSED		= 'mwcl',
 };
 
 
-class MainWindow : public BWindow, private PackageInfoListener {
+class MainWindow : public BWindow, private PackageInfoListener,
+	private PackageActionHandler {
 public:
 								MainWindow(BRect frame);
 	virtual						~MainWindow();
@@ -38,6 +41,11 @@ private:
 	virtual	void				PackageChanged(
 									const PackageInfoEvent& event);
 
+private:
+	// PackageActionHandler
+	virtual	status_t			SchedulePackageActions(
+									PackageActionList& list);
+
 private:
 			void				_BuildMenu(BMenuBar* menuBar);
 			void				_AdoptModel();
@@ -52,6 +60,9 @@ private:
 
 	static	status_t			_RefreshModelThreadWorker(void* arg);
 
+	static	status_t			_PackageActionWorker(void* arg);
+
+
 			void				_NotifyUser(const char* title,
 									const char* message);
 
@@ -64,11 +75,13 @@ private:
 			Model				fModel;
 			PackageList			fVisiblePackages;
 
-			PackageManager
-								fPackageManager;
-
 			bool				fTerminating;
 			thread_id			fModelWorker;
+
+			thread_id			fPendingActionsWorker;
+			PackageActionList	fPendingActions;
+			BLocker				fPendingActionsLock;
+			sem_id				fPendingActionsSem;
 };
 
 #endif // MAIN_WINDOW_H

src/apps/haiku-depot/PackageAction.cpp

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2013, Stephan Aßmus <[email protected]>.
+ * Copyright 2013, Rene Gollent, <[email protected]>
+ *
+ * All rights reserved. Distributed under the terms of the MIT License.
+ */
+
+
+#include "PackageAction.h"
+
+#include "PackageManager.h"
+
+
+using namespace BPackageKit;
+
+// #pragma mark - PackageAction
+
+
+PackageAction::PackageAction(PackageInfoRef package)
+	:
+	fPackage(package)
+{
+	// TODO: allow configuring the installation location
+	fPackageManager = new(std::nothrow) PackageManager(
+		B_PACKAGE_INSTALLATION_LOCATION_HOME);
+}
+
+
+PackageAction::~PackageAction()
+{
+	delete fPackageManager;
+}
+
+

src/apps/haiku-depot/PackageAction.h

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2013, Stephan Aßmus <[email protected]>.
+ * Copyright 2013, Rene Gollent, <[email protected]>
+ *
+ * All rights reserved. Distributed under the terms of the MIT License.
+ */
+#ifndef PACKAGE_ACTION_H
+#define PACKAGE_ACTION_H
+
+#include <Referenceable.h>
+
+#include "PackageInfo.h"
+
+
+class PackageManager;
+
+
+class PackageAction : public BReferenceable {
+public:
+								PackageAction(PackageInfoRef package);
+	virtual						~PackageAction();
+
+	virtual const char*			Label() const = 0;
+
+	virtual status_t			Perform() = 0;
+
+			PackageInfoRef		Package() const
+									{ return fPackage; }
+
+protected:
+			PackageManager*		fPackageManager;
+
+private:
+			PackageInfoRef		fPackage;
+};
+
+
+typedef BReference<PackageAction> PackageActionRef;
+typedef List<PackageActionRef, false> PackageActionList;
+
+
+#endif // PACKAGE_ACTION_H

src/apps/haiku-depot/PackageActionHandler.cpp

@@ -0,0 +1,12 @@
+/*
+ * Copyright 2013, Rene Gollent <[email protected]>.
+ * All rights reserved. Distributed under the terms of the MIT License.
+ */
+
+#include "PackageActionHandler.h"
+
+
+PackageActionHandler::~PackageActionHandler()
+{
+}
+

src/apps/haiku-depot/PackageActionHandler.h

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2013, Rene Gollent <[email protected]>.
+ * All rights reserved. Distributed under the terms of the MIT License.
+ */
+
+#ifndef PACKAGE_ACTION_HANDLER_H
+#define PACkAGE_ACTION_HANDLER_H
+
+
+#include <SupportDefs.h>
+
+#include "PackageAction.h"
+
+
+class PackageActionHandler {
+public:
+	virtual						~PackageActionHandler();
+
+	virtual	status_t			SchedulePackageActions(
+									PackageActionList& list) = 0;
+};
+
+#endif // PACKAGE_ACTION_HANDLER_H

src/apps/haiku-depot/PackageInfoView.cpp

@@ -29,6 +29,7 @@
 #include "BitmapButton.h"
 #include "BitmapView.h"
 #include "MarkupParser.h"
+#include "PackageActionHandler.h"
 #include "PackageManager.h"
 #include "TextDocumentView.h"
 #include "TextView.h"
@@ -488,11 +489,11 @@ private:
 
 class PackageActionView : public BView {
 public:
-	PackageActionView(PackageManager* packageManager)
+	PackageActionView(PackageActionHandler* handler)
 		:
 		BView("about view", B_WILL_DRAW),
-		fPackageManager(packageManager),
+		fLayout(new BGroupLayout(B_HORIZONTAL)),
-		fLayout(new BGroupLayout(B_HORIZONTAL))
+		fPackageActionHandler(handler)
 	{
 		SetViewColor(ui_color(B_PANEL_BACKGROUND_COLOR));
 
@@ -516,7 +517,7 @@ public:
 					if (action.Get() != NULL) {
 						PackageActionList actions;
 						actions.Add(action);
-						status_t result = fPackageManager
+						status_t result = fPackageActionHandler
 							->SchedulePackageActions(actions);
 						if (result != B_OK) {
 							fprintf(stderr, "Failed to schedule action: "
@@ -538,7 +539,10 @@ public:
 	{
 		Clear();
 
-		fPackageActions = fPackageManager->GetPackageActions(
+		PackageManager manager(
+			BPackageKit::B_PACKAGE_INSTALLATION_LOCATION_HOME);
+
+		fPackageActions = manager.GetPackageActions(
 			const_cast<PackageInfo*>(&package));
 
 		// Add Buttons in reverse action order
@@ -567,10 +571,9 @@ public:
 	}
 
 private:
-	PackageManager*		fPackageManager;
-
 	BGroupLayout*		fLayout;
 	PackageActionList	fPackageActions;
+	PackageActionHandler* fPackageActionHandler;
 	BList				fButtons;
 };
 
@@ -1216,14 +1219,14 @@ private:
 
 
 PackageInfoView::PackageInfoView(BLocker* modelLock,
-		PackageManager* packageManager)
+		PackageActionHandler* handler)
 	:
 	BGroupView("package info view", B_VERTICAL),
 	fModelLock(modelLock),
 	fPackageListener(new(std::nothrow) Listener(this))
 {
 	fTitleView = new TitleView();
-	fPackageActionView = new PackageActionView(packageManager);
+	fPackageActionView = new PackageActionView(handler);
 	fPagesView = new PagesView();
 
 	BLayoutBuilder::Group<>(this)

src/apps/haiku-depot/PackageInfoView.h

@@ -13,8 +13,8 @@
 
 class BLocker;
 class TitleView;
+class PackageActionHandler;
 class PackageActionView;
-class PackageManager;
 class PagesView;
 
 enum {
@@ -26,7 +26,7 @@ enum {
 class PackageInfoView : public BGroupView {
 public:
 								PackageInfoView(BLocker* modelLock,
-									PackageManager* packageManager);
+									PackageActionHandler* handler);
 	virtual						~PackageInfoView();
 
 	virtual void				AttachedToWindow();

src/apps/haiku-depot/PackageManager.cpp

@@ -42,31 +42,15 @@ using BPackageKit::BSolver;
 using BPackageKit::BSolverPackage;
 using BPackageKit::BSolverRepository;
 
-// #pragma mark - PackageAction
-
-
-PackageAction::PackageAction(PackageInfoRef package,
-	PackageManager* manager)
-	:
-	fPackageManager(manager),
-	fPackage(package)
-{
-}
-
-
-PackageAction::~PackageAction()
-{
-}
-
 
 // #pragma mark - InstallPackageAction
 
 
 class InstallPackageAction : public PackageAction {
 public:
-	InstallPackageAction(PackageInfoRef package, PackageManager* manager)
+	InstallPackageAction(PackageInfoRef package)
 		:
-		PackageAction(package, manager)
+		PackageAction(package)
 	{
 	}
 
@@ -112,9 +96,9 @@ public:
 
 class UninstallPackageAction : public PackageAction {
 public:
-	UninstallPackageAction(PackageInfoRef package, PackageManager* manager)
+	UninstallPackageAction(PackageInfoRef package)
 		:
-		PackageAction(package, manager)
+		PackageAction(package)
 	{
 	}
 
@@ -171,23 +155,11 @@ PackageManager::PackageManager(BPackageInstallationLocation location)
 	fInstallationInterface = &fClientInstallationInterface;
 	fRequestHandler = this;
 	fUserInteractionHandler = this;
-
-	fPendingActionsSem = create_sem(0, "PendingPackageActions");
-	if (fPendingActionsSem >= 0) {
-		fPendingActionsWorker = spawn_thread(&_PackageActionWorker,
-			"Planet Express", B_NORMAL_PRIORITY, this);
-		if (fPendingActionsWorker >= 0)
-			resume_thread(fPendingActionsWorker);
-	}
 }
 
 
 PackageManager::~PackageManager()
 {
-	delete_sem(fPendingActionsSem);
-	status_t result;
-	wait_for_thread(fPendingActionsWorker, &result);
-
 	if (fProblemWindow != NULL)
 		fProblemWindow->PostMessage(B_QUIT_REQUESTED);
 }
@@ -230,11 +202,11 @@ PackageManager::GetPackageActions(PackageInfoRef package)
 	if (installed) {
 		if (!systemPackage) {
 			actionList.Add(PackageActionRef(new UninstallPackageAction(
-				package, this), true));
+				package), true));
 		}
 	} else {
-		actionList.Add(PackageActionRef(new InstallPackageAction(package,
+		actionList.Add(PackageActionRef(new InstallPackageAction(package),
-					this), true));
+				true));
 	}
 	// TODO: activation status
 
@@ -242,38 +214,12 @@ PackageManager::GetPackageActions(PackageInfoRef package)
 }
 
 
-status_t
-PackageManager::SchedulePackageActions(PackageActionList& list)
-{
-	AutoLocker<BLocker> lock(&fPendingActionsLock);
-	for (int32 i = 0; i < list.CountItems(); i++) {
-		if (!fPendingActions.Add(list.ItemAtFast(i))) {
-			return B_NO_MEMORY;
-		}
-	}
-
-	return release_sem_etc(fPendingActionsSem, list.CountItems(), 0);
-}
-
-
 void
 PackageManager::SetCurrentActionPackage(PackageInfoRef package, bool install)
 {
 	BSolverPackage* solverPackage = _GetSolverPackage(package);
-	if (solverPackage == NULL)
-		ClearCurrentActionPackage();
-	else {
 	fCurrentInstallPackage = install ? solverPackage : NULL;
 	fCurrentUninstallPackage = install ? NULL : solverPackage;
-	}
-}
-
-
-void
-PackageManager::ClearCurrentActionPackage()
-{
-	fCurrentInstallPackage = NULL;
-	fCurrentUninstallPackage = NULL;
 }
 
 
@@ -395,29 +341,6 @@ PackageManager::ProgressApplyingChangesDone(InstalledRepository& repository)
 }
 
 
-status_t
-PackageManager::_PackageActionWorker(void* arg)
-{
-	PackageManager* manager = reinterpret_cast<PackageManager*>(arg);
-
-	while (acquire_sem(manager->fPendingActionsSem) == B_OK) {
-		PackageActionRef ref;
-		{
-			AutoLocker<BLocker> lock(&manager->fPendingActionsLock);
-			ref = manager->fPendingActions.ItemAt(0);
-			if (ref.Get() == NULL)
-				break;
-			manager->fPendingActions.Remove(0);
-		}
-
-		ref->Perform();
-		manager->ClearCurrentActionPackage();
-	}
-
-	return 0;
-}
-
-
 bool
 PackageManager::_AddResults(InstalledRepository& repository,
 	ResultWindow* window)
@@ -431,7 +354,7 @@ PackageManager::_AddResults(InstalledRepository& repository,
 		installPackages.insert(fCurrentInstallPackage);
 
 	if (fCurrentUninstallPackage != NULL)
-		uninstallPackages.insert(fCurrentInstallPackage);
+		uninstallPackages.insert(fCurrentUninstallPackage);
 
 	return window->AddLocationChanges(repository.Name(),
 		repository.PackagesToActivate(), installPackages,

src/apps/haiku-depot/PackageManager.h

@@ -15,6 +15,7 @@
 
 #include "DecisionProvider.h"
 #include "JobStateListener.h"
+#include "PackageAction.h"
 #include "PackageInfo.h"
 
 
@@ -28,33 +29,6 @@ class ProblemWindow;
 class ResultWindow;
 
 
-class PackageAction : public BReferenceable {
-public:
-								PackageAction(PackageInfoRef package,
-									PackageManager* manager);
-	virtual						~PackageAction();
-
-	virtual const char*			Label() const = 0;
-
-	// TODO: Perform() needs to be passed a progress listener
-	// and it needs a mechanism to report and react to errors. The
-	// Package Kit supports this stuff already.
-	virtual status_t			Perform() = 0;
-
-			PackageInfoRef		Package() const
-									{ return fPackage; }
-
-protected:
-			PackageManager*		fPackageManager;
-
-private:
-			PackageInfoRef		fPackage;
-};
-
-
-typedef BReference<PackageAction> PackageActionRef;
-typedef List<PackageActionRef, false> PackageActionList;
-
 using BPackageKit::BContext;
 using BPackageKit::BPackageInstallationLocation;
 using BPackageKit::BRepositoryConfig;
@@ -73,15 +47,10 @@ public:
 	virtual	PackageState		GetPackageState(const PackageInfo& package);
 	virtual	PackageActionList	GetPackageActions(PackageInfoRef package);
 
-			status_t			SchedulePackageActions(
-									PackageActionList& list);
-
 			void				SetCurrentActionPackage(
 									PackageInfoRef package,
 									bool install);
 
-			void				ClearCurrentActionPackage();
-
 private:
 	// RequestHandler
 	virtual	status_t			RefreshRepository(
@@ -105,8 +74,6 @@ private:
 									InstalledRepository& repository);
 
 private:
-		static	status_t		_PackageActionWorker(void* arg);
-
 			bool				_AddResults(
 									BPackageManager::InstalledRepository&
 										repository,
@@ -122,10 +89,6 @@ private:
 			BPackageManager::ClientInstallationInterface
 								fClientInstallationInterface;
 
-			thread_id			fPendingActionsWorker;
-			PackageActionList	fPendingActions;
-			BLocker				fPendingActionsLock;
-			sem_id				fPendingActionsSem;
 			ProblemWindow*		fProblemWindow;
 			BPackageKit::BSolverPackage*
 								fCurrentInstallPackage;