Improve crash reporter errors. b=385359, r=luser

toolkit/airbag/client/crashreporter.cpp

@@ -68,6 +68,22 @@ static string       gExtraFile;
 
 static string kExtraDataExtension = ".extra";
 
+void UIError(const string& message)
+{
+  string errorMessage;
+  if (!gStrings[ST_CRASHREPORTERERROR].empty()) {
+    char buf[2048];
+    UI_SNPRINTF(buf, 2048,
+                gStrings[ST_CRASHREPORTERERROR].c_str(),
+                message.c_str());
+    errorMessage = buf;
+  } else {
+    errorMessage = message;
+  }
+
+  UIError_impl(errorMessage);
+}
+
 static string Unescape(const string& str)
 {
   string ret;
@@ -210,14 +226,20 @@ static bool MoveCrashData(const string& toDir,
                           string& extrafile)
 {
   if (!UIEnsurePathExists(toDir)) {
+    UIError(gStrings[ST_ERROR_CREATEDUMPDIR]);
     return false;
   }
 
   string newDump = toDir + UI_DIR_SEPARATOR + Basename(dumpfile);
   string newExtra = toDir + UI_DIR_SEPARATOR + Basename(extrafile);
 
-  if (!UIMoveFile(dumpfile, newDump) ||
-      !UIMoveFile(extrafile, newExtra)) {
+  if (!UIMoveFile(dumpfile, newDump)) {
+    UIError(gStrings[ST_ERROR_DUMPFILEMOVE]);
+    return false;
+  }
+
+  if (!UIMoveFile(extrafile, newExtra)) {
+    UIError(gStrings[ST_ERROR_EXTRAFILEMOVE]);
     return false;
   }
 
@@ -288,13 +310,52 @@ bool SendCompleted(bool success, const string& serverResponse)
 
 using namespace CrashReporter;
 
+void RewriteStrings(StringTable& queryParameters)
+{
+  // rewrite some UI strings with the values from the query parameters
+  string product = queryParameters["ProductName"];
+  string vendor = queryParameters["Vendor"];
+  if (vendor.empty()) {
+    // Assume Mozilla if no vendor is specified
+    vendor = "Mozilla";
+  }
+
+  char buf[4096];
+  UI_SNPRINTF(buf, sizeof(buf),
+              gStrings[ST_CRASHREPORTERVENDORTITLE].c_str(),
+              vendor.c_str());
+  gStrings[ST_CRASHREPORTERTITLE] = buf;
+
+  // Leave a format specifier for UIError to fill in
+  UI_SNPRINTF(buf, sizeof(buf),
+              gStrings[ST_CRASHREPORTERPRODUCTERROR].c_str(),
+              product.c_str(),
+              "%s");
+  gStrings[ST_CRASHREPORTERERROR] = buf;
+
+  UI_SNPRINTF(buf, sizeof(buf),
+              gStrings[ST_CRASHREPORTERDESCRIPTION].c_str(),
+              product.c_str());
+  gStrings[ST_CRASHREPORTERDESCRIPTION] = buf;
+
+  UI_SNPRINTF(buf, sizeof(buf),
+              gStrings[ST_CHECKSUBMIT].c_str(),
+              vendor.c_str());
+  gStrings[ST_CHECKSUBMIT] = buf;
+
+  UI_SNPRINTF(buf, sizeof(buf),
+              gStrings[ST_RESTART].c_str(),
+              product.c_str());
+  gStrings[ST_RESTART] = buf;
+}
+
 int main(int argc, char** argv)
 {
   gArgc = argc;
   gArgv = argv;
 
   if (!ReadConfig()) {
-    UIError("Couldn't read configuration");
+    UIError("Couldn't read configuration.");
     return 0;
   }
 
@@ -311,36 +372,49 @@ int main(int argc, char** argv)
   } else {
     gExtraFile = GetExtraDataFilename(gDumpFile);
     if (gExtraFile.empty()) {
-      UIError("Couldn't get extra data filename");
+      UIError(gStrings[ST_ERROR_BADARGUMENTS]);
+      return 0;
+    }
+
+    if (!UIFileExists(gExtraFile)) {
+      UIError(gStrings[ST_ERROR_EXTRAFILEEXISTS]);
       return 0;
     }
 
     StringTable queryParameters;
     if (!ReadStringsFromFile(gExtraFile, queryParameters, true)) {
-      UIError("Couldn't read extra data");
+      UIError(gStrings[ST_ERROR_EXTRAFILEREAD]);
       return 0;
     }
 
     if (queryParameters.find("ProductName") == queryParameters.end()) {
-      UIError("No product name specified");
+      UIError(gStrings[ST_ERROR_NOPRODUCTNAME]);
       return 0;
     }
 
+    // There is enough information in the extra file to rewrite strings
+    // to be product specific
+    RewriteStrings(queryParameters);
+
     if (queryParameters.find("ServerURL") == queryParameters.end()) {
-      UIError("No server URL specified");
+      UIError(gStrings[ST_ERROR_NOSERVERURL]);
       return 0;
     }
 
     string product = queryParameters["ProductName"];
     string vendor = queryParameters["Vendor"];
     if (!UIGetSettingsPath(vendor, product, gSettingsPath)) {
-      UIError("Couldn't get settings path");
+      UIError(gStrings[ST_ERROR_NOSETTINGSPATH]);
+      return 0;
+    }
+
+    if (!UIFileExists(gDumpFile)) {
+      UIError(gStrings[ST_ERROR_DUMPFILEEXISTS]);
       return 0;
     }
 
     string pendingDir = gSettingsPath + UI_DIR_SEPARATOR + "pending";
     if (!MoveCrashData(pendingDir, gDumpFile, gExtraFile)) {
-      UIError("Couldn't move crash data");
       return 0;
     }
 
@@ -370,28 +444,6 @@ int main(int argc, char** argv)
       sendURL = urlEnv;
     }
 
-    // rewrite some UI strings with the values from the query parameters
-    char buf[4096];
-    UI_SNPRINTF(buf, sizeof(buf),
-                gStrings[ST_RESTART].c_str(),
-                product.c_str());
-    gStrings[ST_RESTART] = buf;
-
-    UI_SNPRINTF(buf, sizeof(buf),
-                gStrings[ST_CRASHREPORTERDESCRIPTION].c_str(),
-                product.c_str());
-    gStrings[ST_CRASHREPORTERDESCRIPTION] = buf;
-
-    UI_SNPRINTF(buf, sizeof(buf),
-                gStrings[ST_CHECKSUBMIT].c_str(),
-                vendor.empty() ? "Mozilla" : vendor.c_str());
-    gStrings[ST_CHECKSUBMIT] = buf;
-
-    UI_SNPRINTF(buf, sizeof(buf),
-                gStrings[ST_CRASHREPORTERTITLE].c_str(),
-                vendor.empty() ? "Mozilla" : vendor.c_str());
-    gStrings[ST_CRASHREPORTERTITLE] = buf;
-
     UIShowCrashUI(gDumpFile, queryParameters, sendURL, restartArgs);
   }
 

toolkit/airbag/client/crashreporter.h

@@ -29,17 +29,31 @@
 
 typedef std::map<std::string, std::string> StringTable;
 
-#define ST_CRASHREPORTERTITLE       "CrashReporterTitle"
-#define ST_CRASHREPORTERHEADER      "CrashReporterHeader"
-#define ST_CRASHREPORTERDESCRIPTION "CrashReporterDescription"
-#define ST_CRASHREPORTERDEFAULT     "CrashReporterDefault"
-#define ST_VIEWREPORT               "ViewReport"
-#define ST_EXTRAREPORTINFO          "ExtraReportInfo"
-#define ST_CHECKSUBMIT              "CheckSubmit"
-#define ST_CHECKEMAIL               "CheckEmail"
-#define ST_CLOSE                    "Close"
-#define ST_RESTART                  "Restart"
-#define ST_SUBMITFAILED             "SubmitFailed"
+#define ST_CRASHREPORTERTITLE        "CrashReporterTitle"
+#define ST_CRASHREPORTERVENDORTITLE  "CrashReporterVendorTitle"
+#define ST_CRASHREPORTERERROR        "CrashReporterError"
+#define ST_CRASHREPORTERPRODUCTERROR "CrashReporterProductError"
+#define ST_CRASHREPORTERHEADER       "CrashReporterHeader"
+#define ST_CRASHREPORTERDESCRIPTION  "CrashReporterDescription"
+#define ST_CRASHREPORTERDEFAULT      "CrashReporterDefault"
+#define ST_VIEWREPORT                "ViewReport"
+#define ST_EXTRAREPORTINFO           "ExtraReportInfo"
+#define ST_CHECKSUBMIT               "CheckSubmit"
+#define ST_CHECKEMAIL                "CheckEmail"
+#define ST_CLOSE                     "Close"
+#define ST_RESTART                   "Restart"
+#define ST_SUBMITFAILED              "SubmitFailed"
+
+#define ST_ERROR_BADARGUMENTS        "ErrorBadArguments"
+#define ST_ERROR_EXTRAFILEEXISTS     "ErrorExtraFileExists"
+#define ST_ERROR_EXTRAFILEREAD       "ErrorExtraFileRead"
+#define ST_ERROR_EXTRAFILEMOVE       "ErrorExtraFileMove"
+#define ST_ERROR_DUMPFILEEXISTS      "ErrorDumpFileExists"
+#define ST_ERROR_DUMPFILEMOVE        "ErrorDumpFileMove"
+#define ST_ERROR_NOPRODUCTNAME       "ErrorNoProductName"
+#define ST_ERROR_NOSERVERURL         "ErrorNoServerURL"
+#define ST_ERROR_NOSETTINGSPATH      "ErrorNoSettingsPath"
+#define ST_ERROR_CREATEDUMPDIR       "ErrorCreateDumpDir"
 
 //=============================================================================
 // implemented in crashreporter.cpp
@@ -51,6 +65,8 @@ namespace CrashReporter {
   extern int          gArgc;
   extern char**       gArgv;
 
+  void UIError(const std::string& message);
+
   // The UI finished sending the report
   bool SendCompleted(bool success, const std::string& serverResponse);
 
@@ -87,13 +103,14 @@ void UIShowCrashUI(const std::string& dumpfile,
                    const std::string& sendURL,
                    const std::vector<std::string>& restartArgs);
 
-void UIError(const std::string& message);
+void UIError_impl(const std::string& message);
 
 bool UIGetIniPath(std::string& path);
 bool UIGetSettingsPath(const std::string& vendor,
                        const std::string& product,
                        std::string& settingsPath);
 bool UIEnsurePathExists(const std::string& path);
+bool UIFileExists(const std::string& path);
 bool UIMoveFile(const std::string& oldfile, const std::string& newfile);
 bool UIDeleteFile(const std::string& oldfile);
 

toolkit/airbag/client/crashreporter.ini

@@ -1,15 +0,0 @@
-; This file is in the UTF-8 encoding
-[Strings]
-CrashReporterTitle=%s Crash Reporter
-CrashReporterHeader=Crash! Bang! Boom!
-CrashReporterDescription=We're sorry, but %s hit an unexpected problem and crashed.  We'll try to restore your tabs and windows when it restarts.\n\nTo help us diagnose and repair this problem, you can send us a crash report.
-CrashReporterDefault=This application is run after a crash to report the problem to the application vendor.  It should not be run directly.
-ViewReport=View Report
-ExtraReportInfo=This report also contains information about the state of the application when it crashed.
-CheckSubmit=Submit crash report to %s
-CheckEmail=Email me when the problem is fixed
-Close=Close
-Restart=Restart %s
-SubmitFailed=Failed to submit crash report
-CrashID=Crash ID: %s
-CrashDetailsURL=You can view details of this crash at %s

toolkit/airbag/client/crashreporter_linux.cpp

@@ -412,7 +412,7 @@ void UIShowCrashUI(const string& dumpfile,
   gtk_main();
 }
 
-void UIError(const string& message)
+void UIError_impl(const string& message)
 {
   if (!gInitialized) {
     // Didn't initialize, this is the best we can do
@@ -477,6 +477,16 @@ bool UIEnsurePathExists(const string& path)
   return true;
 }
 
+bool UIFileExists(const string& path)
+{
+  struct stat sb;
+  int ret = stat(path.c_str(), &sb);
+  if (ret == -1 || !(sb.st_mode & S_IFREG))
+    return false;
+
+  return true;
+}
+
 bool UIMoveFile(const string& file, const string& newfile)
 {
   return (rename(file.c_str(), newfile.c_str()) != -1);

toolkit/airbag/client/crashreporter_osx.mm

@@ -428,7 +428,8 @@ void UIShutdown()
 
 void UIShowDefaultUI()
 {
-  UIError(gStrings[ST_CRASHREPORTERDEFAULT]);
+  [gUI showErrorUI: gStrings[ST_CRASHREPORTERDEFAULT]];
+  [NSApp run];
 }
 
 void UIShowCrashUI(const string& dumpfile,
@@ -444,7 +445,7 @@ void UIShowCrashUI(const string& dumpfile,
   [NSApp run];
 }
 
-void UIError(const string& message)
+void UIError_impl(const string& message)
 {
   if (!gUI) {
     // UI failed to initialize, printing is the best we can do
@@ -502,6 +503,16 @@ bool UIEnsurePathExists(const string& path)
   return true;
 }
 
+bool UIFileExists(const string& path)
+{
+  struct stat sb;
+  int ret = stat(path.c_str(), &sb);
+  if (ret == -1 || !(sb.st_mode & S_IFREG))
+    return false;
+
+  return true;
+}
+
 bool UIMoveFile(const string& file, const string& newfile)
 {
   return (rename(file.c_str(), newfile.c_str()) != -1);

toolkit/airbag/client/crashreporter_win.cpp

@@ -706,7 +706,7 @@ void UIShowCrashUI(const string& dumpFile,
                  (DLGPROC)CrashReporterDialogProc, 0);
 }
 
-void UIError(const string& message)
+void UIError_impl(const string& message)
 {
   wstring title = Str(ST_CRASHREPORTERTITLE);
   if (title.empty())
@@ -765,6 +765,12 @@ bool UIEnsurePathExists(const string& path)
   return true;
 }
 
+bool UIFileExists(const string& path)
+{
+  DWORD attrs = GetFileAttributes(UTF8ToWide(path).c_str());
+  return (attrs != INVALID_FILE_ATTRIBUTES);
+}
+
 bool UIMoveFile(const string& oldfile, const string& newfile)
 {
   if (oldfile == newfile)