Refactoring of the class CrashReporter

 - remove double code
 - using of all the methode of the CrashReporter-Class
 - all methode only active when crash-dump enable
    except the addKeyValue

With this change the handling for the patch
tdf#127711 A runtime-switch for the MiniCrashDump
would be simpler

Change-Id: I339b3b8e06f7fc2cd3c0d34ece112a6fd352913a
Reviewed-on: https://gerrit.libreoffice.org/79272
Tested-by: Jenkins
Reviewed-by: Juergen Funk (CIB) <juergen.funk_ml@cib.de>

5 files changed, 89 insertions, 79 deletions

diff --git a/desktop/source/app/app.cxx b/desktop/source/app/app.cxx
index 13fece37a5b1..b39f6931ec56 100644
--- a/desktop/source/app/app.cxx
+++ b/desktop/source/app/app.cxx
@@ -127,10 +127,6 @@
 #include <vcl/window.hxx>
 #include "langselect.hxx"
 
-#if HAVE_FEATURE_BREAKPAD
-#include <fstream>
-#endif
-
 #if defined MACOSX
 #include <errno.h>
 #include <sys/wait.h>
@@ -894,26 +890,6 @@ void Desktop::HandleBootstrapErrors(
 
 namespace {
 
-bool crashReportInfoExists()
-{
-#if HAVE_FEATURE_BREAKPAD
-    std::string path = CrashReporter::getIniFileName();
-    std::ifstream aFile(path);
-    while (aFile.good())
-    {
-        std::string line;
-        std::getline(aFile, line);
-        int sep = line.find('=');
-        if (sep >= 0)
-        {
-            std::string key = line.substr(0, sep);
-            if (key == "DumpFile")
-                return true;
-        }
-    }
-#endif
-    return false;
-}
 
 #if HAVE_FEATURE_BREAKPAD
 void handleCrashReport()
@@ -972,7 +948,12 @@ void impl_checkRecoveryState(bool& bCrashed           ,
                              bool& bRecoveryDataExists,
                              bool& bSessionDataExists )
 {
-    bCrashed = officecfg::Office::Recovery::RecoveryInfo::Crashed::get() || crashReportInfoExists();
+    bCrashed = officecfg::Office::Recovery::RecoveryInfo::Crashed::get()
+#if HAVE_FEATURE_BREAKPAD
+        || CrashReporter::crashReportInfoExists();
+#else
+        ;
+#endif
     bool elements = officecfg::Office::Recovery::RecoveryList::get()->
         hasElements();
     bool session
@@ -2005,7 +1986,7 @@ void Desktop::OpenClients()
 #endif
 
 #if HAVE_FEATURE_BREAKPAD
-    if (officecfg::Office::Common::Misc::CrashReport::get() && crashReportInfoExists())
+    if (officecfg::Office::Common::Misc::CrashReport::get() && CrashReporter::crashReportInfoExists())
         handleCrashReport();
 #endif
 
@@ -2083,11 +2064,9 @@ void Desktop::OpenClients()
             }
         }
     }
-#if HAVE_FEATURE_BREAKPAD
-    CrashReporter::writeCommonInfo();
+
     // write this information here to avoid depending on vcl in the crash reporter lib
-    CrashReporter::addKeyValue("Language", Application::GetSettings().GetLanguageTag().getBcp47());
-#endif
+    CrashReporter::addKeyValue("Language", Application::GetSettings().GetLanguageTag().getBcp47(), CrashReporter::Create);
 
     RequestHandler::EnableRequests();
 
diff --git a/desktop/source/app/crashreport.cxx b/desktop/source/app/crashreport.cxx
index 23868fe70c58..88cd6d77b0d3 100644
--- a/desktop/source/app/crashreport.cxx
+++ b/desktop/source/app/crashreport.cxx
@@ -14,17 +14,17 @@
 #include <ucbhelper/proxydecider.hxx>
 #include <unotools/bootstrap.hxx>
 #include <o3tl/char16_t2wchar_t.hxx>
+#include <desktop/minidump.hxx>
 
 #include <config_version.h>
 #include <config_folders.h>
 
 #include <string>
-#include <fstream>
 
-osl::Mutex CrashReporter::maMutex;
 
 #if HAVE_FEATURE_BREAKPAD
 
+#include <fstream>
 #if defined( UNX ) && !defined MACOSX && !defined IOS && !defined ANDROID
 #include <client/linux/handler/exception_handler.h>
 #elif defined WNT
@@ -38,41 +38,44 @@ osl::Mutex CrashReporter::maMutex;
 #endif
 #endif
 
+osl::Mutex CrashReporter::maMutex;
 google_breakpad::ExceptionHandler* CrashReporter::mpExceptionHandler = nullptr;
 bool CrashReporter::mbInit = false;
-std::map<OUString, OUString> CrashReporter::maKeyValues;
+CrashReporter::vmaKeyValues CrashReporter::maKeyValues;
 
