Merge lp:~bilalakhtar/unity/sc-integration-phase2 into lp:unity

Proposed by Bilal Akhtar
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
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.

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://wiki.ubuntu.com/SoftwareCenter#Learning_how_to_launch_an_application

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://code.launchpad.net/~bilalakhtar/unity/fix-sc-launcher-integration/+merge/93908 . The fix has been waiting for review for more than 10 days now, can someone ensure that it gets in for Unity 5.6.0? Since that branch merely contains a fix (all strings have been in since before 5.2.0) so neither that branch nor this branch needs a UIFe.

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://www.youtube.com/watch?v=6peOjOrcURA

To post a comment you must log in.
Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

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.

Revision history for this message
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?

Revision history for this message
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.

review: Approve (design)
Revision history for this message
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.

Revision history for this message
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. ;)).

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

64 + sigc::signal<void, std::string const&, AbstractLauncherIcon::Ptr, std::string const&, std::string const&,
65 + gint32, gint32, gint32> launcher_addrequest_special;

You can just use int at this level, so we are more C++ conformant.

139 + ((SoftwareCenterLauncherIcon*)result.GetPointer())->Animate(launcher_, icon_x, icon_y, icon_size);

Don't use C-style casts.

184 + finished = true;
185 + finished_just_now = true;

Set them using the constructor initializer list.

201 +void SoftwareCenterLauncherIcon::AddSelfToLauncher()
202 +{
203 + _launcher->icon_animation_complete.emit(self_abstract);
204 +}

306 + void AddSelfToLauncher();

322 + AbstractLauncherIcon::Ptr self_abstract;

I don't really think you need those, please don't do that. You already handling an AbstractLauncherIcon.

244 + target_x = (int)current_bamf_icon->GetCenter(launcher->monitor).x;

Again, use static_cast

256 + g_timeout_add(30, &SoftwareCenterLauncherIcon::OnDragWindowAnimComplete, this);

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 OnDragWindowAnimComplete(gpointer data);

Put in private fields.

310 + void Animate(nux::ObjectPtr<Launcher> launcher, gint32 icon_x, gint32 icon_y, gint32 icon_size);

You can also just use Animate(nux::ObjectPtr<Launcher> const& launcher ... And move to pure int's.

312 + void ActivateLauncherIcon(ActionArg arg);

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.

review: Needs Fixing
Revision history for this message
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).

Revision history for this message
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

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

189 + _finished = true;
190 + _finished_just_now = true;

You don't need them anymore.

329 + AbstractLauncherIcon::Ptr self_abstract;

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)current_bamf_icon->GetCenter(launcher->monitor).x) != 0 &&
248 + ((int)current_bamf_icon->GetCenter(launcher->monitor).y) != 0)

Those are still C-style casts :)

139 + static_cast<SoftwareCenterLauncherIcon*>(result.GetPointer())->Animate(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.

Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

Branch is ready again, please review :)

2013. By Bilal Akhtar

Change C-style casts

Revision history for this message
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 + AbstractLauncherIcon::Ptr self_abstract;
>
> 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)current_bamf_icon->GetCenter(launcher->monitor).x) != 0 &&
> 248 + ((int)current_bamf_icon->GetCenter(launcher->monitor).y) !=
> 0)
>
> Those are still C-style casts :)

Fixed in latest commit.

>
> 139 + static_cast<SoftwareCenterLauncherIcon*>(result.GetPointer())->Anim
> 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 SoftwareCenterLauncherIcon and nothing else. So a static cast is safe here.

Thanks for the review anyway, it would be great if you reviewed again!

2014. By Bilal Akhtar

Fix a line in tests

Revision history for this message
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 + AbstractLauncherIcon::Ptr self_abstract;
> >
> > 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 SoftwareCenterLauncherIcon(SimpleLauncherIcon):
358 + """Represents a launcher icon of a Software Center app."""

Also on tests SoftwareCenterLauncherIcon should extend BamfLauncherIcon.
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.

Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

I myself have given up on the AbstractLauncherIcon::Ptr thing after multiple hits and misses. I think it's okay in it's current form, an extra line or two but not too ugly.

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.

Revision history for this message
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.

review: Needs Fixing
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :
Download full text (6.2 KiB)

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::launcher::SoftwareCenterLauncherIcon::Animate (this=0x0, launcher=..., icon_x=100, icon_y=100, icon_size=32)
    at /home/thomi/code/canonical/unity/sc-integration-phase2/plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp:92
#1 0x00007fffe509ecd7 in unity::launcher::Controller::Impl::OnLauncherAddRequestSpecial (this=0x903c90, path=..., before=..., aptdaemon_trans_id=..., icon_path=...,
    icon_x=100, icon_y=100, icon_size=32) at /home/thomi/code/canonical/unity/sc-integration-phase2/plugins/unityshell/src/LauncherController.cpp:406
#2 0x00007fffe50aa989 in sigc::bound_mem_functor7<void, unity::launcher::Controller::Impl, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int>::operator() (this=0xd24498, _A_a1=..., _A_a2=..., _A_a3=..., _A_a4=..., _A_a5=@0x7fffffffe0f0: 100,
    _A_a6=@0x7fffffffe0f4: 100, _A_a7=@0x7fffffffe0f8: 32) at /usr/include/sigc++-2.0/sigc++/functors/mem_fun.h:2277
