Bug 97527 - vcl: reference-count Menu ...
Summary: vcl: reference-count Menu ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
5.1.0.1 rc
Hardware: All All
: medium normal
Assignee: Noel Grandin
QA Contact:
URL:
Whiteboard: target:5.2.0 target:5.3.0
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-03 13:50 UTC by Michael Meeks
Modified: 2016-10-04 06:47 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2016-02-03 13:50:13 UTC
We have a nice VclPtr<> template for reference counting the odd vcl/ types that we have around the place. However it currently is only good for counting OutputDevice derivatives.

That is a shame - we really need to do the same conversion of delete -> dispose, and handling references properly for a number of other types both internal (in vcl/ backends) and external.

To do that we need to split the VclPtr pieces out of OutDev into another base-class; say: "VclReferenceBase" or something (?) - you'll see the acquire, delete, mutable mnRef also these guys:

protected:
    /// release all references to other objects.
    virtual void                dispose();

public:
    /// call the dispose() method if we have not already been disposed.
    void                        disposeOnce();
    bool                        isDisposed() const { return mbDisposed; }

So - step #1 is to factor that out into a nice base-class and inherit 'OutputDevice' from that - then re-run 'make check' etc. ;-)

Step #2 of this - is to then convert the next class to use that; a good target might be: Menu

so:

class VCL_DLLPUBLIC Menu : public ::VclReferenceBase, public Resource

or somesuch - and then switch all Menu instances on the stack, or as members into VclPtr< Menu >'s - and ... well everything do as for the VclPtr<> cleanup - but for Menu classes =)

Hopefully then we can move onto the internal structures in VCL too.

Thanks ! =)
Comment 1 Chris Sherlock 2016-02-03 13:54:22 UTC
I'll take this one. This is going to be interesting :-)
Comment 2 Chris Sherlock 2016-02-09 12:53:36 UTC
Ok, I've reviewed OutputDevice and I can see what Michael is referring to. Step 1 shouldn't be too hard. I'll try to start this tomorrow :-) the easy part!
Comment 3 Robinson Tryon (qubit) 2016-02-18 14:51:28 UTC Comment hidden (obsolete)
Comment 4 Commit Notification 2016-05-16 15:03:42 UTC
melikeyurtoglu committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=51fe4d63dfdf0ea24d2fecf75d25cbe607ed1c09

tdf#97527 vcl: reference-count Menu

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 5 jani 2016-06-14 09:55:37 UTC
Seems solved
Comment 6 Michael Meeks 2016-06-15 08:29:52 UTC
The base-class piece of this was done; which is great.

However - a very large amount of work remains to convert all Menu * pointers to VclPtr<Menu> and for sub-classes; also to add suitable ::Create calls instead of 'new' calls, and so on.

Of course - all of that work needs to be done as a single larger change - I wonder if Noel has some thoughts on how to automate that and/or do it on a branch somewhere =)

Quite possibly it is outside the scope of an easy-hack, and we should re-close this.
Comment 7 jani 2016-06-16 06:09:26 UTC
Changing status to ASSIGNED.

I agree that the scope of the remaining work is more than a easyHack, especially due to the need of doing it in one step.

Removing keywords:
difficultyInteresting, easyHack, skillCpp, topicCleanup
Comment 8 Noel Grandin 2016-06-19 16:13:13 UTC
Looks like there are not that many Menu* fields and variables lying around (ignoring the parameters)

I'll get to this at some point. If anyone else wants to take over, though, they are welcome to.
Comment 9 Commit Notification 2016-06-23 06:28:32 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

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

tdf#97527 - vcl: reference-count Menu

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2016-06-27 16:35:59 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

fix CSV import dialog crash, tdf#97527 follow-up

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Xisco Faulí 2016-09-15 22:42:38 UTC
Hello,
Is this bug fixed?
If so, could you please close it as RESOLVED FIXED?