-namespace {
 
-void writeToStream(std::ofstream& strm, const OUString& rKey, const OUString& rValue)
+void CrashReporter::writeToFile(std::ios_base::openmode Openmode)
 {
-    strm << OUStringToOString(rKey, RTL_TEXTENCODING_UTF8).getStr() << "=";
-    strm << OUStringToOString(rValue, RTL_TEXTENCODING_UTF8).getStr() << "\n";
-}
+    std::ofstream ini_file(getIniFileName(), Openmode);
+
+    for (auto& keyValue : maKeyValues)
+    {
+        ini_file << OUStringToOString(keyValue.first, RTL_TEXTENCODING_UTF8).getStr() << "=";
+        ini_file << OUStringToOString(keyValue.second, RTL_TEXTENCODING_UTF8).getStr() << "\n";
+    }
 
+    maKeyValues.clear();
+    ini_file.close();
 }
 
-void CrashReporter::addKeyValue(const OUString& rKey, const OUString& rValue)
+void CrashReporter::addKeyValue(const OUString& rKey, const OUString& rValue, tAddKeyHandling AddKeyHandling)
 {
     osl::MutexGuard aGuard(maMutex);
-    if (mbInit)
-    {
-        std::string ini_path = getIniFileName();
-        std::ofstream ini_file(ini_path, std::ios_base::app);
-        writeToStream(ini_file, rKey, rValue);
-    }
-    else
+
+    if(!rKey.isEmpty())
+        maKeyValues.push_back(mpair(rKey, rValue));
+
+    if(AddKeyHandling != AddItem)
     {
-        maKeyValues.insert(std::pair<OUString, OUString>(rKey, rValue));
+        if(mbInit)
+            writeToFile(std::ios_base::app);
+        else if (AddKeyHandling == Create)
+            writeCommonInfo();
     }
 }
 
-#endif
-
 void CrashReporter::writeCommonInfo()
 {
-    osl::MutexGuard aGuard(maMutex);
-
     ucbhelper::InternetProxyDecider proxy_decider(::comphelper::getProcessComponentContext());
 
     const OUString protocol = "https";
@@ -81,31 +84,31 @@ void CrashReporter::writeCommonInfo()
 
     const ucbhelper::InternetProxyServer proxy_server = proxy_decider.getProxy(protocol, url, port);
 
+    // save the Keys
+    vmaKeyValues atlast = maKeyValues;
+    maKeyValues.clear();
+
     // limit the amount of code that needs to be executed before the crash reporting
-    std::string ini_path = CrashReporter::getIniFileName();
-    std::ofstream minidump_file(ini_path, std::ios_base::trunc);
-    minidump_file << "ProductName=LibreOffice\n";
-    minidump_file << "Version=" LIBO_VERSION_DOTTED "\n";
-    minidump_file << "BuildID=" << utl::Bootstrap::getBuildIdData("") << "\n";
-    minidump_file << "URL=" << protocol << "://" << url << "/submit/\n";
+    addKeyValue("ProductName", "LibreOffice", AddItem);
+    addKeyValue("Version", LIBO_VERSION_DOTTED, AddItem);
+    addKeyValue("BuildID", utl::Bootstrap::getBuildIdData(""), AddItem);
+    addKeyValue("URL", protocol + "://" + url + "/submit/", AddItem);
 
     if (proxy_server.aName != OUString())
     {
-        minidump_file << "Proxy=" << proxy_server.aName << ":" << proxy_server.nPort << "\n";
+        addKeyValue("Proxy", proxy_server.aName + ":" + OUString::number(proxy_server.nPort), AddItem);
     }
 
-    for (auto& keyValue : maKeyValues)
-    {
-        writeToStream(minidump_file, keyValue.first, keyValue.second);
-    }
-    maKeyValues.clear();
-    minidump_file.close();
+    maKeyValues.insert(maKeyValues.end(), atlast.begin(), atlast.end());
 
     mbInit = true;
 
+    writeToFile(std::ios_base::trunc);
+
     updateMinidumpLocation();
 }
 
+
 namespace {
 
 OUString getCrashDirectory()
@@ -144,6 +147,25 @@ void CrashReporter::updateMinidumpLocation()
 #endif
 }
 
+bool CrashReporter::crashReportInfoExists()
+{
+    static bool first = true;
+    static bool InfoExist = false;
+
+    if (first)
+    {
+        first = false;
+        InfoExist = crashreport::readConfig(CrashReporter::getIniFileName(), nullptr);
+    }
+
+    return InfoExist;
+}
+
+bool CrashReporter::readSendConfig(std::string& response)
+{
+    return crashreport::readConfig(CrashReporter::getIniFileName(), &response);
+}
+
 void CrashReporter::storeExceptionHandler(google_breakpad::ExceptionHandler* pExceptionHandler)
 {
     mpExceptionHandler = pExceptionHandler;
@@ -157,4 +179,7 @@ std::string CrashReporter::getIniFileName()
     return aRet;
 }
 
