GH-907 improve Java testing and PermGen deprecation handling

application/CMakeLists.txt

@@ -156,10 +156,9 @@ SET(MULTIMC_SOURCES
 	InstancePageProvider.h
 	InstancePageProvider.cpp
 
-	# Annoying nag screen logic
-	NagUtils.h
-	NagUtils.cpp
-
+	# Common java checking UI
+	JavaCommon.h
+	JavaCommon.cpp
 
 	# GUI - page dialog pages
 	pages/BasePage.h

application/JavaCommon.cpp

@@ -0,0 +1,107 @@
+#include "JavaCommon.h"
+#include "dialogs/CustomMessageBox.h"
+#include <MMCStrings.h>
+
+bool JavaCommon::checkJVMArgs(QString jvmargs, QWidget *parent)
+{
+	if (jvmargs.contains("-XX:PermSize=") || jvmargs.contains(QRegExp("-Xm[sx]")))
+	{
+		CustomMessageBox::selectable(
+			parent, QObject::tr("JVM arguments warning"),
+			QObject::tr("You tried to manually set a JVM memory option (using "
+						" \"-XX:PermSize\", \"-Xmx\" or \"-Xms\") - there"
+						" are dedicated boxes for these in the settings (Java"
+						" tab, in the Memory group at the top).\n"
+						"Your manual settings will be overridden by the"
+						" dedicated options.\n"
+						"This message will be displayed until you remove them"
+						" from the JVM arguments."),
+			QMessageBox::Warning)->exec();
+		return false;
+	}
+	return true;
+}
+
+void JavaCommon::TestCheck::javaWasOk(JavaCheckResult result)
+{
+	QString text;
+	text += tr("Java test succeeded!<br />Platform reported: %1<br />Java version "
+			   "reported: %2<br />").arg(result.realPlatform, result.javaVersion);
+	if (result.stderr.size())
+	{
+		auto htmlError = result.stderr;
+		htmlError.replace('\n', "<br />");
+		text += tr("<br />Warnings:<br /><font color=\"orange\">%1</font>").arg(htmlError);
+	}
+	CustomMessageBox::selectable(m_parent, tr("Java test success"), text,
+								 QMessageBox::Information)->show();
+}
+
+void JavaCommon::TestCheck::javaArgsWereBad(JavaCheckResult result)
+{
+	auto htmlError = result.stderr;
+	QString text;
+	htmlError.replace('\n', "<br />");
+	text += tr("The specified java binary didn't work with the arguments you provided:<br />");
+	text += tr("<font color=\"red\">%1</font>").arg(htmlError);
+	CustomMessageBox::selectable(m_parent, tr("Java test failure"), text, QMessageBox::Warning)
+		->show();
+}
+
+void JavaCommon::TestCheck::javaBinaryWasBad(JavaCheckResult result)
+{
+	QString text;
+	text += tr(
+		"The specified java binary didn't work.<br />You should use the auto-detect feature, "
+		"or set the path to the java executable.<br />");
+	CustomMessageBox::selectable(m_parent, tr("Java test failure"), text, QMessageBox::Warning)
+		->show();
+}
+
+void JavaCommon::TestCheck::run()
+{
+	if (!JavaCommon::checkJVMArgs(m_args, m_parent))
+	{
+		emit finished();
+		return;
+	}
+	checker.reset(new JavaChecker());
+	connect(checker.get(), SIGNAL(checkFinished(JavaCheckResult)), this,
+			SLOT(checkFinished(JavaCheckResult)));
+	checker->m_path = m_path;
+	checker->performCheck();
+}
+
+void JavaCommon::TestCheck::checkFinished(JavaCheckResult result)
+{
+	if (!result.valid)
+	{
+		javaBinaryWasBad(result);
+		emit finished();
+		return;
+	}
+	checker.reset(new JavaChecker());
+	connect(checker.get(), SIGNAL(checkFinished(JavaCheckResult)), this,
+			SLOT(checkFinishedWithArgs(JavaCheckResult)));
+	checker->m_path = m_path;
+	checker->m_args = m_args;
+	checker->m_minMem = m_minMem;
+	checker->m_maxMem = m_maxMem;
+	if (Strings::naturalCompare(result.javaVersion, "1.8", Qt::CaseInsensitive) < 0)
+	{
+		checker->m_permGen = m_permGen;
+	}
+	checker->performCheck();
+}
+
+void JavaCommon::TestCheck::checkFinishedWithArgs(JavaCheckResult result)
+{
+	if (result.valid)
+	{
+		javaWasOk(result);
+		emit finished();
+		return;
+	}
+	javaArgsWereBad(result);
+	emit finished();
+}

application/JavaCommon.h

@@ -0,0 +1,46 @@
+#pragma once
+#include <java/JavaChecker.h>
+
+class QWidget;
+
+/**
+ * Common UI bits for the java pages to use.
+ */
+namespace JavaCommon
+{
+	bool checkJVMArgs(QString args, QWidget *parent);
+
+	class TestCheck : public QObject
+	{
+		Q_OBJECT
+	public:
+		TestCheck(QWidget *parent, QString path, QString args, int minMem, int maxMem, int permGen)
+			:m_parent(parent), m_path(path), m_args(args), m_minMem(minMem), m_maxMem(maxMem), m_permGen(permGen)
+		{
+		}
+		virtual ~TestCheck() {};
+
+		void run();
+
+	signals:
+		void finished();
+
+	private:
+		void javaBinaryWasBad(JavaCheckResult result);
+		void javaArgsWereBad(JavaCheckResult result);
+		void javaWasOk(JavaCheckResult result);
+
+	private slots:
+		void checkFinished(JavaCheckResult result);
+		void checkFinishedWithArgs(JavaCheckResult result);
+
+	private:
+		std::shared_ptr<JavaChecker> checker;
+		QWidget *m_parent = nullptr;
+		QString m_path;
+		QString m_args;
+		int m_minMem = 0;
+		int m_maxMem = 0;
+		int m_permGen = 64;
+	};
+}

application/MainWindow.cpp

@@ -381,7 +381,7 @@ namespace Ui {
 #include "BaseInstance.h"
 #include "BaseProcess.h"
 #include "java/JavaUtils.h"
-#include "NagUtils.h"
+#include "JavaCommon.h"
 #include "InstancePageProvider.h"
 #include "minecraft/SkinUtils.h"
 
@@ -1545,7 +1545,7 @@ void MainWindow::instanceActivated(QModelIndex index)
 	if (!inst)
 		return;
 
-	NagUtils::checkJVMArgs(inst->settings().get("JvmArgs").toString(), this);
+	JavaCommon::checkJVMArgs(inst->settings().get("JvmArgs").toString(), this);
 
 	doLaunch();
 }
@@ -1554,7 +1554,7 @@ void MainWindow::on_actionLaunchInstance_triggered()
 {
 	if (m_selectedInstance)
 	{
-		NagUtils::checkJVMArgs(m_selectedInstance->settings().get("JvmArgs").toString(), this);
+		JavaCommon::checkJVMArgs(m_selectedInstance->settings().get("JvmArgs").toString(), this);
 		doLaunch();
 	}
 }
@@ -1563,7 +1563,7 @@ void MainWindow::on_actionLaunchInstanceOffline_triggered()
 {
 	if (m_selectedInstance)
 	{
-		NagUtils::checkJVMArgs(m_selectedInstance->settings().get("JvmArgs").toString(), this);
+		JavaCommon::checkJVMArgs(m_selectedInstance->settings().get("JvmArgs").toString(), this);
 		doLaunch(false);
 	}
 }

application/MultiMC.cpp

@@ -453,6 +453,8 @@ void MultiMC::initGlobalSettings(bool test_mode)
 
 	// Java Settings
 	m_settings->registerSetting("JavaPath", "");
+	m_settings->registerSetting("JavaTimestamp", 0);
+	m_settings->registerSetting("JavaVersion", "");
 	m_settings->registerSetting("LastHostname", "");
 	m_settings->registerSetting("JavaDetectionHack", "");
 	m_settings->registerSetting("JvmArgs", "");

application/NagUtils.cpp

@@ -1,38 +0,0 @@
-/* Copyright 2013-2015 MultiMC Contributors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "NagUtils.h"
-#include "dialogs/CustomMessageBox.h"
-
-namespace NagUtils
-{
-void checkJVMArgs(QString jvmargs, QWidget *parent)
-{
-	if (jvmargs.contains("-XX:PermSize=") || jvmargs.contains(QRegExp("-Xm[sx]")))
-	{
-		CustomMessageBox::selectable(
-			parent, QObject::tr("JVM arguments warning"),
-			QObject::tr("You tried to manually set a JVM memory option (using "
-						" \"-XX:PermSize\", \"-Xmx\" or \"-Xms\") - there"
-						" are dedicated boxes for these in the settings (Java"
-						" tab, in the Memory group at the top).\n"
-						"Your manual settings will be overridden by the"
-						" dedicated options.\n"
-						"This message will be displayed until you remove them"
-						" from the JVM arguments."),
-			QMessageBox::Warning)->exec();
-	}
-}
-}

application/NagUtils.h

@@ -1,23 +0,0 @@
-/* Copyright 2013-2015 MultiMC Contributors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma once
-
-#include <QWidget>
-
-namespace NagUtils
-{
-void checkJVMArgs(QString args, QWidget *parent);
-}

application/pages/InstanceSettingsPage.cpp

@@ -6,10 +6,11 @@
 #include <QMessageBox>
 
 #include "dialogs/VersionSelectDialog.h"
-#include "NagUtils.h"
-#include "java/JavaVersionList.h"
+#include "JavaCommon.h"
 #include "MultiMC.h"
 
+#include <java/JavaVersionList.h>
+
 InstanceSettingsPage::InstanceSettingsPage(BaseInstance *inst, QWidget *parent)
 	: QWidget(parent), ui(new Ui::InstanceSettingsPage), m_instance(inst)
 {
@@ -100,7 +101,7 @@ void InstanceSettingsPage::applySettings()
 	if(javaArgs)
 	{
 		m_settings->set("JvmArgs", ui->jvmArgsTextBox->toPlainText().replace("\n", " "));
-		NagUtils::checkJVMArgs(m_settings->get("JvmArgs").toString(), this->parentWidget());
+		JavaCommon::checkJVMArgs(m_settings->get("JvmArgs").toString(), this->parentWidget());
 	}
 	else
 	{
@@ -187,30 +188,18 @@ void InstanceSettingsPage::on_javaBrowseBtn_clicked()
 
 void InstanceSettingsPage::on_javaTestBtn_clicked()
 {
-	checker.reset(new JavaChecker());
-	connect(checker.get(), SIGNAL(checkFinished(JavaCheckResult)), this,
-			SLOT(checkFinished(JavaCheckResult)));
-	checker->path = ui->javaPathTextBox->text();
-	checker->performCheck();
+	if(checker)
+	{
+		return;
+	}
+	checker.reset(new JavaCommon::TestCheck(
+		this, ui->javaPathTextBox->text(), ui->jvmArgsTextBox->toPlainText().replace("\n", " "),
+		ui->minMemSpinBox->value(), ui->maxMemSpinBox->value(), ui->permGenSpinBox->value()));
+	connect(checker.get(), SIGNAL(finished()), SLOT(checkerFinished()));
+	checker->run();
 }
 
-void InstanceSettingsPage::checkFinished(JavaCheckResult result)
+void InstanceSettingsPage::checkerFinished()
 {
-	if (result.valid)
-	{
-		QString text;
-		text += "Java test succeeded!\n";
-		if (result.is_64bit)
-			text += "Using 64bit java.\n";
-		text += "\n";
-		text += "Platform reported: " + result.realPlatform;
-		QMessageBox::information(this, tr("Java test success"), text);
-	}
-	else
-	{
-		QMessageBox::warning(
-			this, tr("Java test failure"),
-			tr("The specified java binary didn't work. You should use the auto-detect feature, "
-			   "or set the path to the java executable."));
-	}
+	checker.reset();
 }

application/pages/InstanceSettingsPage.h

@@ -19,7 +19,9 @@
 
 #include "java/JavaChecker.h"
 #include "BaseInstance.h"
+#include <QObjectPtr.h>
 #include "BasePage.h"
+#include "JavaCommon.h"
 #include "MultiMC.h"
 
 class JavaChecker;
@@ -53,21 +55,20 @@ public:
 		return "Instance-settings";
 	}
 	virtual bool shouldDisplay() const;
+
 private slots:
 	void on_javaDetectBtn_clicked();
-
 	void on_javaTestBtn_clicked();
-
 	void on_javaBrowseBtn_clicked();
 
-	void checkFinished(JavaCheckResult result);
-
 	void applySettings();
 	void loadSettings();
 
+	void checkerFinished();
+
 private:
 	Ui::InstanceSettingsPage *ui;
 	BaseInstance *m_instance;
 	SettingsObject *m_settings;
-	std::shared_ptr<JavaChecker> checker;
+	QObjectPtr<JavaCommon::TestCheck> checker;
 };

application/pages/global/JavaPage.cpp

@@ -14,6 +14,7 @@
  */
 
 #include "JavaPage.h"
+#include "JavaCommon.h"
 #include "ui_JavaPage.h"
 
 #include <QFileDialog>
@@ -22,15 +23,11 @@
 
 #include <pathutils.h>
 
-#include "NagUtils.h"
-
-#include "Platform.h"
 #include "dialogs/VersionSelectDialog.h"
 #include <ColumnResizer.h>
 
 #include "java/JavaUtils.h"
 #include "java/JavaVersionList.h"
-#include "java/JavaChecker.h"
 
 #include "settings/SettingsObject.h"
 #include "MultiMC.h"
@@ -69,7 +66,7 @@ void JavaPage::applySettings()
 	// Java Settings
 	s->set("JavaPath", ui->javaPathTextBox->text());
 	s->set("JvmArgs", ui->jvmArgsTextBox->text());
-	NagUtils::checkJVMArgs(s->get("JvmArgs").toString(), this->parentWidget());
+	JavaCommon::checkJVMArgs(s->get("JvmArgs").toString(), this->parentWidget());
 
 	// Custom Commands
 	s->set("PreLaunchCommand", ui->preLaunchCmdTextBox->text());
@@ -113,33 +110,21 @@ void JavaPage::on_javaBrowseBtn_clicked()
 		ui->javaPathTextBox->setText(dir);
 	}
 }
