don't prompt when the greeter's not running yet: change greeter's payload from an 'is_active' bool to a three-value state of active, inactive, and unavailable

7 files changed, 62 insertions, 31 deletions

diff --git a/src/greeter.cpp b/src/greeter.cpp
index d41160f..317a829 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -41,9 +41,9 @@ public:
 
     ~Impl() =default;
 
-    core::Property<bool>& is_active()
+    core::Property<State>& state()
     {
-        return m_is_active;
+        return m_state;
     }
 
 private:
@@ -130,7 +130,7 @@ private:
         auto self = static_cast<Impl*>(gself);
 
         self->m_owner.clear();
-        self->m_is_active.set(false);
+        self->m_state.set(State::UNAVAILABLE);
     }
 
     static void on_get_is_active_ready(
@@ -147,7 +147,7 @@ private:
         } else {
             GVariant* is_active {};
             g_variant_get_child(v, 0, "v", &is_active);
-            static_cast<Impl*>(gself)->m_is_active.set(g_variant_get_boolean(is_active));
+            static_cast<Impl*>(gself)->m_state.set(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE);
             g_clear_pointer(&is_active, g_variant_unref);
         }
         g_clear_pointer(&v, g_variant_unref);
@@ -175,12 +175,12 @@ private:
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
         {
             g_debug("%s is_active changed to %d", G_STRLOC, int(is_active));
-            self->m_is_active.set(is_active);
+            self->m_state.set(is_active ? State::ACTIVE : State::INACTIVE);
         }
         g_clear_pointer(&v, g_variant_unref);
     }
 
-    core::Property<bool> m_is_active {false};
+    core::Property<State> m_state {State::UNAVAILABLE};
     std::shared_ptr<GCancellable> m_cancellable;
     std::shared_ptr<GDBusConnection> m_bus;
     std::string m_owner;
@@ -201,8 +201,8 @@ UnityGreeter::UnityGreeter():
 
 UnityGreeter::~UnityGreeter() =default;
 
-core::Property<bool>&
-UnityGreeter::is_active()
+core::Property<Greeter::State>&
+UnityGreeter::state()
 {
-    return impl->is_active();
+    return impl->state();
 }
diff --git a/src/greeter.h b/src/greeter.h
index e084d25..cde1429 100644
--- a/src/greeter.h
+++ b/src/greeter.h
@@ -29,7 +29,9 @@ class Greeter
 public:
     Greeter();
     virtual ~Greeter();
-    virtual core::Property<bool>& is_active() =0;
+
+    enum class State { UNAVAILABLE, INACTIVE, ACTIVE };
+    virtual core::Property<State>& state() =0;
 };
 
 
@@ -38,7 +40,7 @@ class UnityGreeter: public Greeter
 public:
     UnityGreeter();
     virtual ~UnityGreeter();
-    core::Property<bool>& is_active() override;
+    core::Property<State>& state() override;
 
 protected:
     class Impl;
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 4d750c0..80b274b 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -49,7 +49,7 @@ public:
             restart();
         });
 
