Merge lp:~bilalakhtar/unity/sc-integration-phase2 into lp:unity
- sc-integration-phase2
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 2183 |
Proposed branch: | lp:~bilalakhtar/unity/sc-integration-phase2 |
Merge into: | lp:unity |
Diff against target: |
458 lines (+213/-17) 9 files modified
plugins/unityshell/src/BamfLauncherIcon.h (+1/-1) plugins/unityshell/src/Launcher.cpp (+7/-1) plugins/unityshell/src/Launcher.h (+7/-3) plugins/unityshell/src/LauncherController.cpp (+24/-8) plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp (+101/-2) plugins/unityshell/src/SoftwareCenterLauncherIcon.h (+20/-0) tests/autopilot/autopilot/emulators/unity/icons.py (+3/-0) tests/autopilot/autopilot/emulators/unity/launcher.py (+14/-1) tests/autopilot/autopilot/tests/test_launcher.py (+36/-1) |
To merge this branch: | bzr merge lp:~bilalakhtar/unity/sc-integration-phase2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alex Launi (community) | Approve | ||
Thomi Richards (community) | Needs Fixing | ||
Marco Trevisan (Treviño) | Needs Fixing | ||
Matthew Paul Thomas (community) | design | Approve | |
Unity Team | Pending | ||
Review via email: mp+95795@code.launchpad.net |
This proposal has been superseded by a proposal from 2012-03-16.
Commit message
Description of the change
This is the second phase of Software Center integration with the Unity launcher. The first phase included progress bars on the icon, as well as a "Waiting to install" tooltip. The second phase includes an icon moving animation from Software Center to the launcher, and also icon Activate() changes. The software center design spec for the whole feature is:
https:/
The first phase landed around Unity 5.2.0, but got broken right before the release of 5.2.0 (bug #932280). The fix to it lies in this branch: https:/
Note that this branch contains the code from the above merge request too, so if someone were to merge this branch into lp:unity, there would be no need to merge lp:~bilalakhtar/unity/fix-sc-launcher-integration. The reason why I've made two different branches is because I'd like to give the lp:~bilalakhtar/unity/fix-sc-launcher-integration a higher priority (5.6.0) while this could get in to either 5.6.0 (unlikely) or 5.8.0.
The implementation is very stable, with unit tests and all.
Here's a YouTube video of this branch in action:
http://
Bilal Akhtar (bilalakhtar) wrote : | # |
Didier Roche-Tolomelli (didrocks) wrote : | # |
This branche introduces a change in behavior (flying icon). It clearly needs an UIFe and a FFe.
Also, we saw that the previous code was broken without anyone noticing. This shows that you need tests for you code. Can you please first ensure that lp:~bilalakhtar/unity/fix-sc-launcher-integration gets autopilot tests for it not regressing again?
Matthew Paul Thomas (mpt) wrote : | # |
Nice work! There are many details to tweak: the origin point is too high, the destination point looks to be on top of another launcher element rather than in a gap, the initial icon size is too small, and (as "krnekhelesh" pointed out on YouTube) the animation should happen after authentication rather than before. But this is a definite improvement, thank you.
Bilal Akhtar (bilalakhtar) wrote : | # |
@didrocks: When is Unity 5.6.0 coming out? Tomorrow, right?
I won't be able to add autopilot tests until today evening and that would mean a merge of lp:~bilalakhtar/unity/fix-sc-launcher-integration AFTER 5.6.0 (since by then everyone would have gone off to sleep). If that's the case, then can you merge that branch right now and I'll add tests today evening?
But if I'm wrong about the release date of 5.6.0 then don't merge that branch until I've added tests.
@mpt: Thanks for the review! Since you've approved already I'll ensure this gets in before I work on those tweaks.
Didier Roche-Tolomelli (didrocks) wrote : | # |
Hey Bilal, there are two things:
5.6 is coming out tomorrow morning if the last regression is fixed. We don't merge anything for a week already as we are in freeze. So this would mean your branch will be merged for 5.8 (once you unfreeze).
Oh, and we don't let entering trunk code without test, so yeah, it's a pre-requisite to push to the other branch some tests.
For that one, you will need to have a Feature freeze exception acked and a UI Freeze exception acked as well before meriging in trunk (in addition to test and from a review from some dx guys, I'm not doing the review. ;)).
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
64 + sigc::signal<void, std::string const&, AbstractLaunche
65 + gint32, gint32, gint32> launcher_
You can just use int at this level, so we are more C++ conformant.
139 + ((SoftwareCente
Don't use C-style casts.
184 + finished = true;
185 + finished_just_now = true;
Set them using the constructor initializer list.
201 +void SoftwareCenterL
202 +{
203 + _launcher-
204 +}
306 + void AddSelfToLaunch
322 + AbstractLaunche
I don't really think you need those, please don't do that. You already handling an AbstractLaunche
244 + target_x = (int)current_
Again, use static_cast
256 + g_timeout_add(30, &SoftwareCenter
Maybe it can be a little more lazy.
Anyway I'd focus on fixing the problem at the source, instead of using this workaround.
308 + static gboolean OnDragWindowAni
Put in private fields.
310 + void Animate(
You can also just use Animate(
312 + void ActivateLaunche
Put it in protected field.
323 + bool finished;
324 + bool finished_just_now;
Add the _ as prefix for private variables. (Actually it would be betteer use a suffixed _, as defined by our coding standards, but in that case you need to update all the members).
For testing, I think you can also emulate something without autopilot by making your LauncherSpecial request to be caused by a fake dbus server you write on tests.
Bilal Akhtar (bilalakhtar) wrote : | # |
I've made all the changes that were possible as per Marco's review, except for the tests (which I'm adding now) and I also left some other ones which would break the code if I did change (I've left code comments at those spots).
Gary Lasker (gary-lasker) wrote : | # |
Hi Bilal! I just want to say thank you for this. It's beautiful!! :D
I am very much looking forward to seeing this in Precise!!!
Thanks everybody,
Gary
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
189 + _finished = true;
190 + _finished_just_now = true;
You don't need them anymore.
329 + AbstractLaunche
Are you sure you need to save that on the class? I guess you can just Reference your icon before that it gets unreferenced...
+ if (((int)
248 + ((int)current_
Those are still C-style casts :)
139 + static_
Do this only if you're sure that this won't happen for another kind of icon... Otherwise use, at least, the dynamic_cast.
Bilal Akhtar (bilalakhtar) wrote : | # |
Branch is ready again, please review :)
- 2013. By Bilal Akhtar
-
Change C-style casts
Bilal Akhtar (bilalakhtar) wrote : | # |
> 189 + _finished = true;
> 190 + _finished_just_now = true;
>
> You don't need them anymore.
Those lines are part of the Finished function, so they're very much needed for the Activate function to work.
>
> 329 + AbstractLaunche
>
> Are you sure you need to save that on the class? I guess you can just
> Reference your icon before that it gets unreferenced...
>
That's the problem. I did try that, but it didn't work. Got a segfault (totally strange). Storing it's own reference in a variable worked, though.
> + if (((int)
> 248 + ((int)current_
> 0)
>
> Those are still C-style casts :)
Fixed in latest commit.
>
> 139 + static_
> ate(launcher_, icon_x, icon_y, icon_size);
>
> Do this only if you're sure that this won't happen for another kind of icon...
> Otherwise use, at least, the dynamic_cast.
Yes, it won't happen, because I'm using a special function to create the result variable. That function is hard-coded to create only a SoftwareCenterL
Thanks for the review anyway, it would be great if you reviewed again!
- 2014. By Bilal Akhtar
-
Fix a line in tests
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
> Those lines are part of the Finished function, so they're very much needed for
> the Activate function to work.
Oh, yes sure. I didn't notice they were inside the lambda, sorry.
> > 329 + AbstractLaunche
> >
> > Are you sure you need to save that on the class? I guess you can just
> > Reference your icon before that it gets unreferenced...
> >
>
> That's the problem. I did try that, but it didn't work. Got a segfault
> (totally strange). Storing it's own reference in a variable worked, though.
I want to look to this further and do some experiments with it... I think we should find a cleaner solution.
357 +class SoftwareCenterL
358 + """Represents a launcher icon of a Software Center app."""
Also on tests SoftwareCenterL
This would also avoid the change on line 378
About the autoplito tests, I think that maybe you should define functions to add icons to the launcher in the emulator, also once you've finished them ask a review by Thomi for the tests.
Bilal Akhtar (bilalakhtar) wrote : | # |
I myself have given up on the AbstractLaunche
Other than that, do you think the code (other than the tests) is good? Could you approve the code then? Of course I'd need to file a UIFe and FFe request before it can get merged, but in the meantime you can approve/reject it.
Thomi Richards (thomir-deactivatedaccount) wrote : | # |
Hi,
There's a problem with the autopilot test (Exception on cleanup), which I will look at and fix on Monday. However, more concerning is that running the autopilot test causes unity / compiz to crash frequently (maybe 50% of the time). I will attempt to look into this as well, and let you know what the issue is.
@Marco - you mentioned above that it might be a good idea to add methods to the Launcher emulator for adding and removing icons - we already have this, but we can't use it in this instance, since this isn't a regular application icon.
Thomi Richards (thomir-deactivatedaccount) wrote : | # |
Hi,
Upon further investigation, the crash is inside the software center launcher icon. I get a segfault about 50% of the time when running the autopilot test. Here's the stack trace:
#0 0x00007fffe4f02ca2 in unity::
at /home/thomi/
#1 0x00007fffe509ecd7 in unity::
icon_x=100, icon_y=100, icon_size=32) at /home/thomi/
#2 0x00007fffe50aa989 in sigc::bound_
_A_
#3 0x00007fffe50a9d5f in sigc::adaptor_
_A_
#4 0x00007fffe50a89b2 in sigc::internal:
a_7=
#5 0x00007fffe508b1e9 in sigc::internal:
_A_
#6 0x00007fffe508995d in sigc::signal7<void, std::string const&, nux::ObjectPtr<
at /usr/include/
- 2015. By Bilal Akhtar
-
Fix crash on second install, THANKS THOMI
Bilal Akhtar (bilalakhtar) wrote : | # |
Thomi:
I've fixed the crash, pull the branch and you won't have the problem again :)
Bilal Akhtar (bilalakhtar) wrote : | # |
Also, Marco: Can you review the code? Everything's final now, Thomi will be reviewing the tests. The code is final and works perfectly now.
- 2016. By Bilal Akhtar
-
Merge from trunk, fix conflicts
Alex Launi (alexlauni) wrote : | # |
I don't like how the test skips if the software center is already pinned. You should have your setup ensure that the launcher is not pinned, so that the test can execute.
- 2017. By Bilal Akhtar
-
Cleanup tests, also first ensure Software Center is removed
Bilal Akhtar (bilalakhtar) wrote : | # |
Thanks for the review Alex, now it unlocks the icon before proceeding. That fixes the issue. Can you review the tests again now?
Didier Roche-Tolomelli (didrocks) wrote : | # |
Note that it will need a Feature Freeze and UI Freeze exception acked. Can you please proceed it?
Bilal Akhtar (bilalakhtar) wrote : | # |
I thought that was after the code review? Anyway, I'll do that now.
Didier Roche-Tolomelli (didrocks) wrote : | # |
that's to be done in parallel in fact as you need to get the release team approval before the code is merged.
Bilal Akhtar (bilalakhtar) wrote : | # |
I've filed bug #955147 , and subscribed ubuntu-release to it. That bug contains everything needed for the exception requests. I've also posted to the docs and translators mailing lists and linked the postings there.
Alex Launi (alexlauni) wrote : | # |
The dbus code should be moved into the launcher emulator. Secondly, the assertion that there are more icons may not be true. Consider the case where some applications crash unexpectedly. I would just check for the waiting to install tooltip. That's the real sign that everything worked.
I don't think this test needs to go in its own class. It will be just fine in LauncherTests. I also feel like this test might need some clean up routine to return the desktop to its original state. Does the AddLauncherItem
- 2018. By Bilal Akhtar
-
Make changes in tests, merge function into LauncherTests, etc
- 2019. By Bilal Akhtar
-
Cleanup at the end, move function to the proper class
Alex Launi (alexlauni) wrote : | # |
emulate_
instead of importing dbus and doing bus.SessionBus just do:
from autopilot.
session_bus is your bus object.
define the desktop file in a variable instead of reusing the string over and over.
sleep should not be the first thing you do in your test, also not the last!
You need to check that the icon isn't None when you get it after the dbus call. If the call failed, then the test will crash. So assert that it is not None
self.assertThat
Instead of asserting that icon_text == "blah", Equals(True) do this
self.assertThat
cleanup should go like this,
add an anonymous function to your test
def cleanup():
if icon is not None:
then
icon = self.launcher.
if icon is not None:
launcher_
sleep(1)
else:
self.
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Also, make sure to cover in your test this change (it should be implicit, I guess): https:/
Bilal Akhtar (bilalakhtar) wrote : | # |
Hi Marco,
That change is already covered. In fact, the whole test function evolves
around that.
There would be no way to make it implicit without going through the whole
procedure,since all functions are interconnected.
I'll make the rest if the changes soon.
-------
*From:* "Marco Trevisan (Treviño)" <mail@3v1n0.net>
*To:* "Bilal Akhtar" <email address hidden>
*Sent:* 14 March, 2012 1:44 PM
*Subject:* Re: [Merge] lp:~bilalakhtar/unity/sc-integration-phase2 into
lp:unity
Also, make sure to cover in your test this change (it should be implicit, I
guess):
https:/
we can merge all together (if we get the exception, otherwise you need
to backport the tests to only that branch).
--
https:/
You are the owner of lp:~bilalakhtar/unity/sc-integration-phase2.
- 2020. By Bilal Akhtar
-
Cleanup tests even further, follow Alex's recommendations, thanks a lot Alex
- 2021. By Bilal Akhtar
-
Merge from trunk
Alex Launi (alexlauni) wrote : | # |
Tests now look good Bilal
Alex Launi (alexlauni) wrote : | # |
I missed this yesterday, please move def cleanup() to be the first thing in test_software_
- 2022. By Bilal Akhtar
-
Move definition of cleanup function - thanks Alex
- 2023. By Bilal Akhtar
-
Merge from trunk
Preview Diff
1 | === modified file 'plugins/unityshell/src/BamfLauncherIcon.h' |
2 | --- plugins/unityshell/src/BamfLauncherIcon.h 2012-02-12 10:43:11 +0000 |
3 | +++ plugins/unityshell/src/BamfLauncherIcon.h 2012-03-15 17:05:24 +0000 |
4 | @@ -43,7 +43,7 @@ |
5 | BamfLauncherIcon(BamfApplication* app); |
6 | virtual ~BamfLauncherIcon(); |
7 | |
8 | - void ActivateLauncherIcon(ActionArg arg); |
9 | + virtual void ActivateLauncherIcon(ActionArg arg); |
10 | |
11 | std::string DesktopFile(); |
12 | |
13 | |
14 | === modified file 'plugins/unityshell/src/Launcher.cpp' |
15 | --- plugins/unityshell/src/Launcher.cpp 2012-03-13 17:20:30 +0000 |
16 | +++ plugins/unityshell/src/Launcher.cpp 2012-03-15 17:05:24 +0000 |
17 | @@ -1720,6 +1720,11 @@ |
18 | Resize(); |
19 | } |
20 | |
21 | +int Launcher::GetIconSize() |
22 | +{ |
23 | + return _icon_size; |
24 | +} |
25 | + |
26 | void Launcher::Resize() |
27 | { |
28 | UScreen* uscreen = UScreen::GetDefault(); |
29 | @@ -2877,7 +2882,8 @@ |
30 | g_variant_get(parameters, "(ssiiiss)", &title, &icon, &icon_x, &icon_y, &icon_size, &desktop_file, &aptdaemon_task, NULL); |
31 | |
32 | Launcher* self = (Launcher*)user_data; |
33 | - self->launcher_addrequest_special.emit(desktop_file, AbstractLauncherIcon::Ptr(), aptdaemon_task, icon); |
34 | + self->launcher_addrequest_special.emit(desktop_file, AbstractLauncherIcon::Ptr(), aptdaemon_task, icon, |
35 | + icon_x, icon_y, icon_size); |
36 | |
37 | g_dbus_method_invocation_return_value(invocation, nullptr); |
38 | g_free(icon); |
39 | |
40 | === modified file 'plugins/unityshell/src/Launcher.h' |
41 | --- plugins/unityshell/src/Launcher.h 2012-03-13 02:07:54 +0000 |
42 | +++ plugins/unityshell/src/Launcher.h 2012-03-15 17:05:24 +0000 |
43 | @@ -40,6 +40,7 @@ |
44 | #include "LauncherHideMachine.h" |
45 | #include "LauncherHoverMachine.h" |
46 | #include "UBusWrapper.h" |
47 | +#include "SoftwareCenterLauncherIcon.h" |
48 | |
49 | |
50 | namespace unity |
51 | @@ -68,6 +69,7 @@ |
52 | AbstractLauncherIcon::Ptr GetSelectedMenuIcon() const; |
53 | |
54 | void SetIconSize(int tile_size, int icon_size); |
55 | + int GetIconSize(); |
56 | |
57 | LauncherHideMachine* HideMachine() { return _hide_machine; } |
58 | |
59 | @@ -113,8 +115,10 @@ |
60 | void Resize(); |
61 | |
62 | sigc::signal<void, char*, AbstractLauncherIcon::Ptr> launcher_addrequest; |
63 | - sigc::signal<void, std::string const&, AbstractLauncherIcon::Ptr, std::string const&, std::string const&> launcher_addrequest_special; |
64 | + sigc::signal<void, std::string const&, AbstractLauncherIcon::Ptr, std::string const&, std::string const&, |
65 | + int, int, int> launcher_addrequest_special; |
66 | sigc::signal<void, AbstractLauncherIcon::Ptr> launcher_removerequest; |
67 | + sigc::signal<void, AbstractLauncherIcon::Ptr> icon_animation_complete; |
68 | sigc::signal<void> selection_change; |
69 | sigc::signal<void> hidden_changed; |
70 | |
71 | @@ -128,6 +132,8 @@ |
72 | |
73 | static const int ANIM_DURATION_SHORT; |
74 | |
75 | + void RenderIconToTexture(nux::GraphicsEngine& GfxContext, AbstractLauncherIcon::Ptr icon, nux::ObjectPtr<nux::IOpenGLBaseTexture> texture); |
76 | + |
77 | protected: |
78 | // Introspectable methods |
79 | std::string GetName() const; |
80 | @@ -278,8 +284,6 @@ |
81 | |
82 | void OnActionDone(GVariant* data); |
83 | |
84 | - void RenderIconToTexture(nux::GraphicsEngine& GfxContext, AbstractLauncherIcon::Ptr icon, nux::ObjectPtr<nux::IOpenGLBaseTexture> texture); |
85 | - |
86 | AbstractLauncherIcon::Ptr MouseIconIntersection(int x, int y); |
87 | void EventLogic(); |
88 | void MouseDownLogic(int x, int y, unsigned long button_flags, unsigned long key_flags); |
89 | |
90 | === modified file 'plugins/unityshell/src/LauncherController.cpp' |
91 | --- plugins/unityshell/src/LauncherController.cpp 2012-03-14 23:22:02 +0000 |
92 | +++ plugins/unityshell/src/LauncherController.cpp 2012-03-15 17:05:24 +0000 |
93 | @@ -86,9 +86,12 @@ |
94 | void OnIconRemoved(AbstractLauncherIcon::Ptr icon); |
95 | |
96 | void OnLauncherAddRequest(char* path, AbstractLauncherIcon::Ptr before); |
97 | - void OnLauncherAddRequestSpecial(std::string const& path, AbstractLauncherIcon::Ptr before, std::string const& aptdaemon_trans_id, std::string const& icon_path); |
98 | + void OnLauncherAddRequestSpecial(std::string const& path, AbstractLauncherIcon::Ptr before, std::string const& aptdaemon_trans_id, std::string const& icon_path, |
99 | + int icon_x, int icon_y, int icon_size); |
100 | void OnLauncherRemoveRequest(AbstractLauncherIcon::Ptr icon); |
101 | |
102 | + void OnSCIconAnimationComplete(AbstractLauncherIcon::Ptr icon); |
103 | + |
104 | void OnLauncherEntryRemoteAdded(LauncherEntryRemote* entry); |
105 | void OnLauncherEntryRemoteRemoved(LauncherEntryRemote* entry); |
106 | |
107 | @@ -340,6 +343,8 @@ |
108 | launcher->launcher_addrequest_special.connect(sigc::mem_fun(this, &Impl::OnLauncherAddRequestSpecial)); |
109 | launcher->launcher_removerequest.connect(sigc::mem_fun(this, &Impl::OnLauncherRemoveRequest)); |
110 | |
111 | + launcher->icon_animation_complete.connect(sigc::mem_fun(this, &Impl::OnSCIconAnimationComplete)); |
112 | + |
113 | parent_->AddChild(launcher); |
114 | |
115 | return launcher; |
116 | @@ -393,7 +398,10 @@ |
117 | Controller::Impl::OnLauncherAddRequestSpecial(std::string const& path, |
118 | AbstractLauncherIcon::Ptr before, |
119 | std::string const& aptdaemon_trans_id, |
120 | - std::string const& icon_path) |
121 | + std::string const& icon_path, |
122 | + int icon_x, |
123 | + int icon_y, |
124 | + int icon_size) |
125 | { |
126 | auto launchers = model_->GetSublist<BamfLauncherIcon>(); |
127 | for (auto icon : launchers) |
128 | @@ -403,14 +411,24 @@ |
129 | } |
130 | |
131 | AbstractLauncherIcon::Ptr result = CreateSCLauncherIcon(path, aptdaemon_trans_id, icon_path); |
132 | + |
133 | + launcher_->ForceReveal(true); |
134 | + |
135 | if (result) |
136 | + static_cast<SoftwareCenterLauncherIcon*>(result.GetPointer())->Animate(launcher_, icon_x, icon_y, icon_size); |
137 | +} |
138 | + |
139 | +void Controller::Impl::OnSCIconAnimationComplete(AbstractLauncherIcon::Ptr icon) |
140 | +{ |
141 | + |
142 | + if (icon) |
143 | { |
144 | - RegisterIcon(result); |
145 | - |
146 | - if (before) |
147 | - model_->ReorderBefore(result, before, false); |
148 | + RegisterIcon(icon); |
149 | } |
150 | Save(); |
151 | + |
152 | + launcher_->ForceReveal(false); |
153 | + |
154 | } |
155 | |
156 | void Controller::Impl::SortAndUpdate() |
157 | @@ -714,8 +732,6 @@ |
158 | return result; |
159 | } |
160 | |
161 | - g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"), GINT_TO_POINTER(1)); |
162 | - |
163 | bamf_view_set_sticky(BAMF_VIEW(app), true); |
164 | AbstractLauncherIcon::Ptr icon(new SoftwareCenterLauncherIcon(app, aptdaemon_trans_id, icon_path)); |
165 | icon->SetSortPriority(sort_priority_++); |
166 | |
167 | === modified file 'plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp' |
168 | --- plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp 2012-02-12 10:43:11 +0000 |
169 | +++ plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp 2012-03-15 17:05:24 +0000 |
170 | @@ -20,6 +20,8 @@ |
171 | |
172 | #include <glib/gi18n-lib.h> |
173 | #include "SoftwareCenterLauncherIcon.h" |
174 | +#include "Launcher.h" |
175 | +#include "LauncherModel.h" |
176 | |
177 | namespace unity |
178 | { |
179 | @@ -34,19 +36,108 @@ |
180 | aptdaemon_trans_id, |
181 | "org.debian.apt.transaction", |
182 | G_BUS_TYPE_SYSTEM, |
183 | - G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START) |
184 | + G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START), |
185 | + _finished (true), |
186 | + _finished_just_now (false) |
187 | { |
188 | + |
189 | _aptdaemon_trans.Connect("PropertyChanged", sigc::mem_fun(this, &SoftwareCenterLauncherIcon::OnPropertyChanged)); |
190 | _aptdaemon_trans.Connect("Finished", [&] (GVariant *) { |
191 | tooltip_text = BamfName(); |
192 | SetQuirk(QUIRK_PROGRESS, false); |
193 | + SetQuirk(QUIRK_URGENT, true); |
194 | SetProgress(0.0f); |
195 | + _finished = true; |
196 | + _finished_just_now = true; |
197 | }); |
198 | |
199 | SetIconType(TYPE_APPLICATION); |
200 | icon_name = icon_path.c_str(); |
201 | tooltip_text = _("Waiting to install"); |
202 | -} |
203 | + |
204 | + // For no clear reason, Unity segfaults if we try to generate a pointer of "this" twice. |
205 | + // Hence, just generate it once and store it. |
206 | + self_abstract = AbstractLauncherIcon::Ptr(this); |
207 | + |
208 | +} |
209 | + |
210 | +void SoftwareCenterLauncherIcon::AddSelfToLauncher() |
211 | +{ |
212 | + _launcher->icon_animation_complete.emit(self_abstract); |
213 | +} |
214 | + |
215 | +gboolean SoftwareCenterLauncherIcon::OnDragWindowAnimComplete(gpointer data) |
216 | +{ |
217 | + SoftwareCenterLauncherIcon* self = (SoftwareCenterLauncherIcon*) data; |
218 | + if (!self->_drag_window->Animating()) { |
219 | + self->_drag_window->ShowWindow(false); |
220 | + |
221 | + // I had to create a new function because for some reason, when trying to access |
222 | + // self->_launcher from this function, I got segfaults. |
223 | + self->AddSelfToLauncher(); |
224 | + return false; |
225 | + } |
226 | + return true; |
227 | +} |
228 | + |
229 | +void |
230 | +SoftwareCenterLauncherIcon::Animate(nux::ObjectPtr<Launcher> launcher, |
231 | + int icon_x, |
232 | + int icon_y, |
233 | + int icon_size) |
234 | +{ |
235 | + int target_x = 0; |
236 | + int target_y = 0; |
237 | + |
238 | + _launcher = launcher.GetPointer(); |
239 | + |
240 | + _icon_texture = nux::GetGraphicsDisplay()->GetGpuDevice()->CreateSystemCapableDeviceTexture(launcher->GetIconSize(), launcher->GetIconSize(), 1, nux::BITFMT_R8G8B8A8); |
241 | + _drag_window = new LauncherDragWindow(_icon_texture); |
242 | + |
243 | + launcher->RenderIconToTexture(nux::GetWindowThread()->GetGraphicsEngine(), self_abstract, _icon_texture); |
244 | + nux::Geometry geo = _drag_window->GetGeometry(); |
245 | + _drag_window->SetBaseXY(icon_x, icon_y); |
246 | + _drag_window->ShowWindow(true); |
247 | + _drag_window->SinkReference(); |
248 | + |
249 | + // Find out the center of last BamfLauncherIcon with non-zero co-ordinates |
250 | + auto launchers = launcher->GetModel()->GetSublist<BamfLauncherIcon>(); |
251 | + for (auto current_bamf_icon : launchers) |
252 | + { |
253 | + if (static_cast<int>(current_bamf_icon->GetCenter(launcher->monitor).x) != 0 && |
254 | + static_cast<int>(current_bamf_icon->GetCenter(launcher->monitor).y) != 0) |
255 | + { |
256 | + target_x = static_cast<int>(current_bamf_icon->GetCenter(launcher->monitor).x); |
257 | + target_y = static_cast<int>(current_bamf_icon->GetCenter(launcher->monitor).y); |
258 | + } |
259 | + } |
260 | + |
261 | + target_y = target_y + (launcher->GetIconSize() / 2); |
262 | + _drag_window->SetAnimationTarget(target_x, target_y); |
263 | + |
264 | + _drag_window->StartAnimation(); |
265 | + |
266 | + // This is to check if the animation has completed, since the anim_completed signal |
267 | + // in the drag window doesn't work (mysteriously) |
268 | + g_timeout_add(30, &SoftwareCenterLauncherIcon::OnDragWindowAnimComplete, this); |
269 | + |
270 | +} |
271 | + |
272 | +void SoftwareCenterLauncherIcon::ActivateLauncherIcon(ActionArg arg) |
273 | +{ |
274 | + if (_finished) |
275 | + { |
276 | + if (_finished_just_now) |
277 | + { |
278 | + SetQuirk(QUIRK_URGENT, false); |
279 | + _finished_just_now = false; |
280 | + } |
281 | + BamfLauncherIcon::ActivateLauncherIcon(arg); |
282 | + } |
283 | + else |
284 | + SetQuirk(QUIRK_STARTING, false); |
285 | +} |
286 | + |
287 | |
288 | void |
289 | SoftwareCenterLauncherIcon::OnPropertyChanged(GVariant* params) |
290 | @@ -63,7 +154,10 @@ |
291 | g_variant_get(property_value, "i", &progress); |
292 | |
293 | if (progress < 100) |
294 | + { |
295 | SetQuirk(QUIRK_PROGRESS, true); |
296 | + _finished = false; |
297 | + } |
298 | |
299 | SetProgress(progress/100.0f); |
300 | } |
301 | @@ -71,5 +165,10 @@ |
302 | g_variant_unref(property_value); |
303 | } |
304 | |
305 | +std::string SoftwareCenterLauncherIcon::GetName() const |
306 | +{ |
307 | + return "SoftwareCenterLauncherIcon"; |
308 | +} |
309 | + |
310 | } |
311 | } |
312 | |
313 | === modified file 'plugins/unityshell/src/SoftwareCenterLauncherIcon.h' |
314 | --- plugins/unityshell/src/SoftwareCenterLauncherIcon.h 2012-02-03 01:24:53 +0000 |
315 | +++ plugins/unityshell/src/SoftwareCenterLauncherIcon.h 2012-03-15 17:05:24 +0000 |
316 | @@ -22,6 +22,8 @@ |
317 | #define SOFTWARE_CENTER_LAUNCHERICON_H |
318 | |
319 | #include "BamfLauncherIcon.h" |
320 | +#include "LauncherDragWindow.h" |
321 | +#include "Launcher.h" |
322 | #include <UnityCore/GLibDBusProxy.h> |
323 | |
324 | namespace unity |
325 | @@ -36,10 +38,28 @@ |
326 | std::string const& aptdaemon_trans_id, |
327 | std::string const& icon_path); |
328 | |
329 | + void AddSelfToLauncher(); |
330 | + |
331 | + void Animate(nux::ObjectPtr<Launcher> launcher, int icon_x, int icon_y, int icon_size); |
332 | + |
333 | + std::string GetName() const; |
334 | + |
335 | +protected: |
336 | + void ActivateLauncherIcon(ActionArg arg); |
337 | + |
338 | private: |
339 | void OnPropertyChanged(GVariant* params); |
340 | |
341 | + static gboolean OnDragWindowAnimComplete(gpointer data); |
342 | + |
343 | glib::DBusProxy _aptdaemon_trans; |
344 | + |
345 | + nux::ObjectPtr<nux::IOpenGLBaseTexture> _icon_texture; |
346 | + LauncherDragWindow* _drag_window; |
347 | + Launcher* _launcher; |
348 | + AbstractLauncherIcon::Ptr self_abstract; |
349 | + bool _finished; |
350 | + bool _finished_just_now; |
351 | }; |
352 | |
353 | } |
354 | |
355 | === modified file 'tests/autopilot/autopilot/emulators/unity/icons.py' |
356 | --- tests/autopilot/autopilot/emulators/unity/icons.py 2012-03-04 23:12:19 +0000 |
357 | +++ tests/autopilot/autopilot/emulators/unity/icons.py 2012-03-15 17:05:24 +0000 |
358 | @@ -48,3 +48,6 @@ |
359 | |
360 | class DesktopLauncherIcon(SimpleLauncherIcon): |
361 | """Represents an icon that may appear in the switcher.""" |
362 | + |
363 | +class SoftwareCenterLauncherIcon(BamfLauncherIcon): |
364 | + """Represents a launcher icon of a Software Center app.""" |
365 | |
366 | === modified file 'tests/autopilot/autopilot/emulators/unity/launcher.py' |
367 | --- tests/autopilot/autopilot/emulators/unity/launcher.py 2012-03-13 20:44:08 +0000 |
368 | +++ tests/autopilot/autopilot/emulators/unity/launcher.py 2012-03-15 17:05:24 +0000 |
369 | @@ -7,6 +7,7 @@ |
370 | # by the Free Software Foundation. |
371 | # |
372 | |
373 | +import dbus |
374 | import logging |
375 | from time import sleep |
376 | |
377 | @@ -14,7 +15,7 @@ |
378 | from autopilot.emulators.unity import UnityIntrospectionObject |
379 | from autopilot.emulators.unity.icons import BamfLauncherIcon, SimpleLauncherIcon |
380 | from autopilot.emulators.X11 import Mouse, ScreenGeometry |
381 | - |
382 | +from autopilot.emulators.dbus_handler import session_bus |
383 | |
384 | logger = logging.getLogger(__name__) |
385 | |
386 | @@ -37,6 +38,18 @@ |
387 | def key_nav_monitor(self): |
388 | return self.key_nav_launcher_monitor |
389 | |
390 | + def add_launcher_item_from_position(self,name,icon,icon_x,icon_y,icon_size,desktop_file,aptdaemon_task): |
391 | + """ Emulate a DBus call from Software Center to pin an icon to the launcher """ |
392 | + launcher_object = session_bus.get_object('com.canonical.Unity.Launcher', |
393 | + '/com/canonical/Unity/Launcher') |
394 | + launcher_iface = dbus.Interface(launcher_object, 'com.canonical.Unity.Launcher') |
395 | + launcher_iface.AddLauncherItemFromPosition(name, |
396 | + icon, |
397 | + icon_x, |
398 | + icon_y, |
399 | + icon_size, |
400 | + desktop_file, |
401 | + aptdaemon_task) |
402 | |
403 | class Launcher(UnityIntrospectionObject, KeybindingsHelper): |
404 | """An individual launcher for a monitor.""" |
405 | |
406 | === modified file 'tests/autopilot/autopilot/tests/test_launcher.py' |
407 | --- tests/autopilot/autopilot/tests/test_launcher.py 2012-03-14 01:58:37 +0000 |
408 | +++ tests/autopilot/autopilot/tests/test_launcher.py 2012-03-15 17:05:24 +0000 |
409 | @@ -8,7 +8,7 @@ |
410 | # by the Free Software Foundation. |
411 | |
412 | import logging |
413 | -from testtools.matchers import Equals, NotEquals, LessThan, GreaterThan |
414 | +from testtools.matchers import Equals, NotEquals, LessThan, GreaterThan, Not, Is |
415 | from time import sleep |
416 | |
417 | from autopilot.tests import AutopilotTestCase |
418 | @@ -395,6 +395,41 @@ |
419 | self.assertThat(self.launcher.key_nav_is_active, Equals(True)) |
420 | self.assertThat(self.launcher.key_nav_is_grabbed, Equals(True)) |
421 | |
422 | + def test_software_center_add_icon(self): |
423 | + """ Test the ability to add a SoftwareCenterLauncherIcon """ |
424 | + |
425 | + def cleanup(): |
426 | + if icon is not None: |
427 | + launcher_instance.unlock_from_launcher(icon[0]) |
428 | + |
429 | + launcher_instance = self.get_launcher() |
430 | + sc_desktop_file = "/usr/share/applications/ubuntu-software-center.desktop" |
431 | + |
432 | + # Check if SC is pinned to the launcher already |
433 | + icon = self.launcher.model.get_icon_by_desktop_file(sc_desktop_file) |
434 | + if icon != None: |
435 | + launcher_instance.unlock_from_launcher(icon[0]) |
436 | + sleep(2.0) # Animation of removing icon can take over a second |
437 | + else: |
438 | + self.addCleanup(cleanup) |
439 | + |
440 | + self.launcher.add_launcher_item_from_position("Unity Test", |
441 | + "softwarecenter", |
442 | + 100, |
443 | + 100, |
444 | + 32, |
445 | + sc_desktop_file, |
446 | + "") |
447 | + |
448 | + sleep(1.0) |
449 | + |
450 | + icon = self.launcher.model.get_icon_by_desktop_file(sc_desktop_file) |
451 | + self.assertThat(icon, Not(Is(None))) |
452 | + |
453 | + # Check for whether: |
454 | + # The new launcher icon has a 'Waiting to install' tooltip |
455 | + self.assertThat(icon[0].tooltip_text, Equals("Waiting to install")) |
456 | + |
457 | class LauncherRevealTests(ScenariodLauncherTests): |
458 | """Test the launcher reveal bahavior when in autohide mode.""" |
459 |
Bug request for the whole issue was bug #761851, but it got marked Fix Released after phase 1. Not sure if I should mark it Triaged again, or not.