[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: SystemTray: Refactoring: UiTask and TasksPool have been removed
From: "Aaron J. Seigo" <aseigo () kde ! org>
Date: 2012-11-01 9:47:05
Message-ID: 20121101094705.6140.68382 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107160/#review21280
-----------------------------------------------------------
Manager does not need to know about Applet (see comment below about why it should be \
this way).
The only reason why TasksPool had an Applet* was so that UiTasks could use it in \
UiTask::widget(). instead, the plasmoid / Applet pointer need to be made available to \
the Task via WidgetItem. as i noted in my email:
"the applet pointer could be made available to the QML and then WidgetItem
could have an applet (or host?) property which it would use to derive its
widget()."
so in the C++ bit of the system tray, a pointer to the SystemTray::Applet can be \
imported into the QML runtime, like this:
QScriptValue applet = m_widget->scriptEngine()->newQObject(this); // where 'this' is \
the SystemTray::Applet m_widget->scriptEngine()->globalObject().setProperty("host", \
applet);
plasma/generic/applets/systemtray/core/manager.h
<http://git.reviewboard.kde.org/r/107160/#comment16588>
the manager is shared across all instances of the system tray plasmoid. (look for \
s_manager) this keeps memory usage and dbus activity down. so this approach can not \
work.
- Aaron J. Seigo
On Oct. 31, 2012, 7 p.m., Dmitry Ashkadov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107160/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2012, 7 p.m.)
>
>
> Review request for Plasma, Aaron J. Seigo and Marco Martin.
>
>
> Description
> -------
>
> Aaron has noticed in his mail to plasma-devel that the system tray requires some \
> refactoring. This is a first step of refactoring. If it is approved I'll continue \
> work on system tray.
>
> Diffs
> -----
>
> plasma/generic/applets/systemtray/CMakeLists.txt d3478a8
> plasma/generic/applets/systemtray/core/manager.h 3b6b6f8
> plasma/generic/applets/systemtray/core/manager.cpp c23225e
> plasma/generic/applets/systemtray/core/task.h 66bf6e1
> plasma/generic/applets/systemtray/core/task.cpp 8754785
> plasma/generic/applets/systemtray/package/contents/code/main.js 10518cd
> plasma/generic/applets/systemtray/package/contents/ui/IconsList.qml f251cc5
> plasma/generic/applets/systemtray/package/contents/ui/StatusNotifierItem.qml \
> 27d476a plasma/generic/applets/systemtray/package/contents/ui/main.qml f80abc7
> plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtraytask.h \
> 34aae74 plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtraytask.cpp \
> 6364272 plasma/generic/applets/systemtray/protocols/fdo/fdoprotocol.h b006967
> plasma/generic/applets/systemtray/protocols/fdo/fdoprotocol.cpp c1bb8b1
> plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.h c86a2d5
> plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp 4257202
> plasma/generic/applets/systemtray/protocols/fdo/fdotask.h 85a9ec5
> plasma/generic/applets/systemtray/protocols/fdo/fdotask.cpp d1a18df
> plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtask.h 1913986
> plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtask.cpp 761a62f
> plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtaskprotocol.cpp \
> 505e0c4 plasma/generic/applets/systemtray/ui/applet.h bab8d9c
> plasma/generic/applets/systemtray/ui/applet.cpp 41efb10
> plasma/generic/applets/systemtray/ui/plasmoid.h 01a7c5b
> plasma/generic/applets/systemtray/ui/plasmoid.cpp d3e1937
> plasma/generic/applets/systemtray/ui/taskspool.h 4b5bcd4
> plasma/generic/applets/systemtray/ui/taskspool.cpp df39e3d
> plasma/generic/applets/systemtray/ui/uitask.h 7b20bde
> plasma/generic/applets/systemtray/ui/uitask.cpp 2a15800
> plasma/generic/applets/systemtray/ui/widgetitem.h 40aa92d
> plasma/generic/applets/systemtray/ui/widgetitem.cpp 9d2c622
>
> Diff: http://git.reviewboard.kde.org/r/107160/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dmitry Ashkadov
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/107160/">http://git.reviewboard.kde.org/r/107160/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Manager does not need to \
know about Applet (see comment below about why it should be this way).
The only reason why TasksPool had an Applet* was so that UiTasks could use it in \
UiTask::widget(). instead, the plasmoid / Applet pointer need to be made available to \
the Task via WidgetItem. as i noted in my email:
"the applet pointer could be made available to the QML and then WidgetItem
could have an applet (or host?) property which it would use to derive its
widget()."
so in the C++ bit of the system tray, a pointer to the SystemTray::Applet can be \
imported into the QML runtime, like this:
QScriptValue applet = m_widget->scriptEngine()->newQObject(this); // where \
'this' is the SystemTray::Applet \
m_widget->scriptEngine()->globalObject().setProperty("host", applet); \
</pre> <br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/107160/diff/1/?file=93099#file93099line54" \
style="color: black; font-weight: bold; text-decoration: \
underline;">plasma/generic/applets/systemtray/core/manager.h</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class \
Job;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">54</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">Manager</span><span class="p">();</span></pre></td> <th bgcolor="#e9eaa8" \
style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">54</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">Manager</span><span class="p">(</span><span class="n">Plasma</span><span \
class="o">::</span><span class="n">Applet</span> <span class="o">*</span><span \
class="n">applet</span><span class="p">);</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">the manager \
is shared across all instances of the system tray plasmoid. (look for s_manager) this \
keeps memory usage and dbus activity down. so this approach can not work.</pre> \
</div> <br />
<p>- Aaron J.</p>
<br />
<p>On October 31st, 2012, 7 p.m., Dmitry Ashkadov wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Plasma, Aaron J. Seigo and Marco Martin.</div>
<div>By Dmitry Ashkadov.</div>
<p style="color: grey;"><i>Updated Oct. 31, 2012, 7 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Aaron has noticed in his mail to plasma-devel that the system tray \
requires some refactoring. This is a first step of refactoring. If it is approved \
I'll continue work on system tray.</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plasma/generic/applets/systemtray/CMakeLists.txt <span style="color: \
grey">(d3478a8)</span></li>
<li>plasma/generic/applets/systemtray/core/manager.h <span style="color: \
grey">(3b6b6f8)</span></li>
<li>plasma/generic/applets/systemtray/core/manager.cpp <span style="color: \
grey">(c23225e)</span></li>
<li>plasma/generic/applets/systemtray/core/task.h <span style="color: \
grey">(66bf6e1)</span></li>
<li>plasma/generic/applets/systemtray/core/task.cpp <span style="color: \
grey">(8754785)</span></li>
<li>plasma/generic/applets/systemtray/package/contents/code/main.js <span \
style="color: grey">(10518cd)</span></li>
<li>plasma/generic/applets/systemtray/package/contents/ui/IconsList.qml <span \
style="color: grey">(f251cc5)</span></li>
<li>plasma/generic/applets/systemtray/package/contents/ui/StatusNotifierItem.qml \
<span style="color: grey">(27d476a)</span></li>
<li>plasma/generic/applets/systemtray/package/contents/ui/main.qml <span \
style="color: grey">(f80abc7)</span></li>
<li>plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtraytask.h \
<span style="color: grey">(34aae74)</span></li>
<li>plasma/generic/applets/systemtray/protocols/dbussystemtray/dbussystemtraytask.cpp \
<span style="color: grey">(6364272)</span></li>
<li>plasma/generic/applets/systemtray/protocols/fdo/fdoprotocol.h <span \
style="color: grey">(b006967)</span></li>
<li>plasma/generic/applets/systemtray/protocols/fdo/fdoprotocol.cpp <span \
style="color: grey">(c1bb8b1)</span></li>
<li>plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.h <span \
style="color: grey">(c86a2d5)</span></li>
<li>plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp <span \
style="color: grey">(4257202)</span></li>
<li>plasma/generic/applets/systemtray/protocols/fdo/fdotask.h <span style="color: \
grey">(85a9ec5)</span></li>
<li>plasma/generic/applets/systemtray/protocols/fdo/fdotask.cpp <span style="color: \
grey">(d1a18df)</span></li>
<li>plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtask.h <span \
style="color: grey">(1913986)</span></li>
<li>plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtask.cpp <span \
style="color: grey">(761a62f)</span></li>
<li>plasma/generic/applets/systemtray/protocols/plasmoid/plasmoidtaskprotocol.cpp \
<span style="color: grey">(505e0c4)</span></li>
<li>plasma/generic/applets/systemtray/ui/applet.h <span style="color: \
grey">(bab8d9c)</span></li>
<li>plasma/generic/applets/systemtray/ui/applet.cpp <span style="color: \
grey">(41efb10)</span></li>
<li>plasma/generic/applets/systemtray/ui/plasmoid.h <span style="color: \
grey">(01a7c5b)</span></li>
<li>plasma/generic/applets/systemtray/ui/plasmoid.cpp <span style="color: \
grey">(d3e1937)</span></li>
<li>plasma/generic/applets/systemtray/ui/taskspool.h <span style="color: \
grey">(4b5bcd4)</span></li>
<li>plasma/generic/applets/systemtray/ui/taskspool.cpp <span style="color: \
grey">(df39e3d)</span></li>
<li>plasma/generic/applets/systemtray/ui/uitask.h <span style="color: \
grey">(7b20bde)</span></li>
<li>plasma/generic/applets/systemtray/ui/uitask.cpp <span style="color: \
grey">(2a15800)</span></li>
<li>plasma/generic/applets/systemtray/ui/widgetitem.h <span style="color: \
grey">(40aa92d)</span></li>
<li>plasma/generic/applets/systemtray/ui/widgetitem.cpp <span style="color: \
grey">(9d2c622)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107160/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic