Merge lp:~townsend/unity/fix-urgent-icon-wiggle into lp:unity

[Christopher Townsend]

Add functionality to wiggle urgent icons at certain time intervals when the Launcher is hidden.

= Issue =
Currently, when the Launcher is hidden and an urgent icon is displayed, the only notification a user sees that the initial pop out wiggle of the icon and then nothing else. IF the user misses this, then they may be unaware of an app that needs attention.

= Fix =
Add timers and logic to pop out and wiggle the icon at certain intervals as defined in bug #893196. Also, handle the wiggle icons once the Launcher is revealed.

= Test =
This is really a visual issue and no tests automated tests are identified to handle this.
A manual test would be to have an application set an icon urgent and watch for the pop out wiggle at the defined intervals. Also, reveal the Launcher to see the icon(s) wiggle one last time.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3322
http://jenkins.qa.ubuntu.com/job/unity-ci/55/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-amd64-ci/55
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/55
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-i386-ci/55

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/55/rebuild

review: Approve (continuous-integration)

[Brandon Schaefer (brandontschaefer)]

Looks nice :)

review: Approve

[Brandon Schaefer (brandontschaefer)]

Yay

review: Approve

[Christopher Townsend (townsend)]

Setting the overall status to Needs Review since I'd like to get Marco's input on tests since he said it may be possible for automated tests.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3325
http://jenkins.qa.ubuntu.com/job/unity-ci/111/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-amd64-ci/115
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-armhf-ci/111
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-raring-i386-ci/111

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/111/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

68 +void Launcher::HandleUrgentIcon(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current)

Since the function is called at every redraw, why not just returning at the beginning of it in case of: if (options()->hide_mode == LAUNCHER_HIDE_NEVER) ?

187 + bool _urgent_acked;
Please include _urgent_acked in the initialization list as well. Also, even if it seems to work properly even if there's more than on icon, wouldn't probably be better to have a set of icons to wiggle instead?

124 + for (it = _model->main_begin(); it != _model->main_end(); ++it)

Using a for-each loop here is nicer...