#3 0x00007fffe50a9d5f in sigc::adaptor_functor<sigc::bound_mem_functor7<void, unity::launcher::Controller::Impl, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int> >::operator()<std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon> const&, std::string const&, std::string const&, int const&, int const&, int const&> (this=0xd24490, _A_arg1=..., _A_arg2=..., _A_arg3=..., _A_arg4=...,
    _A_arg5=@0x7fffffffe0f0: 100, _A_arg6=@0x7fffffffe0f4: 100, _A_arg7=@0x7fffffffe0f8: 32) at /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:213
#4 0x00007fffe50a89b2 in sigc::internal::slot_call7<sigc::bound_mem_functor7<void, unity::launcher::Controller::Impl, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int>, void, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int>::call_it (rep=0xd24460, a_1=..., a_2=..., a_3=..., a_4=..., a_5=@0x7fffffffe0f0: 100, a_6=@0x7fffffffe0f4: 100,
    a_7=@0x7fffffffe0f8: 32) at /usr/include/sigc++-2.0/sigc++/functors/slot.h:383
#5 0x00007fffe508b1e9 in sigc::internal::signal_emit7<void, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int, sigc::nil>::emit (impl=0xd24410, _A_a1=..., _A_a2=..., _A_a3=..., _A_a4=..., _A_a5=@0x7fffffffe0f0: 100, _A_a6=@0x7fffffffe0f4: 100,
    _A_a7=@0x7fffffffe0f8: 32) at /usr/include/sigc++-2.0/sigc++/signal.h:2567
#6 0x00007fffe508995d in sigc::signal7<void, std::string const&, nux::ObjectPtr<unity::launcher::AbstractLauncherIcon>, std::string const&, std::string const&, int, int, int, sigc::nil>::emit (this=0xca1690, _A_a1=..., _A_a2=..., _A_a3=..., _A_a4=..., _A_a5=@0x7fffffffe0f0: 100, _A_a6=@0x7fffffffe0f4: 100, _A_a7=@0x7fffffffe0f8: 32)
    at /usr/include/sigc++-2.0/sigc++/s...

Read more...

review: Needs Fixing
2015. By Bilal Akhtar

Fix crash on second install, THANKS THOMI

Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

Thomi:

I've fixed the crash, pull the branch and you won't have the problem again :)

Revision history for this message
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

Revision history for this message
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.

review: Needs Fixing (test)
2017. By Bilal Akhtar

Cleanup tests, also first ensure Software Center is removed

Revision history for this message
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?

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Note that it will need a Feature Freeze and UI Freeze exception acked. Can you please proceed it?

Revision history for this message
Bilal Akhtar (bilalakhtar) wrote :

I thought that was after the code review? Anyway, I'll do that now.

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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 AddLauncherItemFromPosition call pin the launcher, or is it transient?

review: Needs Fixing
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

Revision history for this message
Alex Launi (alexlauni) wrote :

emulate_software_center_dbus_call is sort of a bad name, this might get used somewhere else so just wrap the launcher function and call it add_launcher_itom_from_position.

instead of importing dbus and doing bus.SessionBus just do:
   from autopilot.emulators.dbus_handler import session_bus
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(icon, Not(Is(None)))

Instead of asserting that icon_text == "blah", Equals(True) do this
self.assertThat(icon[0].tooltip_text, Equals("Waiting to install"))

cleanup should go like this,
add an anonymous function to your test
def cleanup():
    if icon is not None:
        launcher_instance.unlock_from_launcher(icon[0])
then
icon = self.launcher.model.get_icon_by_desktop_file(DESKTOP_FILE_PATH)
if icon is not None:
    launcher_instance.unlock_from_launcher(icon[0])
    sleep(1)
else:
    self.addCleanup(cleanup)

review: Needs Fixing
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Also, make sure to cover in your test this change (it should be implicit, I guess): https://code.launchpad.net/~bilalakhtar/unity/fix-sc-launcher-integration/+merge/93908 so we can merge all together (if we get the exception, otherwise you need to backport the tests to only that branch).

Revision history for this message
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://code.launchpad.net/~bilalakhtar/unity/fix-sc-launcher-integration/+merge/93908so
we can merge all together (if we get the exception, otherwise you need
to backport the tests to only that branch).
--
https://code.launchpad.net/~bilalakhtar/unity/sc-integration-phase2/+merge/95795
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

Revision history for this message
Alex Launi (alexlauni) wrote :

Tests now look good Bilal

review: Approve (testing)
Revision history for this message
Alex Launi (alexlauni) wrote :

I missed this yesterday, please move def cleanup() to be the first thing in test_software_center_add_icon

review: Needs Fixing
2022. By Bilal Akhtar

Move definition of cleanup function - thanks Alex

2023. By Bilal Akhtar

Merge from trunk

Revision history for this message
Alex Launi (alexlauni) wrote :

Aced

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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