vcl: refactor ErrorHandler functions

Current, HandleError_Impl is called on by GetErrorString, and
HandleError_Impl also creates the error string. However, when
you look at what each function does, HandleError_Impl has been
imbued with some extraneous parameters:

1. bJustCreateString, and
2. rError

bJustCreateString is unnecessary, in fact it just mutates
HandleError to create the string, which is not really the core
purpose of this function. In fact, the string should be created
when we get the string from the error ID (GetErrorString), and
in fact the error handler function should call on GetErrorString
and not the other way around!

This has the happy side effect of vastly simplifying this code,
and allows each of the functions only do the thing they are
meant to do, and we no longer need these extraneous parameters
which just cause problems for code readibility. On to of this,
by simplifying the HandleError_Impl function, we just move the
code into the public HandleError function because these
parameters aren't necessary any more.

Change-Id: I4a727b5bbc6d3cdb0519f49b48dc52f8a8976629
Reviewed-on: https://gerrit.libreoffice.org/36849
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Chris Sherlock <chris.sherlock79@gmail.com>

2 files changed, 47 insertions, 58 deletions

diff --git a/include/vcl/errinf.hxx b/include/vcl/errinf.hxx
index 05159b737606..2820729a3676 100644
--- a/include/vcl/errinf.hxx
+++ b/include/vcl/errinf.hxx
@@ -151,14 +151,6 @@ class SAL_WARN_UNUSED VCL_DLLPUBLIC ErrorHandler
 {
     friend class ErrorHandler_Impl;
 
-private:
-    static DialogMask HandleError_Impl( sal_uInt32 lId,
-                              DialogMask nFlags,
-                              bool bJustCreateString,
-                              OUString & rError);
-protected:
-    virtual bool        CreateString(const ErrorInfo *, OUString &) const = 0;
-
 public:
                         ErrorHandler();
     virtual             ~ErrorHandler();
@@ -168,6 +160,10 @@ public:
 
     static void         RegisterDisplay( BasicDisplayErrorFunc* );
     static void         RegisterDisplay( WindowDisplayErrorFunc* );
+
+protected:
+    virtual bool        CreateString(const ErrorInfo *, OUString &) const = 0;
+
 };
 
 #endif
diff --git a/vcl/source/window/errinf.cxx b/vcl/source/window/errinf.cxx
index 3c543cd3a4f8..2f73cd416437 100644
--- a/vcl/source/window/errinf.cxx
+++ b/vcl/source/window/errinf.cxx
@@ -224,11 +224,29 @@ void ErrorHandler::RegisterDisplay(BasicDisplayErrorFunc *aDsp)
     rData.pDsp = reinterpret_cast< DisplayFnPtr >(aDsp);
 }
 
+bool ErrorHandler::GetErrorString(sal_uInt32 nErrCodeId, OUString& rErrStr)
+{
+    OUString aErr;
+
+    if(!nErrCodeId || nErrCodeId == ERRCODE_ABORT)
+        return false;
+
+    ErrorInfo *pInfo = ErrorInfo::GetErrorInfo(nErrCodeId);
+
+    if (ErrorHandler_Impl::CreateString(pInfo,aErr))
+    {
+        rErrStr = aErr;
+        return true;
+    }
+
+    delete pInfo;
+    return false;
+}
+
 /** Handles an error.
 
     If nFlags is not set, the DynamicErrorInfo flags or the
-    resource flags will be used.
-    Thus:
+    resource flags will be used. Thus the order is:
 
     1. nFlags,
     2. Resource Flags
@@ -237,23 +255,19 @@ void ErrorHandler::RegisterDisplay(BasicDisplayErrorFunc *aDsp)
 
     @param nErrCodeId        error id
     @param nFlags            error flags.
-    @param bJustCreateString ???
-    @param rError            ???
 
-    @return ???
+    @return what sort of dialog to use, with what buttons
 */
