Created attachment 44849 [details]
complex.sfx2.UndoManager failure

Make all bugs with whiteboard status "unoapitest" and "complextest" block the 35690 subsequenttests metabug.

All subsequenttest bugs now block their own subsequenttests metabug:

 https://bugs.freedesktop.org/show_bug.cgi?id=35690

=> Removing 35657, 35660, 35661, 35661, 35662, 35663, 35666, 35667 as blockers from 35673

35668 stays as it is a reproducable crasher.

I added another assert:
http://cgit.freedesktop.org/libreoffice/libs-core/commit/?id=1151eaf2612c0037d7bbdd5c45a54b0346377f20

I seems like unlike in other app performing the simple operation unlocks the UndoManager.

@kohei: Could you have a look?

(In reply to comment #4)

> @kohei: Could you have a look?

A look at what exactly?  I didn't modify UndoManager.

Also, what does this test failure mean?

The asserting at:

http://cgit.freedesktop.org/libreoffice/libs-core/tree/sfx2/qa/complex/sfx2/UndoManager.java?id=1151eaf2612c0037d7bbdd5c45a54b0346377f20#n784

fails for Calc and only for Calc. It is fine for other apps. The code locks the undo manager, performs some basic action(*), and wants to unlock the undo manager, which fails because the undo manager was unlocked as a side effect of the operation already. So changing a value in a cell in calc has weird and unexpected effects on the undo manager.


(*) for Calc: http://cgit.freedesktop.org/libreoffice/libs-core/tree/sfx2/qa/complex/sfx2/undo/CalcDocumentTest.java?id=1151eaf2612c0037d7bbdd5c45a54b0346377f20#n43
writing a different value in A1

Ah, yes.  I remember this.  I did that because the Calc code by and large was not ready for this ref-counted locking of the new undo manager, and to change that means substantial change in how Calc handles undo operations in general.  Without that, undo becomes disabled at all times plus causes crash.

Looks like when this undo manager change was made in OOo, Calc's code was not really adopted fully and we are paying the price for that now. *sigh*

I also don't know exactly what the new undo manager expects to do.  Why does it have to do refcounted locking anyway?

see: http://comments.gmane.org/gmane.comp.openoffice.devel.api/21128
for the whole story

[This is an automated message.]
This bug was filed before the changes to Bugzilla on 2011-10-16. Thus it
started right out as NEW without ever being explicitly confirmed. The bug is
changed to state NEEDINFO for this reason. To move this bug from NEEDINFO back
to NEW please check if the bug still persists with the 3.5.0 beta1 or beta2 prereleases.
Details on how to test the 3.5.0 beta1 can be found at:
http://wiki.documentfoundation.org/QA/BugHunting_Session_3.5.0.-1

more detail on this bulk operation: http://nabble.documentfoundation.org/RFC-Operation-Spamzilla-tp3607474p3607474.html

Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=54fbcc39a794025fb419addcd86080c7db308235

fdo#35663: enable UndoManager test, but disable failing Calc test

ah there is already a bug about this; so indeed only the Calc part
of the UndoManager test fails.

note that last week some commits from CWS fs34b were merged that
changed the SfxUndoManager to no longer count the DisableUndo calls
but instead maintain a simple boolean flag.

Kohei, could you please check if that changes anything for
Calc, and if you can make the test work again?

(In reply to comment #11)
> ah there is already a bug about this; so indeed only the Calc part
> of the UndoManager test fails.
> 
> note that last week some commits from CWS fs34b were merged that
> changed the SfxUndoManager to no longer count the DisableUndo calls
> but instead maintain a simple boolean flag.
> 
> Kohei, could you please check if that changes anything for
> Calc, and if you can make the test work again?

I re-enabled the test and ran make check, but I still get this:

finished class: complex.sfx2.UndoManager
--------------------------------------------------------------------------------

Time: 14.171
There was 1 failure:
1) checkCalcUndo(complex.sfx2.UndoManager)
java.lang.AssertionError: Undo manager gets unlocked as a side effect of performing a simple operation
	at complex.sfx2.UndoManager.impl_testLocking(UndoManager.java:774)
	at complex.sfx2.UndoManager.impl_checkUndo(UndoManager.java:630)
	at complex.sfx2.UndoManager.checkCalcUndo(UndoManager.java:125)

FAILURES!!!
Tests run: 12,  Failures: 1

So, I guess the coast is not yet clear.

Bug is still very much alive, I see fails in checkBrokenScripts and checkSerialization on builders -- so doesnt seem to be confined to calc as:

http://cgit.freedesktop.org/libreoffice/core/commit/?id=54fbcc39a794025fb419addcd86080c7db308235

suggests.