-        m_greeter->is_active().changed().connect([this](bool /*is_active*/) {
+        m_greeter->state().changed().connect([this](Greeter::State /*state*/) {
             maybe_snap();
         });
 
@@ -92,9 +92,15 @@ private:
 
     void maybe_snap()
     {
-        // don't prompt in the greeter!
-        if (!m_req.public_key.empty() && !m_greeter->is_active().get())
-            snap();
+        // only prompt if there's something to prompt about
+        if (m_req.public_key.empty())
+            return;
+
+        // only prompt in an unlocked session
+        if (m_greeter->state().get() != Greeter::State::INACTIVE)
+            return;
+
+        snap();
     }
 
     void snap()
diff --git a/tests/integration/usb-manager-test.cpp b/tests/integration/usb-manager-test.cpp
index d62756f..7320640 100644
--- a/tests/integration/usb-manager-test.cpp
+++ b/tests/integration/usb-manager-test.cpp
@@ -206,7 +206,7 @@ TEST_F(UsbManagerFixture, Greeter)
     auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, std::vector<std::string>{"PK"+public_key});
 
     // set up a UsbManager to process the request
-    m_greeter->m_is_active.set(true);
+    m_greeter->m_state.set(Greeter::State::ACTIVE);
     auto usb_manager = std::make_shared<UsbManager>(*socket_path, *public_keys_path, m_usb_monitor, m_greeter);
 
     // add a signal spy to listen to the notification daemon
@@ -219,7 +219,7 @@ TEST_F(UsbManagerFixture, Greeter)
     EXPECT_FALSE(notificationsSpy.wait(2000));
 
     // disable the greeter, the notification should appear
-    m_greeter->m_is_active.set(false);
+    m_greeter->m_state.set(Greeter::State::INACTIVE);
     wait_for_signals(notificationsSpy, 1);
     EXPECT_EQ("Notify", notificationsSpy.at(0).at(0));
     notificationsSpy.clear();
diff --git a/tests/unit/greeter-test.cpp b/tests/unit/greeter-test.cpp
index 72df3bc..bfa88e8 100644
--- a/tests/unit/greeter-test.cpp
+++ b/tests/unit/greeter-test.cpp
@@ -18,6 +18,7 @@
  */
 
 #include <tests/utils/qt-fixture.h>
+#include <tests/utils/gtest-print-helpers.h>
 
 #include <src/dbus-names.h>
 #include <src/greeter.h>
@@ -110,45 +111,49 @@ protected:
 
 TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher)
 {
-    constexpr bool expected {true};
+    constexpr bool is_active {true};
+    constexpr Greeter::State expected {Greeter::State::ACTIVE};
 
-    start_greeter_service(expected);
+    start_greeter_service(is_active);
 
     UnityGreeter greeter;
 
-    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state());
 }
 
 TEST_F(GreeterFixture, WatcherStartsBeforeActiveService)
 {
-    constexpr bool expected {true};
+    constexpr bool is_active {true};
+    constexpr Greeter::State expected {Greeter::State::ACTIVE};
 
     UnityGreeter greeter;
 
-    start_greeter_service(expected);
+    start_greeter_service(is_active);
 
-    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state());
 }
 
 TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher)
 {
-    constexpr bool expected {false};
+    constexpr bool is_active {false};
+    constexpr Greeter::State expected {Greeter::State::INACTIVE};
 
-    start_greeter_service(expected);
+    start_greeter_service(is_active);
 
     UnityGreeter greeter;
 
-    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state());
 }
 
 TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService)
 {
-    constexpr bool expected {false};
+    constexpr bool is_active {false};
+    constexpr Greeter::State expected {Greeter::State::INACTIVE};
 
     UnityGreeter greeter;
 
-    start_greeter_service(expected);
+    start_greeter_service(is_active);
 
-    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state());
 }
 
diff --git a/tests/utils/gtest-print-helpers.h b/tests/utils/gtest-print-helpers.h
new file mode 100644
index 0000000..75ee13b
--- /dev/null
+++ b/tests/utils/gtest-print-helpers.h
@@ -0,0 +1,18 @@
+
+#pragma once
+
+#include <src/greeter.h>
+
+inline void PrintTo(const Greeter::State& state, std::ostream* os) {
+    switch(state) {
+        case Greeter::State::ACTIVE:   *os << "Active"; break;
+        case Greeter::State::INACTIVE: *os << "Inactive"; break;
+        default:                       *os << "Unavailable"; break;
+    }
+}
+
+std::ostream& operator<<(std::ostream& os, const Greeter::State& state) {
+    PrintTo(state, &os);
+    return os;
+}
+
diff --git a/tests/utils/mock-greeter.h b/tests/utils/mock-greeter.h
index 5ac85a0..5015087 100644
--- a/tests/utils/mock-greeter.h
+++ b/tests/utils/mock-greeter.h
@@ -26,7 +26,7 @@ class MockGreeter: public Greeter
 public:
     MockGreeter() =default;
     virtual ~MockGreeter() =default;
-    core::Property<bool>& is_active() override {return m_is_active;}
-    core::Property<bool> m_is_active {false};
+    core::Property<Greeter::State>& state() override {return m_state;}
+    core::Property<Greeter::State> m_state {State::INACTIVE};
 };