+
 void JavaPage::on_javaTestBtn_clicked()
 {
-	checker.reset(new JavaChecker());
-	connect(checker.get(), SIGNAL(checkFinished(JavaCheckResult)), this,
-			SLOT(checkFinished(JavaCheckResult)));
-	checker->path = ui->javaPathTextBox->text();
-	checker->performCheck();
+	if(checker)
+	{
+		return;
+	}
+	checker.reset(new JavaCommon::TestCheck(
+		this, ui->javaPathTextBox->text(), ui->jvmArgsTextBox->text(),
+		ui->minMemSpinBox->value(), ui->maxMemSpinBox->value(), ui->permGenSpinBox->value()));
+	connect(checker.get(), SIGNAL(finished()), SLOT(checkerFinished()));
+	checker->run();
 }
 
-void JavaPage::checkFinished(JavaCheckResult result)
+void JavaPage::checkerFinished()
 {
-	if (result.valid)
-	{
-		QString text;
-		text += "Java test succeeded!\n";
-		if (result.is_64bit)
-			text += "Using 64bit java.\n";
-		text += "\n";
-		text += "Platform reported: " + result.realPlatform + "\n";
-		text += "Java version reported: " + result.javaVersion;
-		QMessageBox::information(this, tr("Java test success"), text);
-	}
-	else
-	{
-		QMessageBox::warning(
-			this, tr("Java test failure"),
-			tr("The specified java binary didn't work. You should use the auto-detect feature, "
-			   "or set the path to the java executable."));
-	}
+	checker.reset();
 }

