From 09df7a12c631f5484d88df80927b7733af6cbed5 Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Mon, 22 Jul 2019 13:30:26 +0100
Subject: [PATCH] Improve poorly written Android install failure messages.
 Closes #1459

* We also add a new message specifically for when the install succeeds but we
  can't verify it, to indicate the problem better than suggesting that
  permission errors are at fault.
---
 qrenderdoc/Windows/MainWindow.cpp   | 33 +++++++++++++++++------------
 renderdoc/android/android.cpp       | 19 +++++++++++++++--
 renderdoc/api/replay/replay_enums.h |  9 ++++++--
 3 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/qrenderdoc/Windows/MainWindow.cpp b/qrenderdoc/Windows/MainWindow.cpp
index 431529891..a5c08ef1b 100644
--- a/qrenderdoc/Windows/MainWindow.cpp
+++ b/qrenderdoc/Windows/MainWindow.cpp
@@ -2878,31 +2878,38 @@ bool MainWindow::LoadLayout(int layout)
 
 void MainWindow::showLaunchError(ReplayStatus status)
 {
-  QString title;
   QString message;
   switch(status)
   {
     case ReplayStatus::AndroidGrantPermissionsFailed:
-      title = tr("Permission is required");
-      message = tr("Enable RenderDocCmd to access storage on your device.");
+      message =
+          tr("Failed to automatically grant Android permissions to installed server.\n\n"
+             "Please manually allow the RenderDocCmd program storage permissions on your device "
+             "to ensure correct functionality.");
       break;
     case ReplayStatus::AndroidABINotFound:
-      title = tr("Failed to install RenderDoc server");
-      message = tr("Couldn't determine supported ABIs.");
+      message =
+          tr("Couldn't determine supported ABIs for your device, please check device connection "
+             "and status.");
       break;
     case ReplayStatus::AndroidAPKFolderNotFound:
-      title = tr("Failed to install RenderDoc server");
-      message = tr("APK folder missing.");
+      message = tr("Couldn't find APK folder, please check that your installation is complete.");
       break;
     case ReplayStatus::AndroidAPKInstallFailed:
-      title = tr("Failed to install RenderDoc server");
-      message = tr("Couldn't find any installed APKs.");
-    default:
-      title = tr("Failed to install RenderDoc server");
-      message = tr("Unknown error.");
+      message =
+          tr("Couldn't install APK, please check that your device is connected and accessible to "
+             "adb.");
+    case ReplayStatus::AndroidAPKVerifyFailed:
+      message =
+          tr("Couldn't correctly verify installed APK version.\n\n"
+             "Please check your installation is not corrupted, or if this is a custom build check "
+             "that all ABIs are built at the same version as this program.");
       break;
+    default: message = tr("Unexpected error: %1.").arg(ToQStr(status)); break;
   }
-  GUIInvoke::call(this, [this, title, message]() { RDDialog::warning(this, title, message); });
+  GUIInvoke::call(this, [this, message]() {
+    RDDialog::warning(this, tr("Problems installing RenderDoc server"), message);
+  });
 }
 
 bool MainWindow::isCapturableAppRunningOnAndroid()
diff --git a/renderdoc/android/android.cpp b/renderdoc/android/android.cpp
index 09873fa22..ddfc0fa7b 100644
--- a/renderdoc/android/android.cpp
+++ b/renderdoc/android/android.cpp
@@ -480,11 +480,25 @@ ReplayStatus InstallRenderDocServer(const std::string &deviceID)
 
     if(!success)
     {
-      status = ReplayStatus::AndroidGrantPermissionsFailed;
       RDCLOG("Failed to install APK. stdout: %s, stderr: %s", trim(adbInstall.strStdout).c_str(),
              trim(adbInstall.strStderror).c_str());
       RDCLOG("Retrying...");
       adbExecCommand(deviceID, "install -r \"" + apk + "\"");
+
+      success = CheckAndroidServerVersion(deviceID, abi);
+
+      if(success)
+      {
+        // if it succeeded this time, then it was the permission grant that failed
+        status = ReplayStatus::AndroidGrantPermissionsFailed;
+      }
+      else
+      {
+        // otherwise something went wrong with verifying. If the install failed completely we'll
+        // return AndroidAPKInstallFailed below, otherwise return a code indicating we couldn't
+        // verify the install properly.
+        status = ReplayStatus::AndroidAPKVerifyFailed;
+      }
     }
   }
 
@@ -653,7 +667,8 @@ extern "C" RENDERDOC_API ReplayStatus RENDERDOC_CC RENDERDOC_StartAndroidRemoteS
 
     // If server is not detected or has been removed due to incompatibility, install it
     status = Android::InstallRenderDocServer(deviceID);
-    if(status != ReplayStatus::Succeeded && status != ReplayStatus::AndroidGrantPermissionsFailed)
+    if(status != ReplayStatus::Succeeded && status != ReplayStatus::AndroidGrantPermissionsFailed &&
+       status != ReplayStatus::AndroidAPKVerifyFailed)
     {
       RDCERR("Failed to install RenderDoc server app");
       return status;
diff --git a/renderdoc/api/replay/replay_enums.h b/renderdoc/api/replay/replay_enums.h
index e6cd2bdb6..df61c1de7 100644
--- a/renderdoc/api/replay/replay_enums.h
+++ b/renderdoc/api/replay/replay_enums.h
@@ -3204,10 +3204,14 @@ a remote server.
 
 .. data:: AndroidAPKFolderNotFound
 
-  Couldn't find the build-android folder which contains the Android remote server APK.
+  Couldn't find the folder which contains the Android remote server APK.
 
 .. data:: AndroidAPKInstallFailed
 
+  Failed to install Android remote server for unknown reasons.
+
+.. data:: AndroidAPKVerifyFailed
+
   Failed to install Android remote server.
 )");
 enum class ReplayStatus : uint32_t
@@ -3235,7 +3239,8 @@ enum class ReplayStatus : uint32_t
   AndroidGrantPermissionsFailed,
   AndroidABINotFound,
   AndroidAPKFolderNotFound,
-  AndroidAPKInstallFailed
+  AndroidAPKInstallFailed,
+  AndroidAPKVerifyFailed,
 };
 
 DECLARE_REFLECTION_ENUM(ReplayStatus);