Merge lp:~azzar1/unity/properly-handle-copy-dialog into lp:unity

[Andrea Azzarone]

Properly handle the file manager copy dialog in FileManagerLauncherIcon and in StorageLauncherIcon.

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

Sorry for being late, but... Yeah, although I'd like to get rid of that WindowManager::Default().IsWindowMapped(app_win->window_id()) check, I know that there are multiple races involved there, so we can't probably avoid it.

Just wondering if it could happen that the mapping would happen afterwards and we'd miss a window because there's no other signal catching that (and compiz might not signal us anything)...

But things work ok with this, so let's put it in production.

review: Approve

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

Ouch, there are merging issues with trunk... Please fix this.

review: Needs Fixing

[lp:~azzar1/unity/properly-handle-copy-dialog]

4199. By Andrea Azzarone on 2016-11-07

Merge trunk and resolve conflicts.

[Andrea Azzarone (azzar1)]

> Ouch, there are merging issues with trunk... Please fix this.

Done.

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

Ok, thanks!

review: Approve

[Amr Ibrahim (amribrahim1987)]

The related bugs were reported against Xenial but there were not fixed there. Please push this fix to Xenial. Users are still affected in 16.04. Have a look at bug #1399077 and its duplicates.

=== modified file 'launcher/FileManagerLauncherIcon.cpp'
--- launcher/FileManagerLauncherIcon.cpp	2016-07-05 13:35:56 +0000
+++ launcher/FileManagerLauncherIcon.cpp	2016-11-07 11:49:14 +0000
@@ -49,6 +49,11 @@
   SetQuirk(Quirk::VISIBLE, false);
   SkipQuirkAnimation(Quirk::VISIBLE);
 
+  signals_conn_.Add(app_->window_opened.connect([this](ApplicationWindowPtr const& win) {
+    signals_conn_.Add(win->monitor.changed.connect([this] (int) { UpdateStorageWindows(); }));
+    UpdateStorageWindows();
+  }));
+
   signals_conn_.Add(app_->desktop_file.changed.connect([this](std::string const& desktop_file) {
     LOG_DEBUG(logger) << tooltip_text() << " desktop_file now " << desktop_file;
     UpdateDesktopFile();
@@ -134,5 +139,30 @@
   return StorageLauncherIcon::OnShouldHighlightOnDrag(dnd_data);
 }
 
+bool FileManagerLauncherIcon::IsUserVisible() const
+{
+  return ApplicationLauncherIcon::IsUserVisible();
+}
+
+WindowList FileManagerLauncherIcon::WindowsOnViewport()
+{
+  WindowFilterMask filter = 0;
+  filter |= WindowFilter::MAPPED;
+  filter |= WindowFilter::ON_CURRENT_DESKTOP;
+  filter |= WindowFilter::ON_ALL_MONITORS;
+
+  return WindowedLauncherIcon::GetWindows(filter);
+}
+
+WindowList FileManagerLauncherIcon::WindowsForMonitor(int monitor)
+{
+  WindowFilterMask filter = 0;
+  filter |= WindowFilter::MAPPED;
+  filter |= WindowFilter::ON_CURRENT_DESKTOP;
+
+  return WindowedLauncherIcon::GetWindows(filter, monitor);
+}
+
+
 } // namespace launcher
 } // namespace unity

=== modified file 'launcher/FileManagerLauncherIcon.h'
--- launcher/FileManagerLauncherIcon.h	2015-12-09 12:02:48 +0000
+++ launcher/FileManagerLauncherIcon.h	2016-11-07 11:49:14 +0000
@@ -32,8 +32,16 @@
 class FileManagerLauncherIcon : public ApplicationLauncherIcon, public StorageLauncherIcon
 {
 public:
+  typedef nux::ObjectPtr<FileManagerLauncherIcon> Ptr;
+
   FileManagerLauncherIcon(ApplicationPtr const&, DeviceLauncherSection::Ptr const&, FileManager::Ptr const& = nullptr);
 
+  bool IsUserVisible() const override;
+
+protected:
+  WindowList WindowsOnViewport() override;
+  WindowList WindowsForMonitor(int monitor) override;
+
 private:
   WindowList GetManagedWindows() const override;
   WindowList GetStorageWindows() const override;

=== modified file 'launcher/StorageLauncherIcon.cpp'
--- launcher/StorageLauncherIcon.cpp	2015-12-08 14:31:30 +0000
+++ launcher/StorageLauncherIcon.cpp	2016-11-07 11:49:14 +0000
@@ -36,7 +36,7 @@
   bool active = false;
   bool urgent = false;
   bool check_visibility = (GetIconType() == IconType::APPLICATION);
-  bool visible = IsSticky();
+  bool visible = false;
 
   managed_windows_ = GetStorageWindows();
   windows_connections_.Clear();
@@ -54,13 +54,8 @@
     if (!urgent && win->urgent())
       urgent = true;
 
-    if (check_visibility)
-    {
-      windows_connections_.Add(win->visible.changed.connect([this] (bool) { OnWindowStateChanged(); }));
-
-      if (!visible && win->visible())
-        visible = true;
-    }
+    if (check_visibility && !visible)
+      visible = true;
   }
 
   SetQuirk(Quirk::RUNNING, !managed_windows_.empty());
