sync with lp:~charlesk/indicator-display/lp-1572545-prompt-in-lockscreen

4 files changed, 185 insertions, 138 deletions

diff --git a/src/greeter.cpp b/src/greeter.cpp
index f9cd965..9bd5db0 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -26,117 +26,166 @@ class UnityGreeter::Impl
 {
 public:
 
-    Impl():
-        m_cancellable{g_cancellable_new()}
+    Impl()
     {
-        g_bus_get(G_BUS_TYPE_SESSION, m_cancellable, on_bus_ready_static, this);
+        m_cancellable.reset(
+            g_cancellable_new(),
+            [](GCancellable* o){
+                g_cancellable_cancel(o);
+                g_clear_object(&o);
+            }
+        );
+
+        g_bus_get(G_BUS_TYPE_SESSION, m_cancellable.get(), on_bus_ready, this);
     }
 
-    ~Impl()
-    {
-        g_cancellable_cancel(m_cancellable);
-        g_clear_object(&m_cancellable);
-
-        if (m_subscription_id != 0)
-            g_dbus_connection_signal_unsubscribe (m_bus, m_subscription_id);
+    ~Impl() =default;
 
-        g_clear_object(&m_bus);
-    }
-
-    core::Property<bool>& is_active()
+    core::Property<State>& state()
     {
-        return m_is_active;
+        return m_state;
     }
 
 private:
 
-    static void on_bus_ready_static(GObject* /*source*/, GAsyncResult* res, gpointer gself)
+    void set_state(const State& state)
+    {
+        m_state.set(state);
+    }
+
+    static void on_bus_ready(
+        GObject* /*source*/,
+        GAsyncResult* res,
+        gpointer gself)
     {
         GError* error {};
-        auto bus = g_bus_get_finish (res, &error);
-        if (error != nullptr) {
+        auto bus = g_bus_get_finish(res, &error);
+        if (error != nullptr)
+        {
             if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-                g_warning("UsbSnap: Error getting session bus: %s", error->message);
+                g_warning("Greeter: Error getting bus: %s", error->message);
             g_clear_error(&error);
-        } else {
-            static_cast<Impl*>(gself)->on_bus_ready(bus);
         }
-        g_clear_object(&bus);
+        else
+        {
+            auto self = static_cast<Impl*>(gself);
+
+            const auto watcher_id = g_bus_watch_name_on_connection(
+                bus,
+                DBusNames::UnityGreeter::NAME,
+                G_BUS_NAME_WATCHER_FLAGS_AUTO_START,
+                on_greeter_appeared,
+                on_greeter_vanished,
+                gself,
+                nullptr);
+
+            const auto subscription_id = g_dbus_connection_signal_subscribe(
+                bus,
+                DBusNames::UnityGreeter::NAME,
+                DBusNames::Properties::INTERFACE,
+                DBusNames::Properties::PropertiesChanged::NAME,
+                DBusNames::UnityGreeter::PATH,
+                DBusNames::UnityGreeter::INTERFACE,
+                G_DBUS_SIGNAL_FLAGS_NONE,
+                on_properties_changed_signal,
+                gself,
+                nullptr);
+
+            self->m_bus.reset(
+                bus,
+                [watcher_id, subscription_id](GDBusConnection* o){
+                    g_bus_unwatch_name(watcher_id);
+                    g_dbus_connection_signal_unsubscribe(o, subscription_id);
+                    g_clear_object(&o);
+                }
+            );
+        }
     }
 
-    void on_bus_ready(GDBusConnection* bus)
+    static void on_greeter_appeared(
+        GDBusConnection* bus,
+        const char* /*name*/,
+        const char* name_owner,
+        gpointer gself)
     {
-        m_bus = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(bus)));
-
-        g_dbus_connection_call(m_bus,
-                               DBusNames::UnityGreeter::NAME,
-                               DBusNames::UnityGreeter::PATH,
-                               DBusNames::Properties::INTERFACE,
-                               "Get",
-                               g_variant_new("(ss)", DBusNames::UnityGreeter::INTERFACE, "IsActive"),
-                               G_VARIANT_TYPE("(v)"),
-                               G_DBUS_CALL_FLAGS_NONE,
-                               -1,
-                               m_cancellable,
-                               on_get_is_active_ready,
-                               this);
-
-        m_subscription_id = g_dbus_connection_signal_subscribe(m_bus,
-                                                               DBusNames::UnityGreeter::NAME,
-                                                               DBusNames::Properties::INTERFACE,
-                                                               DBusNames::Properties::PropertiesChanged::NAME,
-                                                               DBusNames::UnityGreeter::PATH,
-                                                               DBusNames::UnityGreeter::INTERFACE,
-                                                               G_DBUS_SIGNAL_FLAGS_NONE,
-                                                               on_properties_changed_signal,
-                                                               this,
-                                                               nullptr);
+        auto self = static_cast<Impl*>(gself);
+
+        self->m_owner = name_owner;
+
+        g_dbus_connection_call(
+            bus,
+            DBusNames::UnityGreeter::NAME,
+            DBusNames::UnityGreeter::PATH,
+            DBusNames::Properties::INTERFACE,
+            "Get",
+            g_variant_new("(ss)", DBusNames::UnityGreeter::INTERFACE, "IsActive"),
+            G_VARIANT_TYPE("(v)"),
+            G_DBUS_CALL_FLAGS_NONE,
+            -1,
+            self->m_cancellable.get(),
+            on_get_is_active_ready,
+            gself);
     }
 