127 + if (icon->GetQuirk(AbstractLauncherIcon::Quirk::URGENT) &&

Traling space here

153 + sources_.Remove(URGENT_TIMEOUT);

This is not needed since it's automatically done when returning false on this callback function (see line 238 of SourceManager::Add on GLibSource.cpp).

157 + SetUrgentTimer(_urgent_wiggle_time);

Mh, why the current timer is not deleted in case you set up a new timer with different period?

[Marco Trevisan (Treviño) (3v1n0)]

Ah, on the bug description there's written:
«- When the Launcher is visible, application alerts should should be indicated by a short wiggle (in addition to the existing blue pips). This short wiggle should not repeat.»

I wonder this applies to the case where the launcher is always visible, and it that case it should wiggle only when the urgency is set and not after a minute or so, right?

[Christopher Townsend (townsend)]

Hi Marco,

> 68 +void Launcher::HandleUrgentIcon(AbstractLauncherIcon::Ptr const&
> icon, struct timespec const& current)
>
> Since the function is called at every redraw, why not just returning at the
> beginning of it in case of: if (options()->hide_mode == LAUNCHER_HIDE_NEVER) ?

Yeah, that's a good point. I may put this in the if statement before calling HandleUrgentIcon().

>
> 187 + bool _urgent_acked;
> Please include _urgent_acked in the initialization list as well. Also, even if
> it seems to work properly even if there's more than on icon, wouldn't probably
> be better to have a set of icons to wiggle instead?

Sure, I can put it in the initialization list. Not sure what you really mean by your question about a set of icons though.

>
>
> 124 + for (it = _model->main_begin(); it != _model->main_end(); ++it)
>
> Using a for-each loop here is nicer...

Yes indeed.

>
> 127 + if (icon->GetQuirk(AbstractLauncherIcon::Quirk::URGENT) &&
>
> Traling space here

OK

>
>
> 153 + sources_.Remove(URGENT_TIMEOUT);
>
> This is not needed since it's automatically done when returning false on this
> callback function (see line 238 of SourceManager::Add on GLibSource.cpp).

Ah, didn't catch that.

>
>
> 157 + SetUrgentTimer(_urgent_wiggle_time);
>
> Mh, why the current timer is not deleted in case you set up a new timer with
> different period?

I thought the current time is automatically deleted when setting up a new timer with the same name.

[Christopher Townsend (townsend)]

> Ah, on the bug description there's written:
> «- When the Launcher is visible, application alerts should should be indicated
> by a short wiggle (in addition to the existing blue pips). This short wiggle
> should not repeat.»
>
> I wonder this applies to the case where the launcher is always visible, and it
> that case it should wiggle only when the urgency is set and not after a minute
> or so, right?

That is the way I understand this too which is the current behavior.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3328
http://jenkins.qa.ubuntu.com/job/unity-ci/122/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/10

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/122/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

Nice...

229 + EXPECT_FALSE(launcher_->_urgent_timer_running);

244 + EXPECT_EQ(launcher_->_urgent_wiggle_time, 0);
245 + EXPECT_EQ(launcher_->_urgent_finished_time.tv_sec, 0);
246 + EXPECT_EQ(launcher_->_urgent_finished_time.tv_nsec, 0);

Probably better to use ASSERT_EQ there^.

Is it now possible to also test that the icons wiggle once the launcher is shown?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3329
http://jenkins.qa.ubuntu.com/job/unity-ci/126/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/14

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/126/rebuild

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

I think we're fine now, thanks! :)

review: Approve

=== modified file 'launcher/Launcher.cpp'
--- launcher/Launcher.cpp	2013-05-15 04:59:15 +0000
+++ launcher/Launcher.cpp	2013-06-10 19:12:26 +0000
@@ -96,9 +96,14 @@
 const int SCROLL_AREA_HEIGHT = 24;
 const int SCROLL_FPS = 30;
 
+const int BASE_URGENT_WIGGLE_PERIOD = 60; // In seconds
+const int MAX_URGENT_WIGGLE_DELTA = 960;  // In seconds
+const int DOUBLE_TIME = 2;
+
 const std::string START_DRAGICON_TIMEOUT = "start-dragicon-timeout";
 const std::string SCROLL_TIMEOUT = "scroll-timeout";
 const std::string ANIMATION_IDLE = "animation-idle";
+const std::string URGENT_TIMEOUT = "urgent-timeout";
 }
 
 
@@ -145,6 +150,10 @@
   , _launcher_drag_delta_min(0)
   , _enter_y(0)
   , _last_button_press(0)
+  , _urgent_wiggle_time(0)
+  , _urgent_acked(false)
+  , _urgent_timer_running(false)
+  , _urgent_ack_needed(false)
   , _drag_out_delta_x(0.0f)
   , _drag_gesture_ongoing(false)
   , _last_reveal_progress(0.0f)
@@ -194,6 +203,9 @@
     _times[i].tv_nsec = 0;
   }
 
+  _urgent_finished_time.tv_sec = 0;
+  _urgent_finished_time.tv_nsec = 0;
+
   ubus_.RegisterInterest(UBUS_OVERLAY_SHOWN, sigc::mem_fun(this, &Launcher::OnOverlayShown));
   ubus_.RegisterInterest(UBUS_OVERLAY_HIDDEN, sigc::mem_fun(this, &Launcher::OnOverlayHidden));
   ubus_.RegisterInterest(UBUS_LAUNCHER_LOCK_HIDE, sigc::mem_fun(this, &Launcher::OnLockHideChanged));
@@ -1133,6 +1145,11 @@
   {
     RenderArg arg;
     AbstractLauncherIcon::Ptr const& icon = *it;
+
+    if (icon->GetQuirk(AbstractLauncherIcon::Quirk::URGENT) && options()->hide_mode == LAUNCHER_HIDE_AUTOHIDE)
+    {
+      HandleUrgentIcon(icon, current);
+    }
     FillRenderArg(icon, arg, center, parent_abs_geo, folding_threshold, folded_size, folded_spacing,
                   autohide_offset, folded_z_distance, animation_neg_rads, current);
     arg.colorify = colorify;
@@ -1566,6 +1583,112 @@
   }
 }
 
