[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Provide (mostly) automatic synchronization between
From:       "Thomas Friedrichsmeier" <thomas.friedrichsmeier () ruhr-uni-bochum ! de>
Date:       2009-08-14 12:11:06
Message-ID: 20090814121106.10278.30589 () localhost
[Download RAW message or body]



> On 2009-08-14 08:04:33, Nick Shaforostoff wrote:
> > in case of several instances inside same process: am i right in that \
> > all the data is duplicated between all the instances? (if yes, why not \
> > store the data only once...)

Yes, but "all the data" in this case is one entry in the \
QString->KActionCollection* hash and one connected slot per instance (lines \
72 and 73 of the patch). So that is little more than simply keeping track \
of all similar instances.

It is true, that each instance of the same KXMLGUIClient keeps a separate \
copy of the XML document. It might be possible to improve this, but such \
optimization should likely happen in KXMLGUIClient itself (and may not be \
quite trivial). This patch does not change anything in that respect.

One potential optimization in this patch would be to tell only one client \
to reload the XML, then pass the domDocument() using setDOMDocument(). In \
the case of many instances of the same KXMLGUIClient, this could save quite \
a few cylces looking up, parsing, and merging the XML files, though I'm not \
sure this would really matter in practice (keep in mind that these reloads \
happen after user-interaction, only). This would have the side effect of \
saving some memory via implicit sharing of the QDomDocument (*after* \
reloading, only). But again, if we are to do this sort of optimization, I \
believe it should go into KXMLGUIClient, directly.

Yet another potential optimization would be to limit rebuildGUIs() to \
really refresh the action properties and toolbars, only, not the complete \
gui. If anybody knows, just how to do that, that would probably be a good \
idea (and such functionality should be added to KXMLGUIFactory).


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1238/#review2023
-----------------------------------------------------------


On 2009-08-11 12:19:00, Thomas Friedrichsmeier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1238/
> -----------------------------------------------------------
> 
> (Updated 2009-08-11 12:19:00)
> 
> 
> Review request for Kate and kdelibs.
> 
> 
> Summary
> -------
> 
> First to sum up the problem, I'm trying to solve. I'll use konqueror as \
> an example: 1) Fire up two instances of konqueror. In one of the \
> instances, set the shortcut of "Show History" to "Shift+F6". Now switch \
> to the other instance: The change has not been propagated. Ok, perhaps \
> you did not expect that to work in the first place, but now: 2) Fire up a \
> single instance of konqueror, the open a new window (File->New Window). \
> Do the same game as in 1. The change is not propagated between the two \
> toplevel windows of the same konqueror instance, either. Finally: 3) Fire \
> up a single instance of konqueror, with a single toplevel window. Open a \
> second tab inside this window. Navigate both tabs to e.g. \
> http://www.kde.org. Switch to tab A. Change the shortcut of "View \
> Document Source" to "Shift+F6". Take a look at the "View"-menu to see the \
> new shortcut. Now switch to tab B. Take another look at the "View"-menu. \
> The shortcut is still at the previous setting, here. Switch back to tab \
> A. The shortcut is back to the previous setting as well. 4) If you like \
> to, play games 1-3 with modifying the toolbars, instead of shortcuts. 
> So much for the bug description. Konqueror is not really to blame, here, \
> and neither is it the only application to be affected. The problem is \
> that we're missing a mechanism to synchronize user settings between \
> separate instances of KXMLGUIClients. That's what the new class \
> KXmlGuiClientSyncer - contained in this patch - does. See the \
> API-documentation within for usage details. For the "essence" of the \
> code, see KXmlGuiClientSyncerPrivate::uiRcFileChanged(). 
> Some points, I wasn't sure about:
> 1) Originally, I tried to incorporate this into KXMLGUIClient, directly \
> (or rather KXMLGUIClientPrivate). That would have allowed for *fully* \
> automatic synchronization, i.e. without having to register/watch the \
> clients manually. However, kio depends on kdeui, and this would have \
> added a cyclic dependency back to kio. Any idea on how to work around \
> this? Or this semi-automatic solution safer in the first place? 2) I \
> placed this inside kio, pretty much for the reason above. Is this \
> appropriate? 3) Which kDebug area to use?
> 
> 
> Diffs
> -----
> 
> trunk/KDE/kdelibs/includes/CMakeLists.txt 1006783 
> trunk/KDE/kdelibs/includes/KXmlGuiClientSyncer PRE-CREATION 
> trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiclient.h 1009993 
> trunk/KDE/kdelibs/kio/CMakeLists.txt 1006783 
> trunk/KDE/kdelibs/kio/kio/kxmlguiclientsyncer.h PRE-CREATION 
> trunk/KDE/kdelibs/kio/kio/kxmlguiclientsyncer.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kio/kio/kxmlguiclientsyncer_p.h PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1238/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Thomas
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]