application/pages/global/JavaPage.h

@@ -17,10 +17,10 @@
 
 #include <memory>
 #include <QDialog>
-
-#include "java/JavaChecker.h"
 #include "pages/BasePage.h"
+#include "JavaCommon.h"
 #include <MultiMC.h>
+#include <QObjectPtr.h>
 
 class SettingsObject;
 
@@ -64,10 +64,9 @@ slots:
 	void on_javaDetectBtn_clicked();
 	void on_javaTestBtn_clicked();
 	void on_javaBrowseBtn_clicked();
-
-	void checkFinished(JavaCheckResult result);
+	void checkerFinished();
 
 private:
 	Ui::JavaPage *ui;
-	std::shared_ptr<JavaChecker> checker;
+	QObjectPtr<JavaCommon::TestCheck> checker;
 };

application/pages/global/JavaPage.ui

@@ -161,45 +161,6 @@
             </property>
            </widget>
           </item>
-          <item row="1" column="1">
-           <widget class="QPushButton" name="javaDetectBtn">
-            <property name="sizePolicy">
-             <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
-              <horstretch>0</horstretch>
-              <verstretch>0</verstretch>
-             </sizepolicy>
-            </property>
-            <property name="text">
-             <string>Auto-detect...</string>
-            </property>
-           </widget>
-          </item>
-          <item row="1" column="2">
-           <widget class="QPushButton" name="javaTestBtn">
-            <property name="sizePolicy">
-             <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
-              <horstretch>0</horstretch>
-              <verstretch>0</verstretch>
-             </sizepolicy>
-            </property>
-            <property name="text">
-             <string>Test</string>
-            </property>
-           </widget>
-          </item>
-          <item row="2" column="0">
-           <widget class="QLabel" name="labelJVMArgs">
-            <property name="sizePolicy">
-             <sizepolicy hsizetype="Fixed" vsizetype="Preferred">
-              <horstretch>0</horstretch>
-              <verstretch>0</verstretch>
-             </sizepolicy>
-            </property>
-            <property name="text">
-             <string>JVM arguments:</string>
-            </property>
-           </widget>
-          </item>
           <item row="0" column="1" colspan="2">
            <layout class="QHBoxLayout" name="horizontalLayout">
             <item>
