Bug 96888 - Kill internal vcl dog-tags ...
Summary: Kill internal vcl dog-tags ...
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: Dipankar Niranjan
URL:
Whiteboard: target:5.2.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2016-01-04 15:51 UTC by Michael Meeks
Modified: 2018-12-11 18:19 UTC (History)
2 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-01-04 15:51:46 UTC
We want to remove this structure:

vcl/inc/svdata.hxx

// ImplDelData is used as a "dog tag" by a window when it
// does something that could indirectly destroy the window
// TODO: wild destruction of a window should not be possible
struct ImplDelData


But first we should remove all of its users. Please do this one side by one - and test them individually. The ideal is that we can push a number of these patches separately over time - so that we can bibisect back to an individual patch easily later =)

There is no longer a need for this structure; checkout:

vcl/README.lifecycle.

So instead of its usage we should have:

	ImplDelData aDogTag( this );	// 'orrible old code
	Show( true, ShowFlags::NoActivate );
	if( !aDogTag.IsDead() )         // did 'this' go invalid yet ?
		Update();

should be:

        VclPtr xWindow(this);
        Show( true, ShowFlags::NoActivate );
	if( !aDogTag.IsDisposed() )
		Update();

Of course 'this' continues to be valid in this case.

There is a small functional change here; if you read:

void Window::dispose()
...
    // notify ImplDelData subscribers of this window about the window deletion

... there is some logic here ...

    delete mpWindowImpl; mpWindowImpl = nullptr;
}

ie. you will see that the-dog-tags are cleared a few lines before mpWindowImpl is set to NULL (cf.):

bool Window::IsDisposed() const
{
    return !mpWindowImpl;
}

So it is well worth testing each case that is changed; but I think the risk is reasonably low; and in the few cases that are like this we can use 'isDisposed' instead - which is set very much earlier =)

Thanks !
Comment 1 Dipankar Niranjan 2016-01-05 19:21:53 UTC
Hi,
This is a method where deletion listener was being used.
It has been replaced by a VclPtr reference.
Please verify if I'm on the right track.

http://pastebin.com/aea0PSVf

Build and unit-tests are fine with the above code.

But build gives a unit-test failed if I use 
xWindow.disposeAndClear()
instead of
xWindow.clear()
xWindow.reset()
I might need some clarification on that.

Also vcl/README.lifecycle was a real help.
And do you have ScopedVclPtr in place of VclPtr in mind for the future?
I could also notice a lot of things left in the  "What remains to be done ?" section of the doc.

Thanks!
Comment 2 Michael Meeks 2016-01-05 19:59:09 UTC
Hi there,

> This is a method where deletion listener was being used.
> It has been replaced by a VclPtr reference.
> Please verify if I'm on the right track.
> http://pastebin.com/aea0PSVf

Looks lovely =) Then again if we did xWindow.clear() there is no need for the reset as well.

> Build and unit-tests are fine with the above code.

Did you try 'make check' ? well worth running that at the top level before and after.

> But build gives a unit-test failed if I use 
> xWindow.disposeAndClear()

Sure - that forcibly destroys the window - we don't want to do that ;-)

> Also vcl/README.lifecycle was a real help.

Glad you like it; that's why I wrote it =)

> And do you have ScopedVclPtr in place of VclPtr in mind for the future?

Well; we have such a thing, but the templates generally could use a fair bit of cleanup there.

> I could also notice a lot of things left in the  "What remains to be done ?" 
> section of the doc.

Sure - this is one of them; great to have you on it - I suspect there is scope for a lot of nice, small patches in gerrit - separated by other commits - so we can bisect to them nicely =)

Thanks !


Hi there,



+        xWindow.clear();
+   xWindow.reset();
Comment 3 Dipankar Niranjan 2016-01-06 18:35:52 UTC
Hi,
I've submitted the first patch with four files modified.
https://gerrit.libreoffice.org/#/c/21170/
Build, unit tests and make check were fine.
Please review when free.

But in  /core/vcl/source/window/winproc.cxx
In line 2001 before commit:

if ( pSVEvent->mbCall && !pSVEvent->maDelData.IsDead() )

I do not know how to remove usage of pSVEvent->maDelData.IsDead()

pSVEvent->mpWindow and pSVEvent->mpInstanceRef both fail to build.

This is because ImplHandleUserEvent() is a static method.
It is called in line 2574 in winproc.cxx

Please help with this.
I've tried to analyze this part but I've drawn blanks so far..
Thanks!
Comment 4 Dipankar Niranjan 2016-01-06 21:31:05 UTC
The second patch has been uploaded with eight files modified.
https://gerrit.libreoffice.org/#/c/21174/
Build, unit tests and make check were fine.
Please review when free.

You might want to look at line 413 of window2.cxx and tell me what to do as it involves inheritance if I'm not wrong.
Also I've not touched svapp.cxx as it involves a constructor, a method and a destructor of ImplDelData.
I would need help with these two files.
Thanks!
Comment 5 Michael Meeks 2016-01-06 22:14:48 UTC
Hi there - I did some review on your first patch; looks nice - can you do some tweaks & re-submit it ? =) I'll review your 2nd patch as/when that's merged - but please feel free to adapt it based on the 1st one.

It is really good to split the work into lots of small commits that are merged over time - -particularly- for this work: where it is quite possible that we will get bisected issues later - and being able to isolate just a single patch is extremely helpful.

Anyhow - thanks again for the great progress here.
Comment 6 Dipankar Niranjan 2016-01-07 20:51:10 UTC
Hi,
Thanks for the really helpful Code Review..
I hope I've learned from it while I was resubmitting the patches.
So here they are:
dndevis.cxx
https://gerrit.libreoffice.org/21211
dialog.cxx
https://gerrit.libreoffice.org/21212
https://gerrit.libreoffice.org/21215
event.cxx
https://gerrit.libreoffice.org/21216
https://gerrit.libreoffice.org/21219
https://gerrit.libreoffice.org/21220
winproc.cxx
https://gerrit.libreoffice.org/21221
https://gerrit.libreoffice.org/21223
https://gerrit.libreoffice.org/21224
https://gerrit.libreoffice.org/21226
https://gerrit.libreoffice.org/21227
https://gerrit.libreoffice.org/21228

In  /core/vcl/source/window/winproc.cxx
In line 2001 before commit:

if ( pSVEvent->mbCall && !pSVEvent->maDelData.IsDead() )

I do not know how to remove usage of pSVEvent->maDelData.IsDead()

pSVEvent->mpWindow and pSVEvent->mpInstanceRef both fail to build.

This is because ImplHandleUserEvent() is a static method.
It is called in line 2574 in winproc.cxx

I would need some help with the above like I had previously mentioned.

button.cxx
https://gerrit.libreoffice.org/21230
lstbox.cxx
https://gerrit.libreoffice.org/21231
dockwin.cxx
Noticed a couple of empty methods at line 716
https://gerrit.libreoffice.org/21232
menu.cxx
https://gerrit.libreoffice.org/21233
menufloatingwindow.cxx
https://gerrit.libreoffice.org/21234
syswin.cxx
https://gerrit.libreoffice.org/21235
toolbox.cxx
https://gerrit.libreoffice.org/21236
toolbox2.cxx
https://gerrit.libreoffice.org/21237

Provided these patches are correct, I would need some help with:
1)Line 413 of window2.cxx as it involves inheritance if I'm not wrong.
2)I've not touched svapp.cxx as it involves a constructor, a method and a destructor of ImplDelData.
Thanks!
Comment 7 Commit Notification 2016-01-08 09:54:44 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 8 Commit Notification 2016-01-08 09:56:39 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 9 Commit Notification 2016-01-08 10:00:48 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 10 Commit Notification 2016-01-08 10:05:01 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 11 Commit Notification 2016-01-08 10:08:51 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 12 Commit Notification 2016-01-08 10:10:47 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 13 Michael Meeks 2016-01-08 10:18:35 UTC
Hi there,