+void Launcher::SetUrgentTimer(int urgent_wiggle_period)
+{
+  sources_.AddTimeoutSeconds(urgent_wiggle_period, sigc::mem_fun(this, &Launcher::OnUrgentTimeout), URGENT_TIMEOUT);
+}
+
+void Launcher::WiggleUrgentIcon(AbstractLauncherIcon::Ptr const& icon)
+{
+  icon->SetQuirk(AbstractLauncherIcon::Quirk::URGENT, false);
+  icon->SetQuirk(AbstractLauncherIcon::Quirk::URGENT, true);
+
+  clock_gettime(CLOCK_MONOTONIC, &_urgent_finished_time);
+}
+
+void Launcher::HandleUrgentIcon(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current)
+{
+  struct timespec urgent_time = icon->GetQuirkTime(AbstractLauncherIcon::Quirk::URGENT);
+  DeltaTime urgent_delta = unity::TimeUtil::TimeDelta(&urgent_time, &_urgent_finished_time);
+
+  // If the Launcher is hidden, then add a timer to wiggle the urgent icons at 
+  // certain intervals (1m, 2m, 4m, 8m, 16m, & 32m).
+  if (_hidden && !_urgent_timer_running && urgent_delta > 0)
+  {
+    _urgent_timer_running = true;
+    _urgent_ack_needed = true;
+    SetUrgentTimer(BASE_URGENT_WIGGLE_PERIOD);
+  }
+  // If the Launcher is hidden, the timer is running, an urgent icon is newer than the last time
+  // icons were wiggled, and the timer did not just start, then reset the timer since a new
+  // urgent icon just showed up.
+  else if (_hidden && _urgent_timer_running && urgent_delta > 0 && _urgent_wiggle_time != 0)
+  {
+    _urgent_wiggle_time = 0;
+    SetUrgentTimer(BASE_URGENT_WIGGLE_PERIOD);
+  }
+  // If the Launcher is no longer hidden, then after the Launcher is fully revealed, wiggle the 
+  // urgent icon and then stop the timer (if it's running).
+  else if (!_hidden && _urgent_ack_needed)
+  {
+    if (_last_reveal_progress > 0)
+    {
+      _urgent_acked = false;
+    }
+    else
+    {
+      if (!_urgent_acked && IconUrgentProgress(icon, current) == 1.0f)
+      {
+        WiggleUrgentIcon(icon);
+      }
+      else if (IconUrgentProgress(icon, current) < 1.0f)
+      {
+        if (_urgent_timer_running)
+        {
+          sources_.Remove(URGENT_TIMEOUT);
+          _urgent_timer_running = false;
+        }
+        _urgent_acked = true;
+        _urgent_ack_needed = false;
+      }
+    }
+  }
+}
+
+bool Launcher::OnUrgentTimeout()
+{
+  bool foundUrgent = false,
+       continue_urgent = true;
+
+  if (options()->urgent_animation() == URGENT_ANIMATION_WIGGLE && _hidden)
+  {
+    // Look for any icons that are still urgent and wiggle them
+    for (auto icon : *_model)
+    {
+      if (icon->GetQuirk(AbstractLauncherIcon::Quirk::URGENT))
+      {
+        WiggleUrgentIcon(icon);
+
+        foundUrgent = true;
+      }
+    }
+  }
+  // Update the time for when the next wiggle will occur.
+  if (_urgent_wiggle_time == 0)
+  {
+    _urgent_wiggle_time = BASE_URGENT_WIGGLE_PERIOD;
+  }
+  else
+  {
+    _urgent_wiggle_time = _urgent_wiggle_time * DOUBLE_TIME;
+  }
+
+  // If no urgent icons were found or we have reached the time threshold,
+  // then let's stop the timer.  Otherwise, start a new timer with the
+  // updated wiggle time.
+  if (!foundUrgent || (_urgent_wiggle_time > MAX_URGENT_WIGGLE_DELTA))
+  {
+    continue_urgent = false;
+    _urgent_timer_running = false;
+  }
+  else
+  {
+    SetUrgentTimer(_urgent_wiggle_time);
+  }
+
+  return continue_urgent;
+}
+
 void Launcher::SetIconSize(int tile_size, int icon_size)
 {
   ui::IconRenderer::DestroyShortcutTextures();
 
=== modified file 'launcher/Launcher.h'
--- launcher/Launcher.h	2013-03-06 15:44:44 +0000
+++ launcher/Launcher.h	2013-06-10 19:12:26 +0000
@@ -223,6 +223,11 @@
   bool StartIconDragTimeout(int x, int y);
   bool OnScrollTimeout();
 
+  void SetUrgentTimer(int urgent_wiggle_period);
+  void WiggleUrgentIcon(AbstractLauncherIcon::Ptr const& icon);
+  void HandleUrgentIcon(AbstractLauncherIcon::Ptr const& icon, struct timespec const& current);
+  bool OnUrgentTimeout();
+
   void SetMousePosition(int x, int y);
   void SetIconUnderMouse(AbstractLauncherIcon::Ptr const& icon);
 
@@ -380,6 +385,10 @@
   int _enter_y;
   int _last_button_press;
   int _drag_icon_position;
+  int _urgent_wiggle_time;
+  bool _urgent_acked;
+  bool _urgent_timer_running;
+  bool _urgent_ack_needed;
   float _drag_out_delta_x;
   bool _drag_gesture_ongoing;
   float _last_reveal_progress;
@@ -396,6 +405,7 @@
   Atom _selection_atom;
 
   struct timespec  _times[TIME_LAST];
+  struct timespec _urgent_finished_time;
 
   BaseTexturePtr launcher_sheen_;
   BaseTexturePtr launcher_pressure_effect_;
 
=== modified file 'tests/test_launcher.cpp'
--- tests/test_launcher.cpp	2013-04-02 18:47:58 +0000
+++ tests/test_launcher.cpp	2013-06-10 19:12:26 +0000
@@ -108,6 +108,11 @@
     using Launcher::IconStartingPulseValue;
     using Launcher::HandleBarrierEvent;
     using Launcher::SetHidden;
+    using Launcher::HandleUrgentIcon;
+    using Launcher::SetUrgentTimer;
+    using Launcher::_urgent_timer_running;
+    using Launcher::_urgent_finished_time;
+    using Launcher::_urgent_wiggle_time;
 
     void FakeProcessDndMove(int x, int y, std::list<std::string> uris)
     {
@@ -618,6 +623,72 @@
   launcher_->ProcessDndMove(0,0,{});
 }
 
+TEST_F(TestLauncher, UrgentIconWiggleTimerStart)
+{
+  MockMockLauncherIcon::Ptr icon(new MockMockLauncherIcon);
+  timespec current;
+
+  launcher_->SetHidden(true);
+  icon->SetQuirk(AbstractLauncherIcon::Quirk::URGENT, true);
+
+  clock_gettime(CLOCK_MONOTONIC, &current);
+
+  ASSERT_FALSE(launcher_->_urgent_timer_running);
+
+  launcher_->HandleUrgentIcon(icon, current);
+
+  EXPECT_TRUE(launcher_->_urgent_timer_running);
+}
+
+TEST_F(TestLauncher, UrgentIconWiggleTimerTimeout)
+{
+  MockMockLauncherIcon::Ptr icon(new MockMockLauncherIcon);
+
+  model_->AddIcon(icon);
+  launcher_->SetHidden(true);
+  icon->SetQuirk(AbstractLauncherIcon::Quirk::URGENT, true);
+
+  ASSERT_EQ(launcher_->_urgent_wiggle_time, 0);
+  ASSERT_EQ(launcher_->_urgent_finished_time.tv_sec, 0);
+  ASSERT_EQ(launcher_->_urgent_finished_time.tv_nsec, 0);
+  
+  launcher_->SetUrgentTimer(1);
+
+  // Make sure the Urgent Timer expires before checking
+  Utils::WaitForTimeout(2);
+  
+  EXPECT_THAT(launcher_->_urgent_wiggle_time, Gt(0));
+  EXPECT_THAT(launcher_->_urgent_finished_time.tv_sec, Gt(0));
+  EXPECT_THAT(launcher_->_urgent_finished_time.tv_nsec, Gt(0));
+}
+
+TEST_F(TestLauncher, WiggleUrgentIconAfterLauncherIsRevealed)
+{
+  MockMockLauncherIcon::Ptr icon(new MockMockLauncherIcon);
+  timespec current;
+
+  model_->AddIcon(icon);
+  launcher_->SetHidden(true);
+  icon->SetQuirk(AbstractLauncherIcon::Quirk::URGENT, true);
+
+  // Wait a bit to simulate the icon's initial wiggle
+  Utils::WaitForTimeout(1);
+
+  clock_gettime(CLOCK_MONOTONIC, &current);
+  launcher_->HandleUrgentIcon(icon, current);
+
+  ASSERT_EQ(launcher_->_urgent_finished_time.tv_sec, 0);
+  ASSERT_EQ(launcher_->_urgent_finished_time.tv_nsec, 0);
+
+  launcher_->SetHidden(false);
+
+  clock_gettime(CLOCK_MONOTONIC, &current);
+  launcher_->HandleUrgentIcon(icon, current);
+
+  EXPECT_THAT(launcher_->_urgent_finished_time.tv_sec, Gt(0));
+  EXPECT_THAT(launcher_->_urgent_finished_time.tv_nsec, Gt(0));  
+}  
+
 }
 }