@@ -68,7 +63,7 @@
   SetQuirk(Quirk::URGENT, urgent);
 
   if (check_visibility)
-    SetQuirk(Quirk::VISIBLE, visible);
+    SetQuirk(Quirk::VISIBLE, visible || IsSticky());
 
   EnsureWindowsLocation();
 }
@@ -83,7 +78,7 @@
   bool active = false;
   bool urgent = false;
   bool check_visibility = (GetIconType() == IconType::APPLICATION);
-  bool visible = IsSticky();
+  bool visible = false;
 
   for (auto const& win : managed_windows_)
   {
@@ -93,7 +88,7 @@
     if (!urgent && win->urgent())
       urgent = true;
 
-    if (check_visibility && !visible && win->visible())
+    if (check_visibility && !visible)
       visible = true;
   }
 
@@ -101,7 +96,7 @@
   SetQuirk(Quirk::URGENT, urgent);
 
   if (check_visibility)
-    SetQuirk(Quirk::VISIBLE, visible);
+    SetQuirk(Quirk::VISIBLE, visible || IsSticky());
 }
 
 bool StorageLauncherIcon::OnShouldHighlightOnDrag(DndData const& dnd_data)

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2016-10-27 08:40:57 +0000
+++ tests/CMakeLists.txt	2016-11-07 11:49:14 +0000
@@ -224,6 +224,7 @@
                  test_error_preview.cpp
                  test_edge_barrier_controller.cpp
                  test_expo_launcher_icon.cpp
+                 test_file_manager_launcher_icon.cpp
                  test_filter_widgets.cpp
                  test_glib_dbus_server.cpp
                  test_gnome_session_manager.cpp