+
+#endif //HAVE_FEATURE_BREAKPAD
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/desktop/source/app/sofficemain.cxx b/desktop/source/app/sofficemain.cxx
index 719446e03a81..70d396d232b6 100644
--- a/desktop/source/app/sofficemain.cxx
+++ b/desktop/source/app/sofficemain.cxx
@@ -45,7 +45,6 @@
 #include <unotools/mediadescriptor.hxx>
 
 #if HAVE_FEATURE_BREAKPAD
-#include <fstream>
 #include <desktop/crashreport.hxx>
 
 #if defined( UNX ) && !defined MACOSX && !defined IOS && !defined ANDROID
@@ -81,11 +80,9 @@
 #if defined( UNX ) && !defined MACOSX && !defined IOS && !defined ANDROID
 static bool dumpCallback(const google_breakpad::MinidumpDescriptor& descriptor, void* /*context*/, bool succeeded)
 {
-    std::string ini_path = CrashReporter::getIniFileName();
-    std::ofstream minidump_file(ini_path, std::ios_base::app);
-    minidump_file << "DumpFile=" << descriptor.path() << "\n";
-    minidump_file.close();
+    CrashReporter::addKeyValue("DumpFile", OStringToOUString(descriptor.path(), RTL_TEXTENCODING_UTF8), CrashReporter::Write);
     SAL_WARN("desktop", "minidump generated: " << descriptor.path());
+
     return succeeded;
 }
 #elif defined WNT
@@ -94,17 +91,14 @@ static bool dumpCallback(const wchar_t* path, const wchar_t* id,
                             MDRawAssertionInfo* /*assertion*/,
                             bool succeeded)
 {
-    std::string ini_path = CrashReporter::getIniFileName();
-    std::ofstream minidump_file(ini_path, std::ios_base::app);
     // TODO: moggi: can we avoid this conversion
 #ifdef _MSC_VER
 #pragma warning (disable: 4996)
 #endif
     std::wstring_convert<std::codecvt_utf8<wchar_t>> conv1;
     std::string aPath = conv1.to_bytes(std::wstring(path)) + conv1.to_bytes(std::wstring(id)) + ".dmp";
-    minidump_file << "DumpFile=" << aPath << "\n";
-    minidump_file << "GDIHandles=" << ::GetGuiResources (::GetCurrentProcess(), GR_GDIOBJECTS) << "\n";
-    minidump_file.close();
+    CrashReporter::addKeyValue("DumpFile", OStringToOUString(aPath.c_str(), RTL_TEXTENCODING_UTF8), CrashReporter::AddItem);
+    CrashReporter::addKeyValue("GDIHandles", OUString::number(::GetGuiResources (::GetCurrentProcess(), GR_GDIOBJECTS)), CrashReporter::Write);
     SAL_WARN("desktop", "minidump generated: " << aPath);
     return succeeded;
 }
diff --git a/desktop/source/minidump/minidump.cxx b/desktop/source/minidump/minidump.cxx
index f9666bff5998..8b96d2ff37f1 100644
--- a/desktop/source/minidump/minidump.cxx
+++ b/desktop/source/minidump/minidump.cxx
@@ -191,7 +191,7 @@ static bool uploadContent(std::map<std::string, std::string>& parameters, std::s
 
 namespace crashreport {
 
-bool readConfig(const std::string& iniPath, std::string& response)
+bool readConfig(const std::string& iniPath, std::string * response)
 {
     std::ifstream file(iniPath);
     std::map<std::string, std::string> parameters = readStrings(file);
@@ -199,17 +199,29 @@ bool readConfig(const std::string& iniPath, std::string& response)
     // make sure that at least the mandatory parameters are in there
     if (parameters.find("DumpFile") == parameters.end())
     {
-        response = "ini file needs to contain a key DumpFile!";
+        if(response != nullptr)
+            *response = "ini file needs to contain a key DumpFile!";
         return false;
     }
 
     if (parameters.find("Version") == parameters.end())
     {
-        response = "ini file needs to contain a key Version!";
+        if (response != nullptr)
+            *response = "ini file needs to contain a key Version!";
         return false;
     }
 
-    return uploadContent(parameters, response);
+    if (parameters.find("URL") == parameters.end())
+    {
+        if (response != nullptr)
+            *response = "ini file needs to contain a key URL!";
+        return false;
+    }
+
+    if (response != nullptr)
+        return uploadContent(parameters, *response);
+
+    return true;
 }
 
 }
diff --git a/desktop/source/minidump/minidump_upload.cxx b/desktop/source/minidump/minidump_upload.cxx
index ded2f1df0a43..15af26430764 100644
--- a/desktop/source/minidump/minidump_upload.cxx
+++ b/desktop/source/minidump/minidump_upload.cxx
@@ -22,7 +22,7 @@ int main(int argc, char** argv)
 
     std::string iniPath(argv[1]);
     std::string response;
-    if (!crashreport::readConfig(iniPath, response))
+    if (!crashreport::readConfig(iniPath, &response))
         return EXIT_FAILURE;
 
     std::cout << "Response: " << response << std::endl;