-    static void on_get_is_active_ready(GObject* source, GAsyncResult* res, gpointer gself)
+    static void on_greeter_vanished(
+        GDBusConnection* /*bus*/,
+        const char* /*name*/,
+        gpointer gself)
+    {
+        auto self = static_cast<Impl*>(gself);
+
+        self->m_owner.clear();
+        self->set_state(State::UNAVAILABLE);
+    }
+
+    static void on_get_is_active_ready(
+        GObject* source,
+        GAsyncResult* res, 
+        gpointer gself)
     {
         GError* error {};
         auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error);
         if (error != nullptr) {
             if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-                g_warning("UsbSnap: Error getting session bus: %s", error->message);
+                g_warning("Greeter: Error getting IsActive property: %s", error->message);
             g_clear_error(&error);
         } 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)->set_state(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);
     }
 
-    static void on_properties_changed_signal(GDBusConnection* /*connection*/,
-                                             const gchar* /*sender_name*/,
-                                             const gchar* object_path,
-                                             const gchar* interface_name,
-                                             const gchar* signal_name,
-                                             GVariant* parameters,
-                                             gpointer gself)
+    static void on_properties_changed_signal(
+        GDBusConnection* /*bus*/,
+        const gchar* sender_name,
+        const gchar* object_path,
+        const gchar* interface_name,
+        const gchar* signal_name,
+        GVariant* parameters,
+        gpointer gself)
     {
+        auto self = static_cast<Impl*>(gself);
+
+        g_return_if_fail(!g_strcmp0(sender_name, self->m_owner.c_str()));
         g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH));
         g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Properties::INTERFACE));
         g_return_if_fail(!g_strcmp0(signal_name, DBusNames::Properties::PropertiesChanged::NAME));
         g_return_if_fail(g_variant_is_of_type(parameters, G_VARIANT_TYPE(DBusNames::Properties::PropertiesChanged::ARGS_VARIANT_TYPE)));
 
-        auto v = g_variant_get_child_value (parameters, 1);
+        auto v = g_variant_get_child_value(parameters, 1);
         gboolean is_active {};
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
-        {
-            g_debug("%s is_active changed to %d", G_STRLOC, int(is_active));
-            static_cast<Impl*>(gself)->m_is_active.set(is_active);
-        }
+            self->set_state(is_active ? State::ACTIVE : State::INACTIVE);
         g_clear_pointer(&v, g_variant_unref);
     }
 
-    core::Property<bool> m_is_active;
-    GCancellable* m_cancellable {};
-    GDBusConnection* m_bus {};
-    unsigned int m_subscription_id {};
+    core::Property<State> m_state {State::UNAVAILABLE};
+    std::shared_ptr<GCancellable> m_cancellable;
+    std::shared_ptr<GDBusConnection> m_bus;
+    std::string m_owner;
 };
 
 /***
@@ -154,8 +203,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..f3012f6 100644
--- a/src/greeter.h
+++ b/src/greeter.h
@@ -29,7 +29,13 @@ class Greeter
 public:
     Greeter();
     virtual ~Greeter();
-    virtual core::Property<bool>& is_active() =0;
+
+    enum class State { UNAVAILABLE, INACTIVE, ACTIVE };
+static inline const char* state_str(const State& state) {
+    static constexpr char const * state_str[] = { "Unavailable", "Inactive", "Active" };
+    return state_str[int(state)];
+}
+    virtual core::Property<State>& state() =0;
 };
 
 
@@ -38,7 +44,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 8b24f17..81c1b9d 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -48,103 +48,94 @@ public:
 g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_usb_monitor->on_usb_disconnected().connect([this](const std::string& /*usb_name*/) {
 g_message("%s %s", G_STRLOC, G_STRFUNC);
-            restart();
+            m_req.reset();
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         });
 
 g_message("%s %s", G_STRLOC, G_STRFUNC);