> In  /core/vcl/source/window/winproc.cxx
> In line 2001 before commit:
> if ( pSVEvent->mbCall && !pSVEvent->maDelData.IsDead() )
> I do not know how to remove usage of pSVEvent->maDelData.IsDead()

   Oh ? so - what was associated with the maDelData ? - it rather depends on that really =) so I think we need to look at all of the calls that did an ImplAddDel on that member - and make sure we're checking them.

> pSVEvent->mpWindow and pSVEvent->mpInstanceRef both fail to build.

Oh ? why ? surely pSVEvent->mpWindow->IsDisposed() would work nicely.

> This is because ImplHandleUserEvent() is a static method.
> It is called in line 2574 in winproc.cxx

Hmm ? the above question is unrelated to static-ness AFAICS.

> Provided these patches are correct, I would need some help with:
> 1)Line 413 of window2.cxx as it involves inheritance if I'm not wrong.

Wow - that is horrible ! =) wow ... why do we bother casting to sal_uIntPtr there. So - in this case we need to update the methods to instead of using this awful sal_uIntPtr - instead passing a VclPtr<> around for the mpFocusWindow. Then we can rip out the ImplFocusDelData struct: another nice cleanup ! =)

> 2)I've not touched svapp.cxx as it involves a constructor, a method and a
> destructor of ImplDelData.

When there are no other calls of ImplDelData - we will remove the class, and kill all these methods, along with the notification code in window =)

You're getting there ... nice work ! =)
Comment 14 Commit Notification 2016-01-08 13:50:44 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 15 Commit Notification 2016-01-08 13:50:47 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 16 Commit Notification 2016-01-08 14:26:04 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 17 Commit Notification 2016-01-08 16:13:07 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 18 Commit Notification 2016-01-08 16:15:04 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 19 Commit Notification 2016-01-08 16:15:08 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 20 Commit Notification 2016-01-08 21:26:04 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 22 Michael Meeks 2016-01-09 11:43:04 UTC
Hi Dipankar:

> https://gerrit.libreoffice.org/21280 is a cleanup of
> https://gerrit.libreoffice.org/21215

Ah - this is a bit of a pain to manage =) Luckily gerrit has a feature to make this very, very easy - if you use it.

If you ensure that your new commit has the same 'Change-Id:' entry in the commit message - then gerrit will just update the old patch, rather than creating a new gerrit entry.

That -really- helps - it keeps the old comments together with the new patch, and reduces the thrash of gerrit and so on.

Please can you ensure that new patches retain the old Change-IDs ? I appreciate it's somewhat annoying having all these small patches ;-) but good practice for dealing with eg. 'git rebase -i' which is a powerful tool for squashing changes into patches =)

Thanks !
Comment 23 Commit Notification 2016-01-09 12:15:35 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 24 Commit Notification 2016-01-09 12:17:31 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 25 Michael Meeks 2016-01-09 12:20:55 UTC
Hi Dipankar

I reviewed a number of those changes; really keeping the Change-Id: the same would save a lot of effort. Also - please do not refer to gerrit URLs in your commit messages. git commit messages are part of the permanent record of the code - once in they cannot be changed; but gerrit URLs are not really ;-) so ... better to add a comment into gerrit itself.

Thanks for the good work !
Comment 26 Commit Notification 2016-01-09 20:31:49 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ..

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 27 Commit Notification 2016-01-09 20:37:06 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ...

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 28 Michael Meeks 2016-01-09 20:41:50 UTC
Nice to see you pushing updated with the same Change-Id: =)
Can you abandon any patches that are not current ? we're really starting to get there ;-) I'm really looking forward to having these all killed.
Comment 29 Michael Meeks 2016-01-09 20:42:25 UTC
by "abandon" I mean click the 'abandon' button in the gerrit web-ui for any patches that are obsolete - so they don't show up there anymore ? =)
Comment 30 Dipankar Niranjan 2016-01-09 22:35:59 UTC
Hi,
I did not know that we could submit updates with the same Change-Id.
Made things a lot easier and cleaner..
Thanks for teaching that..!

