Merge lp://staging/~jjed/unity/tooltips-redux into lp://staging/unity

(1) Please fix the copyright dates in the new source files.
(2) Please do not prepend an underscore wart to indicate a member variable: the Unity style is to use appended underscore warts.
(3) You need to add a commit message to this MP.

Other than that, this seems OK.

I believe that should fix them. Thanks for the quick review!

+#define NULL_ICON (AbstractLauncherIcon::Ptr)nullptr

Don't do this please :)

314 +void TooltipManager::StopTimer() {
315 + hover_timer_.reset(new glib::Timeout(TOOLTIPS_SHOW_TIMEOUT_LENGTH));
316 +}

Is this the right way to stop the timeout?

Also I think you can unit test the new class ;)

> Don't do this please :)

Okay, removed.

> Is this the right way to stop the timeout?

It seems to be (you can `grep -R "reset(new glib::Timeout" to see similar uses). `hover_timer_->Remove() would also stop it, but then I'd just need add the same line afterwards to set a new timeout.

> Also I think you can unit test the new class

Okay, added. There's not much to test, as the modules only interaction with the outside world is calling `icon_->ShowTooltip()` and `icon_->HideTooltip()`. Tell me if there are any other test cases you'd like to see.

324 +void TooltipManager::StopTimer() {
325 + hover_timer_.reset(new glib::Timeout(TOOLTIPS_SHOW_TIMEOUT_LENGTH));
326 +}

Please just call hover_timer_.reset();

Then when you need to start it again, reset it to a new one.

> > Don't do this please :)
>
> Okay, removed.
>
> > Is this the right way to stop the timeout?
>
> It seems to be (you can `grep -R "reset(new glib::Timeout" to see similar
> uses). `hover_timer_->Remove() would also stop it, but then I'd just need add
> the same line afterwards to set a new timeout.

hover_timer_->reset() should just work.

>
> > Also I think you can unit test the new class
>
> Okay, added. There's not much to test, as the modules only interaction with
> the outside world is calling `icon_->ShowTooltip()` and
> `icon_->HideTooltip()`. Tell me if there are any other test cases you'd like
> to see.

Okay, that works (and does fewer glib::Timeout allocations too).

81 + SetIconUnderMouse((AbstractLauncherIcon::Ptr)nullptr);

Please pass an empty icon PTR to the function as it expects:
SetIconUnderMouse(AbstractLauncherIcon::Ptr());

245 + : show_tooltips_(false)
246 + , hovered_(false)
247 + , icon_(nullptr)
248 + , icon_clicked_(false)

This is pure style, but we use 2-spaces to indent initialization list, also you don't need to initialize the icon_ at all.

366 +class TooltipManager : public sigc::trackable

You don't need sigc::trackable, but you probably want this to be non-copyable instead.

371 + void SetHover(bool on_launcher);

What about a nux::Property<bool> for this?

258 + if (show_tooltips_) {

Please add a space after the if statements (so that the brace is on the new line)

539 + tm.SetIcon((AbstractLauncherIcon::Ptr)icon);

Pass to it an icon ptr: tm.SetIcon(AbstractLauncherIcon::Ptr(icon));

Okay, changed most of that.

> 371 + void SetHover(bool on_launcher);
>
> What about a nux::Property<bool> for this?

I don't think that makes sense, given that `hover_` is private and the TooltipMannager doesn't need any internal hooks for when it changes. LauncherHoverMachine already has a signal for changing hover-state as well.

Cool, this is fine now..

Stephen, are you ok now with removing the "Needs fixing" flag? :)

Looks good.

As Launcher icons don't visually change at all when hovered with the mouse, Unity again feels so sluggish/unresponsive, that I have to wonder if it has freezed. I reported that as bug 1152390.