-        m_greeter->is_active().changed().connect([this](bool /*is_active*/) {
+        m_greeter->state().changed().connect([this](const Greeter::State& state) {
+g_message("%s %s", G_STRLOC, G_STRFUNC);
+            if (state == Greeter::State::INACTIVE)
+                maybe_snap();
+            else
+                stop_snap();
 g_message("%s %s", G_STRLOC, G_STRFUNC);
-            maybe_snap();
         });
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-        restart();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-    }
+        // create a new adbd client
+        m_adbd_client.reset(new GAdbdClient{m_socket_path});
+        m_adbd_client->on_pk_request().connect(
+            [this](const AdbdClient::PKRequest& req) {
+                g_debug("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str());
+
+                m_response = AdbdClient::PKResponse::DENY; // set the fallback response
+                m_req.reset(
+                    new AdbdClient::PKRequest(req),
+                    [this](AdbdClient::PKRequest* r) {
+                        stop_snap();
+                        r->respond(m_response);
+                        delete r;
+                    }
+                );
+                maybe_snap();
+            }
+        );
 
     ~Impl()
     {
 g_message("%s %s", G_STRLOC, G_STRFUNC);
-        if (m_restart_idle_tag)
-            g_source_remove(m_restart_idle_tag);
-
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-        clear();
+        if (m_request_complete_idle_tag)
+            g_source_remove(m_request_complete_idle_tag);
 g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
 private:
 
-    void clear()
+    void stop_snap()
     {
 g_message("%s %s", G_STRLOC, G_STRFUNC);
-        // clear out old state
         m_snap_connections.clear();
 g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_snap.reset();
 g_message("%s %s", G_STRLOC, G_STRFUNC);
-        m_req = decltype(m_req)();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-        m_adbd_client.reset();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
-    void restart()
+    void maybe_snap()
     {
 g_message("%s %s", G_STRLOC, G_STRFUNC);
-        clear();
+        // only prompt if there's something to prompt about
+        if (!m_req)
+            return;
 
 g_message("%s %s", G_STRLOC, G_STRFUNC);
-        // set a new client
-        m_adbd_client.reset(new GAdbdClient{m_socket_path});
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-        m_adbd_client->on_pk_request().connect(
-            [this](const AdbdClient::PKRequest& req) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                g_debug("%s got pk request: %s", G_STRLOC, req.fingerprint.c_str());
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                m_req = req;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                maybe_snap();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-            }
-        );
-    }
+        // only prompt in an unlocked session
+        if (m_greeter->state().get() != Greeter::State::INACTIVE)
+            return;
 
-    void maybe_snap()
-    {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-        // don't prompt in the greeter!
-        if (!m_req.public_key.empty() && !m_greeter->is_active().get())
-            snap();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
+        snap();
     }
 
     void snap()
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-        m_snap = std::make_shared<UsbSnap>(m_req.fingerprint);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
+        m_snap = std::make_shared<UsbSnap>(m_req->fingerprint);
         m_snap_connections.insert((*m_snap).on_user_response().connect(
             [this](AdbdClient::PKResponse response, bool remember_choice){
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                g_debug("%s thread %p user responded! response %d, remember %d", G_STRLOC, g_thread_self(), int(response), int(remember_choice));
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                m_req.respond(response);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
+
                 if (remember_choice && (response == AdbdClient::PKResponse::ALLOW))
-                    write_public_key(m_req.public_key);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                m_restart_idle_tag = g_idle_add([](gpointer gself){
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                    auto self = static_cast<Impl*>(gself);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                    self->m_restart_idle_tag = 0;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                    self->restart();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                    return G_SOURCE_REMOVE;
-                }, this);
+                    write_public_key(m_req->public_key);
+
+                m_response = response;
+
+                // defer finishing the request into an idle func because
+                // ScopedConnections can't be destroyed inside their callbacks
+                if (m_request_complete_idle_tag == 0) {
+                    m_request_complete_idle_tag = g_idle_add([](gpointer gself){
+                        auto self = static_cast<Impl*>(gself);
+                        self->m_request_complete_idle_tag = 0;
+                        self->m_req.reset();
+                        return G_SOURCE_REMOVE;
+                    }, this);
+                }
             }
         ));
 g_message("%s %s", G_STRLOC, G_STRFUNC);
@@ -194,12 +185,13 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
     const std::shared_ptr<UsbMonitor> m_usb_monitor;
     const std::shared_ptr<Greeter> m_greeter;
  
-    unsigned int m_restart_idle_tag {};
+    unsigned int m_request_complete_idle_tag {};
 
     std::shared_ptr<GAdbdClient> m_adbd_client;
-    AdbdClient::PKRequest m_req;
     std::shared_ptr<UsbSnap> m_snap;
     std::set<core::ScopedConnection> m_snap_connections;
+    AdbdClient::PKResponse m_response {AdbdClient::PKResponse::DENY};
+    std::shared_ptr<AdbdClient::PKRequest> m_req;
 };
 
 /***
@@ -218,5 +210,4 @@ UsbManager::UsbManager(
 
 UsbManager::~UsbManager()
 {
-}
-
+}
\ No newline at end of file
diff --git a/src/usb-snap.cpp b/src/usb-snap.cpp
index ba964fb..53db6c4 100644
--- a/src/usb-snap.cpp
+++ b/src/usb-snap.cpp
@@ -208,6 +208,7 @@ private:
         const bool remember_this_choice = response == AdbdClient::PKResponse::ALLOW;
 
         m_on_user_response(response, remember_this_choice);
+        m_notification_id = 0;
     }
 
     void on_notification_closed(uint32_t close_reason)