Reporting: Delete the now unused extra threads, fix up compat reports.

Common/LogReporting.cpp

@@ -67,7 +67,7 @@ void ReportMessage(const char *message, ...) {
 		return;
 
 	const int MESSAGE_BUFFER_SIZE = 65536;
-	char temp[MESSAGE_BUFFER_SIZE];
+	char *temp = new char [MESSAGE_BUFFER_SIZE];
 
 	va_list args;
 	va_start(args, message);
@@ -76,6 +76,8 @@ void ReportMessage(const char *message, ...) {
 	va_end(args);
 
 	messageCallback(message, temp);
+
+	delete[] temp;
 }
 
 void ReportMessageFormatted(const char *message, const char *formatted) {

Core/Reporting.cpp

@@ -84,22 +84,13 @@ namespace Reporting
 	static int lastModuleVersion;
 	static uint32_t lastModuleCrc;
 
-	static std::mutex pendingMessageLock;
-	static std::condition_variable pendingMessageCond;
-	static std::deque<int> pendingMessages;
-	static bool pendingMessagesDone = false;
-	static std::thread messageThread;
-	static std::thread compatThread;
-
-	enum class RequestType
-	{
+	enum class RequestType {
 		NONE,
 		MESSAGE,
 		COMPAT,
 	};
 
-	struct Payload
-	{
+	struct Payload {
 		RequestType type;
 		std::string string1;
 		std::string string2;
@@ -107,8 +98,6 @@ namespace Reporting
 		int int2;
 		int int3;
 	};
-	static Payload payloadBuffer[PAYLOAD_BUFFER_SIZE];
-	static int payloadBufferPos = 0;
 
 	static std::mutex crcLock;
 	static std::condition_variable crcCond;
@@ -240,8 +229,7 @@ namespace Reporting
 	}
 
 	// Returns the full host (e.g. report.ppsspp.org:80.)
-	std::string ServerHost()
-	{
+	std::string ServerHost() {
 		if (g_Config.sReportHost.compare("default") == 0)
 			return "";
 		return g_Config.sReportHost;
@@ -306,20 +294,15 @@ namespace Reporting
 		return ++spamProtectionCount >= SPAM_LIMIT;
 	}
 
-	bool SendReportRequest(const char *uri, const std::string &data, const std::string &mimeType, Buffer *output = NULL)
-	{
+	static void SendReportRequest(const char *uri, const std::string &data, const std::string &mimeType, std::function<void(http::Request &)> callback) {
 		char url[1024];
 		const char *hostname = ServerHostname();
 		int port = ServerPort();
 		snprintf(url, sizeof(url), "http://%s:%d%s", hostname, port, uri);
-		g_DownloadManager.AsyncPostWithCallback(url, data, mimeType, http::ProgressBarMode::NONE, [=](http::Request &req) {
-			serverWorking = !req.Failed();
-		});
-		return true;
+		g_DownloadManager.AsyncPostWithCallback(url, data, mimeType, http::ProgressBarMode::NONE, callback);
 	}
 
-	std::string StripTrailingNull(const std::string &str)
-	{
+	std::string StripTrailingNull(const std::string &str) {
 		size_t pos = str.find_first_of('\0');
 		if (pos != str.npos)
 			return str.substr(0, pos);
@@ -369,14 +352,12 @@ namespace Reporting
 	bool MessageAllowed();
 	void SendReportMessage(const char *message, const char *formatted);
 
-	void Init()
-	{
+	void Init() {
 		// New game, clean slate.
 		spamProtectionCount = 0;
 		ResetCounts();
 		everUnsupported = false;
 		currentSupported = IsSupported();
-		pendingMessagesDone = false;
 		Reporting::SetupCallbacks(&MessageAllowed, &SendReportMessage);
 
 		lastModuleName.clear();
@@ -384,24 +365,14 @@ namespace Reporting
 		lastModuleCrc = 0;
 	}
 
-	void Shutdown()
-	{
-		pendingMessageLock.lock();
-		pendingMessagesDone = true;
-		pendingMessageCond.notify_one();
-		pendingMessageLock.unlock();
-		if (compatThread.joinable())
-			compatThread.join();
-		if (messageThread.joinable())
-			messageThread.join();
+	void Shutdown() {
 		PurgeCRC();
 
 		// Just so it can be enabled in the menu again.
 		Init();
 	}
 
-	void DoState(PointerWrap &p)
-	{
+	void DoState(PointerWrap &p) {
 		const int LATEST_VERSION = 1;
 		auto s = p.Section("Reporting", 0, LATEST_VERSION);
 		if (!s || s < LATEST_VERSION) {
@@ -413,8 +384,7 @@ namespace Reporting
 		Do(p, everUnsupported);
 	}
 
-	void UpdateConfig()
-	{
+	void UpdateConfig() {
 		currentSupported = IsSupported();
 		if (!currentSupported && PSP_IsInited())
 			everUnsupported = true;
@@ -498,9 +468,7 @@ namespace Reporting
 		}
 	}
 
-	// Not the thread func, but called from it.
-	int Process(int pos) {
-		Payload &payload = payloadBuffer[pos];
+	int Process(const Payload &payload) {
 		Buffer output;
 
 		MultipartFormDataEncoder postdata;
@@ -509,21 +477,18 @@ namespace Reporting
 		AddConfigInfo(postdata);
 		AddGameplayInfo(postdata);
 
-		switch (payload.type)
-		{
+		switch (payload.type) {
 		case RequestType::MESSAGE:
 			// TODO: Add CRC?
 			postdata.Add("message", payload.string1);
 			postdata.Add("value", payload.string2);
 			// We tend to get corrupted data, this acts as a very primitive verification check.
 			postdata.Add("verify", payload.string1 + payload.string2);
-			payload.string1.clear();
-			payload.string2.clear();
 
 			postdata.Finish();
-			serverWorking = true;
-			if (!SendReportRequest("/report/message", postdata.ToString(), postdata.GetMimeType()))
-				serverWorking = false;
+			SendReportRequest("/report/message", postdata.ToString(), postdata.GetMimeType(), [=](http::Request &req) {
+				serverWorking = !req.Failed();
+			});
 			break;
 
 		case RequestType::COMPAT:
@@ -536,31 +501,28 @@ namespace Reporting
 			postdata.Add("crc", StringFromFormat("%08x", RetrieveCRCUnlessPowerSaving(PSP_CoreParameter().fileToStart)));
 			postdata.Add("suggestions", payload.string1 != "perfect" && payload.string1 != "playable" ? "1" : "0");
 			AddScreenshotData(postdata, Path(payload.string2));
-			payload.string1.clear();
-			payload.string2.clear();
 
 			postdata.Finish();
 			serverWorking = true;
-			if (!SendReportRequest("/report/compat", postdata.ToString(), postdata.GetMimeType(), &output)) {
+			SendReportRequest("/report/compat", postdata.ToString(), postdata.GetMimeType(), [=](http::Request &req) {
+				if (req.Failed()) {
 					serverWorking = false;
-			} else {
-				std::string result;
-				output.TakeAll(&result);
+					return;
+				}
+				serverWorking = true;
 
+				std::string result;
+				req.buffer().TakeAll(&result);
 				lastCompatResult.clear();
 				if (result.empty() || result[0] == '0')
 					serverWorking = false;
 				else if (result[0] != '1')
 					SplitString(result, '\n', lastCompatResult);
-			}
+			});
 			break;
-
 		case RequestType::NONE:
 			break;
 		}
-
-		payload.type = RequestType::NONE;
-
 		return 0;
 	}
 
@@ -623,56 +585,12 @@ namespace Reporting
 		g_Config.sReportHost = "default";
 	}
 
-	ReportStatus GetStatus()
-	{
+	ReportStatus GetStatus() {
 		if (!serverWorking)
 			return ReportStatus::FAILING;
-
-		for (int pos = 0; pos < PAYLOAD_BUFFER_SIZE; ++pos)
-		{
-			if (payloadBuffer[pos].type != RequestType::NONE)
-				return ReportStatus::BUSY;
-		}
-
 		return ReportStatus::WORKING;
 	}
 
-	int NextFreePos()
-	{
-		int start = payloadBufferPos % PAYLOAD_BUFFER_SIZE;
-		do
-		{
-			int pos = payloadBufferPos++ % PAYLOAD_BUFFER_SIZE;
-			if (payloadBuffer[pos].type == RequestType::NONE)
-				return pos;
-		}
-		while (payloadBufferPos != start);
-
-		return -1;
-	}
-
-	int SendPendingReportsThread() {
-		SetCurrentThreadName("Report");
-
-		std::unique_lock<std::mutex> guard(pendingMessageLock);
-		while (!pendingMessagesDone) {
-			while (!pendingMessages.empty() && !pendingMessagesDone) {
-				int pos = pendingMessages.front();
-				pendingMessages.pop_front();
-
-				guard.unlock();
-				Process(pos);
-				guard.lock();
-			}
-			if (pendingMessagesDone) {
-				break;
-			}
-			pendingMessageCond.wait(guard);
-		}
-
-		return 0;
-	}
-
 	bool MessageAllowed() {
 		if (!IsEnabled() || CheckSpamLimited())
 			return false;
@@ -680,33 +598,20 @@ namespace Reporting
 	}
 
 	void SendReportMessage(const char *message, const char *formatted) {
-		int pos = NextFreePos();
-		if (pos == -1)
-			return;
+		// MessageAllowed is checked first.
 
-		Payload &payload = payloadBuffer[pos];
+		Payload payload{};
 		payload.type = RequestType::MESSAGE;
 		payload.string1 = message;
 		payload.string2 = formatted;
 
-		std::lock_guard<std::mutex> guard(pendingMessageLock);
-		pendingMessages.push_back(pos);
-		pendingMessageCond.notify_one();
-
-		if (!messageThread.joinable()) {
-			messageThread = std::thread(SendPendingReportsThread);
-		}
+		Process(payload);
 	}
 
-	void ReportCompatibility(const char *compat, int graphics, int speed, int gameplay, const std::string &screenshotFilename)
-	{
+	void ReportCompatibility(const char *compat, int graphics, int speed, int gameplay, const std::string &screenshotFilename) {
 		if (!IsEnabled())
 			return;
-		int pos = NextFreePos();
-		if (pos == -1)
-			return;
-
-		Payload &payload = payloadBuffer[pos];
+		Payload payload{};
 		payload.type = RequestType::COMPAT;
 		payload.string1 = compat;
 		payload.string2 = screenshotFilename;
@@ -714,9 +619,7 @@ namespace Reporting
 		payload.int2 = speed;
 		payload.int3 = gameplay;
 
-		if (compatThread.joinable())
-			compatThread.join();
-		compatThread = std::thread(Process, pos);
+		Process(payload);
 	}
 
 	std::vector<std::string> CompatibilitySuggestions() {

UI/ReportScreen.cpp

@@ -168,7 +168,7 @@ ReportScreen::ReportScreen(const Path &gamePath)  // unused gamePath, after remo
 
 ScreenRenderFlags ReportScreen::render(ScreenRenderMode mode) {
 	_dbg_assert_(mode & ScreenRenderMode::FIRST);
-	_dbg_assert_(mode & ScreenRenderMode::TOP);
+	// _dbg_assert_(mode & ScreenRenderMode::TOP);
 
 	if (mode & ScreenRenderMode::TOP) {
 		// We do this after render because we need it to be within the frame (so the screenshot works).