@@ -229,6 +190,45 @@
           <item row="2" column="1" colspan="2">
            <widget class="QLineEdit" name="jvmArgsTextBox"/>
           </item>
+          <item row="2" column="0">
+           <widget class="QLabel" name="labelJVMArgs">
+            <property name="sizePolicy">
+             <sizepolicy hsizetype="Fixed" vsizetype="Preferred">
+              <horstretch>0</horstretch>
+              <verstretch>0</verstretch>
+             </sizepolicy>
+            </property>
+            <property name="text">
+             <string>JVM arguments:</string>
+            </property>
+           </widget>
+          </item>
+          <item row="3" column="1">
+           <widget class="QPushButton" name="javaDetectBtn">
+            <property name="sizePolicy">
+             <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
+              <horstretch>0</horstretch>
+              <verstretch>0</verstretch>
+             </sizepolicy>
+            </property>
+            <property name="text">
+             <string>Auto-detect...</string>
+            </property>
+           </widget>
+          </item>
+          <item row="3" column="2">
+           <widget class="QPushButton" name="javaTestBtn">
+            <property name="sizePolicy">
+             <sizepolicy hsizetype="Preferred" vsizetype="Fixed">
+              <horstretch>0</horstretch>
+              <verstretch>0</verstretch>
+             </sizepolicy>
+            </property>
+            <property name="text">
+             <string>Test</string>
+            </property>
+           </widget>
+          </item>
          </layout>
         </widget>
        </item>