-DialogMask ErrorHandler::HandleError_Impl(
-    sal_uInt32 nErrCodeId, DialogMask nFlags, bool bJustCreateString, OUString & rError)
+DialogMask ErrorHandler::HandleError(sal_uInt32 nErrCodeId, DialogMask nFlags)
 {
-    OUString aErr;
-    OUString aAction;
-
     if(!nErrCodeId || nErrCodeId == ERRCODE_ABORT)
         return DialogMask::NONE;
 
     ErrorRegistry &rData = TheErrorRegistry::get();
     vcl::Window *pParent = nullptr;
     ErrorInfo *pInfo = ErrorInfo::GetErrorInfo(nErrCodeId);
+    OUString aAction;
+
     if (!rData.contexts.empty())
     {
         rData.contexts.front()->GetString(pInfo->GetErrorCode(), aAction);
@@ -283,46 +297,40 @@ DialogMask ErrorHandler::HandleError_Impl(
             nErrFlags = nDynFlags;
     }
 
-    if(ErrorHandler_Impl::CreateString(pInfo,aErr))
+    OUString aErr;
+    if (ErrorHandler::GetErrorString(nErrCodeId, aErr))
     {
-        if (bJustCreateString)
+        if(!rData.pDsp)
         {
-            rError = aErr;
-            return DialogMask::ButtonsOk;
+            OStringBuffer aStr("Action: ");
+            aStr.append(OUStringToOString(aAction, RTL_TEXTENCODING_ASCII_US));
+            aStr.append("\nError: ");
+            aStr.append(OUStringToOString(aErr, RTL_TEXTENCODING_ASCII_US));
+            OSL_FAIL(aStr.getStr());
         }
         else
         {
-            if(!rData.pDsp)
+            delete pInfo;
+
+            if(!rData.bIsWindowDsp)
             {
-                OStringBuffer aStr("Action: ");
-                aStr.append(OUStringToOString(aAction, RTL_TEXTENCODING_ASCII_US));
-                aStr.append("\nError: ");
-                aStr.append(OUStringToOString(aErr, RTL_TEXTENCODING_ASCII_US));
-                OSL_FAIL(aStr.getStr());
+                (*reinterpret_cast<BasicDisplayErrorFunc*>(rData.pDsp))(aErr,aAction);
+                return DialogMask::NONE;
             }
             else
             {
-                delete pInfo;
-                if(!rData.bIsWindowDsp)
-                {
-                    (*reinterpret_cast<BasicDisplayErrorFunc*>(rData.pDsp))(aErr,aAction);
-                    return DialogMask::NONE;
-                }
-                else
-                {
-                    if (nFlags != DialogMask::MAX)
-                        nErrFlags = nFlags;
-                    return (*reinterpret_cast<WindowDisplayErrorFunc*>(rData.pDsp))(
-                        pParent, nErrFlags, aErr, aAction);
-                }
+                if (nFlags != DialogMask::MAX)
+                    nErrFlags = nFlags;
+                return (*reinterpret_cast<WindowDisplayErrorFunc*>(rData.pDsp))(
+                    pParent, nErrFlags, aErr, aAction);
             }
         }
     }
 
     OSL_FAIL("Error not handled");
     // Error 1 is classified as a General Error in sfx
-    if(pInfo->GetErrorCode()!=1)
-        HandleError_Impl(1, DialogMask::MAX, bJustCreateString, rError);
+    if (pInfo->GetErrorCode()!=1)
+        HandleError(1);
     else
         OSL_FAIL("Error 1 not handled");
 
@@ -330,21 +338,6 @@ DialogMask ErrorHandler::HandleError_Impl(
     return DialogMask::NONE;
 }
 
-bool ErrorHandler::GetErrorString(sal_uInt32 lId, OUString& rStr)
-{
-    return HandleError_Impl( lId, DialogMask::MAX, true, rStr ) != DialogMask::NONE;
-}
-
-/** Handles an error.
-
-    @see ErrorHandler::HandleError_Impl
-*/
-DialogMask ErrorHandler::HandleError(sal_uInt32 lId, DialogMask nFlags)
-{
-    OUString aDummy;
-    return HandleError_Impl( lId, nFlags, false, aDummy );
-}
-
 bool ErrorHandler_Impl::CreateString(const ErrorInfo* pInfo, OUString& rStr)
 {
     for(const ErrorHandler *pHdl : TheErrorRegistry::get().errorHandlers)