Also I've abandoned the older commits.
So at present, the status of all inactive and old commits should be either Merged or Abandoned.
Thanks!
Comment 31 Commit Notification 2016-01-11 09:52:29 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ..

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 32 Commit Notification 2016-01-11 11:41:03 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 Kill internal vcl dog-tags ..

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 33 Dipankar Niranjan 2016-01-11 21:09:58 UTC
Hi,
> > In  /core/vcl/source/window/winproc.cxx
> > In line 2001 before commit:
> > if ( pSVEvent->mbCall && !pSVEvent->maDelData.IsDead() )
> > I do not know how to remove usage of pSVEvent->maDelData.IsDead()
> 
>    Oh ? so - what was associated with the maDelData ? - it rather depends on
> that really =) so I think we need to look at all of the calls that did an
> ImplAddDel on that member - and make sure we're checking them.
> 
> > pSVEvent->mpWindow and pSVEvent->mpInstanceRef both fail to build.
> 
> Oh ? why ? surely pSVEvent->mpWindow->IsDisposed() would work nicely.

pSVEvent->mpWindow->IsDisposed() doesn't build and the vcl_timer test fails.
We get a segmentation fault.
Running Valgrind gives:http://pastebin.com/HPqeGN0t
Also, I'm pretty sure we haven't called ImplAddDel on maDelData anywhere as can be seen from grok.
So how do I proceed with this?
I don't think we've initialized or allocated mem for mpWindowImpl which IsDisposed checks..

> > Provided these patches are correct, I would need some help with:
> > 1)Line 413 of window2.cxx as it involves inheritance if I'm not wrong.
> 
> Wow - that is horrible ! =) wow ... why do we bother casting to sal_uIntPtr
> there. So - in this case we need to update the methods to instead of using
> this awful sal_uIntPtr - instead passing a VclPtr<> around for the
> mpFocusWindow. Then we can rip out the ImplFocusDelData struct: another nice
> cleanup ! =)

This cleanup may take a little bit of time too.. 
By mpFocusWindow, I think you meant mpFocusWin
And should a new struct be created exclusively for mpFocusWin as we intend to remove ImplFocusDelData which presently contains mpFocusWin?
Else, how do you propose to use mpFocusWin?
Comment 34 Commit Notification 2016-01-19 18:38:07 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 - Kill internal vcl dog-tags ...

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 35 Michael Stahl (allotropia) 2016-01-27 22:04:11 UTC
i take it 04d4af8496c8fae5515c7f76e143310eb7098702 "ImplSVEvent::maDelData is unused now" means this is fixed, thanks Dipankar!
Comment 36 Michael Meeks 2016-01-27 23:41:10 UTC
Heh =) I think we need to completely remove the ImplDelData structure completely as well - there is a chunk more (fun) cleanup work to do here I think - ripping out that dead wood still.
Comment 37 Commit Notification 2016-01-29 16:11:51 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 - Kill internal vcl dog-tags ...

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 38 Commit Notification 2016-02-03 12:55:11 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 - Kill internal vcl dog-tags ...

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 39 Michael Meeks 2016-02-03 13:24:40 UTC
Great work - this is finally gone =)

Thanks Dipankar !
Comment 40 Dipankar Niranjan 2016-02-03 13:35:37 UTC
There are some remnants though..(Maybe two or three usages..)
ImplDelData is still used at those places..
But ImplAddDel and ImplRemoveDel along with IsDead are completely gone..
So, if the cleanup is finished I would like to say I learned a lot during this cleanup!
Thanks for all the help..
Comment 41 Michael Meeks 2016-02-03 13:43:16 UTC
Oh - true ! =) not quite done - would love another patch to finally kill those.

Thanks !
Comment 42 Dipankar Niranjan 2016-02-03 13:46:03 UTC
(In reply to Michael Meeks from comment #41)
Sure.. Will do it once Grok gets updated.
Comment 43 Commit Notification 2016-02-04 07:23:05 UTC
Ras-al-Ghul committed a patch related to this issue.
It has been pushed to "master":

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

tdf#96888 - Kill internal vcl dog-tags ...

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 44 Robinson Tryon (qubit) 2016-02-18 16:37:23 UTC Comment hidden (obsolete)