@@ -292,8 +292,6 @@
   <tabstop>permGenSpinBox</tabstop>
   <tabstop>javaPathTextBox</tabstop>
   <tabstop>javaBrowseBtn</tabstop>
-  <tabstop>javaDetectBtn</tabstop>
-  <tabstop>javaTestBtn</tabstop>
   <tabstop>jvmArgsTextBox</tabstop>
   <tabstop>preLaunchCmdTextBox</tabstop>
   <tabstop>postExitCmdTextBox</tabstop>

application/pages/global/MinecraftPage.cpp

@@ -23,18 +23,6 @@
 #include <pathutils.h>
 
 #include "Platform.h"
-#include "dialogs/VersionSelectDialog.h"
-#include "dialogs/CustomMessageBox.h"
-
-#include "NagUtils.h"
-
-#include "java/JavaUtils.h"
-#include "java/JavaVersionList.h"
-#include "java/JavaChecker.h"
-
-#include "updater/UpdateChecker.h"
-
-#include "tools/BaseProfiler.h"
 
 #include "settings/SettingsObject.h"
 #include "MultiMC.h"

application/pages/global/MultiMCPage.cpp

@@ -22,22 +22,9 @@
 #include <QTextCharFormat>
 
 #include <pathutils.h>
-
-#include "Platform.h"
-#include "dialogs/VersionSelectDialog.h"
-#include "dialogs/CustomMessageBox.h"
 #include <ColumnResizer.h>
-
-#include "NagUtils.h"
-
-#include "java/JavaUtils.h"
-#include "java/JavaVersionList.h"
-#include "java/JavaChecker.h"
-
 #include "updater/UpdateChecker.h"
 
-#include "tools/BaseProfiler.h"
-
 #include "settings/SettingsObject.h"
 #include "MultiMC.h"
 
@@ -456,4 +443,4 @@ void MultiMCPage::refreshFontPreview()
 		workCursor.insertText(tr("[Something/WARN] A not so spooky warning."), format);
 		workCursor.insertBlock();
 	}
-}
+}