=== added file 'tests/test_file_manager_launcher_icon.cpp'
--- tests/test_file_manager_launcher_icon.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_file_manager_launcher_icon.cpp	2016-11-07 11:49:14 +0000
@@ -0,0 +1,202 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the  Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 3 along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>
+ *
+ * Authored by: Andrea Azzarone <[e-mail]>
+ */
+
+#include <gmock/gmock.h>
+using namespace testing;
+
+#include "FileManagerLauncherIcon.h"
+#include "UnityCore/DesktopUtilities.h"
+
+#include "test_mock_devices.h"
+#include "test_mock_filemanager.h"
+#include "mock-application.h"
+
+using namespace unity;
+using namespace unity::launcher;
+using namespace testmocks;
+
+namespace
+{
+
+const std::string TRASH_URI = "trash:";
+const std::string TRASH_PATH = "file://" + DesktopUtilities::GetUserTrashDirectory();
+
+struct TestFileManagerLauncherIcon : public Test
+{
+  TestFileManagerLauncherIcon()
+    : app_(std::make_shared<MockApplication::Nice>())
+    , fm_(std::make_shared<MockFileManager::Nice>())
+    , dev_ (std::make_shared<MockDeviceLauncherSection>(2))
+    , icon_(new FileManagerLauncherIcon(app_, dev_, fm_))
+  {
+  }
+
+  MockApplication::Ptr app_;
+  MockFileManager::Ptr fm_;
+  DeviceLauncherSection::Ptr dev_;
+  FileManagerLauncherIcon::Ptr icon_;
+};
+
+TEST_F(TestFileManagerLauncherIcon, IconType)
+{
+  EXPECT_EQ(icon_->GetIconType(), AbstractLauncherIcon::IconType::APPLICATION);
+}
+
+TEST_F(TestFileManagerLauncherIcon, NoWindow)
+{
+  EXPECT_FALSE(icon_->IsVisible());
+}
+
+TEST_F(TestFileManagerLauncherIcon, NoManagedWindow_TrashUri)
+{
+  EXPECT_CALL(*fm_, LocationForWindow(_)).Times(1);
+  ON_CALL(*fm_, LocationForWindow(_)).WillByDefault(Return(TRASH_URI));
+
+  auto win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  app_->windows_ = { win };
+  app_->window_opened.emit(win);
+
+  EXPECT_FALSE(icon_->IsVisible());
+  EXPECT_FALSE(icon_->IsRunning());
+}
+
+TEST_F(TestFileManagerLauncherIcon, NoManagedWindow_TrashPath)
+{
+  EXPECT_CALL(*fm_, LocationForWindow(_)).Times(1);
+  ON_CALL(*fm_, LocationForWindow(_)).WillByDefault(Return(TRASH_PATH));
+
+  auto win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  app_->windows_ = { win };
+  app_->window_opened.emit(win);
+
+  EXPECT_FALSE(icon_->IsVisible());
+  EXPECT_FALSE(icon_->IsRunning());
+}
+
+TEST_F(TestFileManagerLauncherIcon, NoManagedWindow_Device)
+{
+  auto const& device_icons = dev_->GetIcons();
+  ASSERT_EQ(2, device_icons.size());
+
+  device_icons.at(0)->Activate(ActionArg());
+
+  EXPECT_CALL(*fm_, LocationForWindow(_)).Times(1);
+  ON_CALL(*fm_, LocationForWindow(_)).WillByDefault(Return(device_icons.at(0)->GetVolumeUri()));
+
+  auto win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  app_->windows_ = { win };
+  app_->window_opened.emit(win);
+
+  EXPECT_FALSE(icon_->IsVisible());
+  EXPECT_FALSE(icon_->IsRunning());
+}
+
+TEST_F(TestFileManagerLauncherIcon, ManagedWindows)
+{
+  EXPECT_CALL(*fm_, LocationForWindow(_)).Times(1);
+  ON_CALL(*fm_, LocationForWindow(_)).WillByDefault(Return("/usr/bin"));
+
+  auto win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  app_->windows_ = { win };
+  app_->window_opened.emit(win);
+
+  EXPECT_TRUE(icon_->IsVisible());
+  EXPECT_TRUE(icon_->IsRunning());
+}
+
+TEST_F(TestFileManagerLauncherIcon, ManagedWindows_EmptyLocation)
+{
+  EXPECT_CALL(*fm_, LocationForWindow(_)).Times(1);
+  ON_CALL(*fm_, LocationForWindow(_)).WillByDefault(Return(""));
+
+  auto win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  app_->windows_ = { win };
+  app_->window_opened.emit(win);
+
+  EXPECT_TRUE(icon_->IsVisible());
+  EXPECT_TRUE(icon_->IsRunning());
+}
+
+TEST_F(TestFileManagerLauncherIcon, ManagedWindows_CopyDialog)
+{
+  EXPECT_CALL(*fm_, LocationForWindow(_)).Times(1);
+  ON_CALL(*fm_, LocationForWindow(_)).WillByDefault(Return(""));
+
+  auto win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  win->visible_ = false;
+  app_->windows_ = { win };
+  app_->window_opened.emit(win);
+
+  EXPECT_TRUE(icon_->IsVisible());
+  EXPECT_TRUE(icon_->IsRunning());
+}
+
+
+TEST_F(TestFileManagerLauncherIcon, ManagedWindows_CopyDialogAndManagedWindow)
+{
+  EXPECT_CALL(*fm_, LocationForWindow(_)).Times(3);
+  ON_CALL(*fm_, LocationForWindow(_)).WillByDefault(Return(""));
+
+  auto win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  app_->windows_ = { win };
+  app_->window_opened.emit(win);
+
+  win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  win->visible_ = false;
+  app_->windows_.push_back(win);
+  app_->window_opened.emit(win);
+
+  EXPECT_TRUE(icon_->IsVisible());
+  EXPECT_TRUE(icon_->IsRunning());
+  EXPECT_EQ(2, icon_->WindowsVisibleOnMonitor(0));
+}
+
+TEST_F(TestFileManagerLauncherIcon, ManagedWindows_CopyDialogAndNoManagedWindow)
+{
+  {
+    InSequence s;
+
+    EXPECT_CALL(*fm_, LocationForWindow(_))
+      .Times(1)
+      .WillOnce(Return(""));
+
+    EXPECT_CALL(*fm_, LocationForWindow(_))
+      .Times(1)
+      .WillOnce(Return(""));
+
+    EXPECT_CALL(*fm_, LocationForWindow(_))
+      .Times(1)
+      .WillOnce(Return(TRASH_PATH));
+  }
+
+  auto win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  app_->windows_ = { win };
+  app_->window_opened.emit(win);
+
+  win = std::make_shared<MockApplicationWindow::Nice>(g_random_int());
+  win->visible_ = false;
+  app_->windows_.push_back(win);
+  app_->window_opened.emit(win);
+
+  EXPECT_TRUE(icon_->IsVisible());
+  EXPECT_TRUE(icon_->IsRunning());
+  EXPECT_EQ(1, icon_->WindowsVisibleOnMonitor(0));
+}
+
+}