Re: [REVIEW 3-6] fdo#43989 Automatic restart on Windows

On 30/07/12 11:04, Stephan Bergmann wrote:
> Backporting the fix for 
> <https://bugs.freedesktop.org/show_bug.cgi?id=51252> "LO cannot start 
> (reports runtime error with Visual C++ Runtime Library)" to LO 3.6.0 
> (which causes LO to routinely try to restart itself now after an 
> upgrade on Windows) unfortunately revealed that the fix 
> <http://cgit.freedesktop.org/libreoffice/core/commit/?id=55836370ffb70a34b888f81cdacdfede8fee29cf> 
> "fdo#43989: Revert 'win32-dont-attempt-restart.diff: Don't attempt to 
> restart OOo after crash'" for 
> <https://bugs.freedesktop.org/show_bug.cgi?id=43989> "After 
> installation first launch terminates after splash screen" should have 
> been backported as well.  (And the latter undoes a broken fix that has 
> been corrected on master with 
> <http://cgit.freedesktop.org/libreoffice/core/commit/?id=b5ad981518f4407243f89507b2561dd0b1ef28ee> 
> "bnc#524250: Show backing component after cancelled recovery.")
>
> The net effect is that, on Windows, the first start of LO 3.6.0 (both 
> a completely new installation, as well as after an upgrade from older 
> versions) silently terminates after the splash screen.  The user needs 
> to start LO anew.  While this is unfortunate, it is IMO only a minor 
> annoyance with a trivial workaround, so does not warrant yet another 
> LO 3.6.0 RC5.  (See 
> <https://bugs.freedesktop.org/show_bug.cgi?id=51252#c58>.)
>
> So, please review for cherry-picking into libreoffice-3-6 the two commits
>
> <http://cgit.freedesktop.org/libreoffice/core/commit/?id=b5ad981518f4407243f89507b2561dd0b1ef28ee> 
> "bnc#524250: Show backing component after cancelled recovery"

@@ -1203,24 +1203,19 @@ sal_Bool impl_callRecoveryUI(sal_Bool 
bEmergencySave ,
css::util::URL aURL;
if (bEmergencySave)
[...]
- return bRet;
+ return !bEmergencySave || bRet;

is that what you really want?, seems suspicious because if 
bEmergencySave is passed in false ( which it is in the case of this patch )

- impl_callRecoveryUI(
+ bRecovery = impl_callRecoveryUI(
sal_False , // false => force recovery instead of emergency save
bCrashed ,
bExistsRecoveryData);

then impl_callRevoveryUI  always going to return true in which case I 
would have just expected that bRecovery would have been just set to true 
instead of the result of impl_callRecoveryUI.


Noel