From 19506296905cddb9a286921a0d56ab22c404aa12 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 20 Apr 2016 15:09:44 -0500 Subject: debug tracers --- src/usb-manager.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/usb-manager.cpp') diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp index 4d750c0..d2d82c3 100644 --- a/src/usb-manager.cpp +++ b/src/usb-manager.cpp @@ -92,6 +92,7 @@ private: void maybe_snap() { +g_message("%s m_req.public_key.empty() %d && m_greeter->is_active().get() %d", G_STRLOC, int(m_req.public_key.empty()), int(m_greeter->is_active().get())); // don't prompt in the greeter! if (!m_req.public_key.empty() && !m_greeter->is_active().get()) snap(); -- cgit v1.2.3 From e1284eb774d75f96d61787f116b0d79328b27286 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 20 Apr 2016 17:32:17 -0500 Subject: remove tracers --- src/greeter.cpp | 10 +--------- src/usb-manager.cpp | 1 - 2 files changed, 1 insertion(+), 10 deletions(-) (limited to 'src/usb-manager.cpp') diff --git a/src/greeter.cpp b/src/greeter.cpp index 9d331d7..903bd2a 100644 --- a/src/greeter.cpp +++ b/src/greeter.cpp @@ -30,8 +30,6 @@ public: m_bus(G_DBUS_CONNECTION(g_object_ref(connection))), m_cancellable{g_cancellable_new()} { -g_message("%s %s", G_STRLOC, G_STRFUNC); - m_watcher_id = g_bus_watch_name_on_connection( m_bus, DBusNames::UnityGreeter::NAME, @@ -76,7 +74,6 @@ private: const char* name_owner, gpointer gself) { - g_message("%s %s", G_STRLOC, G_STRFUNC); auto self = static_cast(gself); self->m_owner = name_owner; @@ -101,7 +98,6 @@ private: const char* /*name*/, gpointer gself) { - g_message("%s %s", G_STRLOC, G_STRFUNC); auto self = static_cast(gself); self->m_owner.clear(); @@ -113,7 +109,6 @@ private: GAsyncResult* res, gpointer gself) { - g_message("%s", G_STRLOC); GError* error {}; auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error); if (error != nullptr) { @@ -121,7 +116,6 @@ private: g_warning("Greeter: Error getting IsActive property: %s", error->message); g_clear_error(&error); } else { - g_message("%s v is %s", G_STRLOC, g_variant_print(v, true)); GVariant* is_active {}; g_variant_get_child(v, 0, "v", &is_active); static_cast(gself)->m_is_active.set(g_variant_get_boolean(is_active)); @@ -148,13 +142,11 @@ private: 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); - if (v != nullptr) - g_message("%s v is %s", G_STRLOC, g_variant_print(v, true)); gboolean is_active {}; if (g_variant_lookup(v, "IsActive", "b", &is_active)) { - g_message("%s is_active changed to %d", G_STRLOC, int(is_active)); + g_debug("%s is_active changed to %d", G_STRLOC, int(is_active)); self->m_is_active.set(is_active); } g_clear_pointer(&v, g_variant_unref); diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp index d2d82c3..4d750c0 100644 --- a/src/usb-manager.cpp +++ b/src/usb-manager.cpp @@ -92,7 +92,6 @@ private: void maybe_snap() { -g_message("%s m_req.public_key.empty() %d && m_greeter->is_active().get() %d", G_STRLOC, int(m_req.public_key.empty()), int(m_greeter->is_active().get())); // don't prompt in the greeter! if (!m_req.public_key.empty() && !m_greeter->is_active().get()) snap(); -- cgit v1.2.3 From ce5234162fa0c534ff9abf3fce3d03f4b01e893e Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 22 Apr 2016 09:43:44 -0500 Subject: 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 --- src/greeter.cpp | 18 +++++++++--------- src/greeter.h | 6 ++++-- src/usb-manager.cpp | 14 ++++++++++---- tests/integration/usb-manager-test.cpp | 4 ++-- tests/unit/greeter-test.cpp | 29 +++++++++++++++++------------ tests/utils/gtest-print-helpers.h | 18 ++++++++++++++++++ tests/utils/mock-greeter.h | 4 ++-- 7 files changed, 62 insertions(+), 31 deletions(-) create mode 100644 tests/utils/gtest-print-helpers.h (limited to 'src/usb-manager.cpp') 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& is_active() + core::Property& state() { - return m_is_active; + return m_state; } private: @@ -130,7 +130,7 @@ private: auto self = static_cast(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(gself)->m_is_active.set(g_variant_get_boolean(is_active)); + static_cast(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 m_is_active {false}; + core::Property m_state {State::UNAVAILABLE}; std::shared_ptr m_cancellable; std::shared_ptr m_bus; std::string m_owner; @@ -201,8 +201,8 @@ UnityGreeter::UnityGreeter(): UnityGreeter::~UnityGreeter() =default; -core::Property& -UnityGreeter::is_active() +core::Property& +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& is_active() =0; + + enum class State { UNAVAILABLE, INACTIVE, ACTIVE }; + virtual core::Property& state() =0; }; @@ -38,7 +40,7 @@ class UnityGreeter: public Greeter public: UnityGreeter(); virtual ~UnityGreeter(); - core::Property& is_active() override; + core::Property& 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(*socket_path, std::vector{"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(*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 +#include #include #include @@ -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 + +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& is_active() override {return m_is_active;} - core::Property m_is_active {false}; + core::Property& state() override {return m_state;} + core::Property m_state {State::INACTIVE}; }; -- cgit v1.2.3 From 7d2fac8f46110a84d8396251cd0f38351b92f6ce Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 25 Apr 2016 11:05:22 +0200 Subject: add tracer log messages --- src/greeter.cpp | 24 ++++++++++++++++-------- src/greeter.h | 4 ++++ src/usb-manager.cpp | 6 +++++- 3 files changed, 25 insertions(+), 9 deletions(-) (limited to 'src/usb-manager.cpp') diff --git a/src/greeter.cpp b/src/greeter.cpp index 317a829..ed210b0 100644 --- a/src/greeter.cpp +++ b/src/greeter.cpp @@ -48,6 +48,12 @@ public: private: + void set_state(const State& state) + { +g_message("%s setting state to %s", G_STRLOC, state_str(state)); + m_state.set(state); + } + static void on_bus_ready( GObject* /*source*/, GAsyncResult* res, @@ -99,10 +105,11 @@ private: static void on_greeter_appeared( GDBusConnection* bus, - const char* /*name*/, + const char* name, const char* name_owner, gpointer gself) { +g_message("%s %s appared on bus, owned by %s", G_STRLOC, name, name_owner); auto self = static_cast(gself); self->m_owner = name_owner; @@ -124,13 +131,14 @@ private: static void on_greeter_vanished( GDBusConnection* /*bus*/, - const char* /*name*/, + const char* name, gpointer gself) { +g_message("%s %s disappeared from bus", G_STRLOC, name); auto self = static_cast(gself); self->m_owner.clear(); - self->m_state.set(State::UNAVAILABLE); + self->set_state(State::UNAVAILABLE); } static void on_get_is_active_ready( @@ -138,6 +146,7 @@ private: GAsyncResult* res, gpointer gself) { +g_message("%s", G_STRLOC); GError* error {}; auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error); if (error != nullptr) { @@ -145,9 +154,10 @@ private: g_warning("Greeter: Error getting IsActive property: %s", error->message); g_clear_error(&error); } else { +g_message("%s got '%s'", G_STRLOC, g_variant_print(v, true)); GVariant* is_active {}; g_variant_get_child(v, 0, "v", &is_active); - static_cast(gself)->m_state.set(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE); + static_cast(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); @@ -163,6 +173,7 @@ private: gpointer gself) { auto self = static_cast(gself); +g_message("%s on_properties_changed got %s", G_STRLOC, g_variant_print(parameters, true)); g_return_if_fail(!g_strcmp0(sender_name, self->m_owner.c_str())); g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH)); @@ -173,10 +184,7 @@ private: 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)); - self->m_state.set(is_active ? State::ACTIVE : State::INACTIVE); - } + self->set_state(is_active ? State::ACTIVE : State::INACTIVE); g_clear_pointer(&v, g_variant_unref); } diff --git a/src/greeter.h b/src/greeter.h index cde1429..f3012f6 100644 --- a/src/greeter.h +++ b/src/greeter.h @@ -31,6 +31,10 @@ public: virtual ~Greeter(); 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() =0; }; diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp index 80b274b..9f67720 100644 --- a/src/usb-manager.cpp +++ b/src/usb-manager.cpp @@ -50,6 +50,7 @@ public: }); m_greeter->state().changed().connect([this](Greeter::State /*state*/) { +g_message("%s greeter state changed; calling maybe_snap()", G_STRLOC); maybe_snap(); }); @@ -83,7 +84,7 @@ private: 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", G_STRLOC, req.fingerprint.c_str()); + g_message("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str()); m_req = req; maybe_snap(); } @@ -92,14 +93,17 @@ private: void maybe_snap() { +g_message("%s m_req.public_key [%s]", G_STRLOC, m_req.public_key.c_str()); // only prompt if there's something to prompt about if (m_req.public_key.empty()) return; +g_message("%s m_greeter->state() %s", G_STRLOC, Greeter::state_str(m_greeter->state().get())); // only prompt in an unlocked session if (m_greeter->state().get() != Greeter::State::INACTIVE) return; +g_message("%s calling snap!", G_STRLOC); snap(); } -- cgit v1.2.3 From f8a8646b5cc0e9b59a5f953dcd8129c93d162915 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 27 Apr 2016 09:56:53 +0200 Subject: add tracers for the notification warning --- src/usb-manager.cpp | 11 +++++++++-- src/usb-snap.cpp | 6 ++++++ 2 files changed, 15 insertions(+), 2 deletions(-) (limited to 'src/usb-manager.cpp') diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp index 9f67720..ce546fe 100644 --- a/src/usb-manager.cpp +++ b/src/usb-manager.cpp @@ -50,6 +50,8 @@ public: }); m_greeter->state().changed().connect([this](Greeter::State /*state*/) { +g_message("%s clearing old snap, if any", G_STRLOC); + clear_snap(); g_message("%s greeter state changed; calling maybe_snap()", G_STRLOC); maybe_snap(); }); @@ -67,11 +69,16 @@ g_message("%s greeter state changed; calling maybe_snap()", G_STRLOC); private: - void clear() + void clear_snap() { - // clear out old state m_snap_connections.clear(); m_snap.reset(); + } + + void clear() + { + // clear out old state + clear_snap(); m_req = decltype(m_req)(); m_adbd_client.reset(); } diff --git a/src/usb-snap.cpp b/src/usb-snap.cpp index ba964fb..2b5424d 100644 --- a/src/usb-snap.cpp +++ b/src/usb-snap.cpp @@ -119,6 +119,7 @@ private: g_variant_builder_add(&hints_builder, "{sv}", "x-canonical-snap-decisions", g_variant_new_string("true")); g_variant_builder_add(&hints_builder, "{sv}", "x-canonical-private-affirmative-tint", g_variant_new_string("true")); +g_message("%s calling notification notify", G_STRLOC); auto args = g_variant_new("(susssasa{sv}i)", "", uint32_t(0), @@ -153,6 +154,7 @@ private: g_warning("UsbSnap: Error calling Notify: %s", error->message); g_clear_error(&error); } else { +g_message("%s on_notify reply %s", G_STRLOC, g_variant_print(reply, true)); uint32_t id {}; g_variant_get(reply, "(u)", &id); static_cast(gself)->on_notify_reply(id); @@ -162,6 +164,7 @@ private: void on_notify_reply(uint32_t id) { +g_message("%s setting m_notification_id to %d", G_STRLOC, int(id)); m_notification_id = id; } @@ -177,6 +180,7 @@ private: g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Notify::INTERFACE)); auto self = static_cast(gself); +g_message("%s got signal %s with parameters %s", G_STRLOC, signal_name, g_variant_print(parameters, true)); if (!g_strcmp0(signal_name, DBusNames::Notify::ActionInvoked::NAME)) { @@ -212,9 +216,11 @@ private: void on_notification_closed(uint32_t close_reason) { +g_message("%s closed with reason %d", G_STRLOC, int(close_reason)); if (close_reason == DBusNames::Notify::NotificationClosed::Reason::EXPIRED) m_on_user_response(AdbdClient::PKResponse::DENY, false); +g_message("%s setting m_notification_id to 0", G_STRLOC); m_notification_id = 0; } -- cgit v1.2.3 From 020e49163abd558ae095ab9a00b6b69b4fdaef2a Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 27 Apr 2016 10:43:08 +0200 Subject: remove the temporary tracers --- src/greeter.cpp | 6 ------ src/usb-manager.cpp | 7 +------ src/usb-snap.cpp | 7 ------- 3 files changed, 1 insertion(+), 19 deletions(-) (limited to 'src/usb-manager.cpp') diff --git a/src/greeter.cpp b/src/greeter.cpp index ed210b0..962a01d 100644 --- a/src/greeter.cpp +++ b/src/greeter.cpp @@ -50,7 +50,6 @@ private: void set_state(const State& state) { -g_message("%s setting state to %s", G_STRLOC, state_str(state)); m_state.set(state); } @@ -109,7 +108,6 @@ g_message("%s setting state to %s", G_STRLOC, state_str(state)); const char* name_owner, gpointer gself) { -g_message("%s %s appared on bus, owned by %s", G_STRLOC, name, name_owner); auto self = static_cast(gself); self->m_owner = name_owner; @@ -134,7 +132,6 @@ g_message("%s %s appared on bus, owned by %s", G_STRLOC, name, name_owner); const char* name, gpointer gself) { -g_message("%s %s disappeared from bus", G_STRLOC, name); auto self = static_cast(gself); self->m_owner.clear(); @@ -146,7 +143,6 @@ g_message("%s %s disappeared from bus", G_STRLOC, name); GAsyncResult* res, gpointer gself) { -g_message("%s", G_STRLOC); GError* error {}; auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error); if (error != nullptr) { @@ -154,7 +150,6 @@ g_message("%s", G_STRLOC); g_warning("Greeter: Error getting IsActive property: %s", error->message); g_clear_error(&error); } else { -g_message("%s got '%s'", G_STRLOC, g_variant_print(v, true)); GVariant* is_active {}; g_variant_get_child(v, 0, "v", &is_active); static_cast(gself)->set_state(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE); @@ -173,7 +168,6 @@ g_message("%s got '%s'", G_STRLOC, g_variant_print(v, true)); gpointer gself) { auto self = static_cast(gself); -g_message("%s on_properties_changed got %s", G_STRLOC, g_variant_print(parameters, true)); g_return_if_fail(!g_strcmp0(sender_name, self->m_owner.c_str())); g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH)); diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp index ce546fe..cf3330d 100644 --- a/src/usb-manager.cpp +++ b/src/usb-manager.cpp @@ -50,9 +50,7 @@ public: }); m_greeter->state().changed().connect([this](Greeter::State /*state*/) { -g_message("%s clearing old snap, if any", G_STRLOC); clear_snap(); -g_message("%s greeter state changed; calling maybe_snap()", G_STRLOC); maybe_snap(); }); @@ -91,7 +89,7 @@ private: m_adbd_client.reset(new GAdbdClient{m_socket_path}); m_adbd_client->on_pk_request().connect( [this](const AdbdClient::PKRequest& req) { - g_message("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str()); + g_debug("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str()); m_req = req; maybe_snap(); } @@ -100,17 +98,14 @@ private: void maybe_snap() { -g_message("%s m_req.public_key [%s]", G_STRLOC, m_req.public_key.c_str()); // only prompt if there's something to prompt about if (m_req.public_key.empty()) return; -g_message("%s m_greeter->state() %s", G_STRLOC, Greeter::state_str(m_greeter->state().get())); // only prompt in an unlocked session if (m_greeter->state().get() != Greeter::State::INACTIVE) return; -g_message("%s calling snap!", G_STRLOC); snap(); } diff --git a/src/usb-snap.cpp b/src/usb-snap.cpp index 629898b..53db6c4 100644 --- a/src/usb-snap.cpp +++ b/src/usb-snap.cpp @@ -119,7 +119,6 @@ private: g_variant_builder_add(&hints_builder, "{sv}", "x-canonical-snap-decisions", g_variant_new_string("true")); g_variant_builder_add(&hints_builder, "{sv}", "x-canonical-private-affirmative-tint", g_variant_new_string("true")); -g_message("%s calling notification notify", G_STRLOC); auto args = g_variant_new("(susssasa{sv}i)", "", uint32_t(0), @@ -154,7 +153,6 @@ g_message("%s calling notification notify", G_STRLOC); g_warning("UsbSnap: Error calling Notify: %s", error->message); g_clear_error(&error); } else { -g_message("%s on_notify reply %s", G_STRLOC, g_variant_print(reply, true)); uint32_t id {}; g_variant_get(reply, "(u)", &id); static_cast(gself)->on_notify_reply(id); @@ -164,7 +162,6 @@ g_message("%s on_notify reply %s", G_STRLOC, g_variant_print(reply, true)); void on_notify_reply(uint32_t id) { -g_message("%s setting m_notification_id to %d", G_STRLOC, int(id)); m_notification_id = id; } @@ -180,7 +177,6 @@ g_message("%s setting m_notification_id to %d", G_STRLOC, int(id)); g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Notify::INTERFACE)); auto self = static_cast(gself); -g_message("%s got signal %s with parameters %s", G_STRLOC, signal_name, g_variant_print(parameters, true)); if (!g_strcmp0(signal_name, DBusNames::Notify::ActionInvoked::NAME)) { @@ -212,17 +208,14 @@ g_message("%s got signal %s with parameters %s", G_STRLOC, signal_name, g_varian const bool remember_this_choice = response == AdbdClient::PKResponse::ALLOW; m_on_user_response(response, remember_this_choice); -g_message("%s clearing m_notification_id", G_STRLOC); m_notification_id = 0; } void on_notification_closed(uint32_t close_reason) { -g_message("%s closed with reason %d", G_STRLOC, int(close_reason)); if (close_reason == DBusNames::Notify::NotificationClosed::Reason::EXPIRED) m_on_user_response(AdbdClient::PKResponse::DENY, false); -g_message("%s clearing m_notification_id", G_STRLOC); m_notification_id = 0; } -- cgit v1.2.3 From d7a0cac8ebd836172ad6e4b9298ae92b3f9d8de9 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 29 Apr 2016 11:51:22 +0200 Subject: don't clear the snap decision when the greeter changes state --- src/usb-manager.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'src/usb-manager.cpp') diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp index cf3330d..9c45a56 100644 --- a/src/usb-manager.cpp +++ b/src/usb-manager.cpp @@ -50,7 +50,6 @@ public: }); m_greeter->state().changed().connect([this](Greeter::State /*state*/) { - clear_snap(); maybe_snap(); }); -- cgit v1.2.3 From c66c33c4c1c70b197941f8c486196f3a56abd0e3 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 3 May 2016 18:07:48 -0500 Subject: always send a response to adb even in edge cases; e.g. send an auto-NO if the usb connection is broken while the prompt is up --- src/usb-manager.cpp | 92 +++++++++++++++++++++++++++-------------------------- 1 file changed, 47 insertions(+), 45 deletions(-) (limited to 'src/usb-manager.cpp') diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp index 9c45a56..65617c5 100644 --- a/src/usb-manager.cpp +++ b/src/usb-manager.cpp @@ -46,59 +46,54 @@ public: m_greeter{greeter} { m_usb_monitor->on_usb_disconnected().connect([this](const std::string& /*usb_name*/) { - restart(); + m_req.reset(); }); - m_greeter->state().changed().connect([this](Greeter::State /*state*/) { - maybe_snap(); + m_greeter->state().changed().connect([this](const Greeter::State& state) { + if (state == Greeter::State::INACTIVE) + maybe_snap(); + else + stop_snap(); }); - restart(); + // 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() { - if (m_restart_idle_tag) - g_source_remove(m_restart_idle_tag); - - clear(); + if (m_request_complete_idle_tag) + g_source_remove(m_request_complete_idle_tag); } private: - void clear_snap() + void stop_snap() { m_snap_connections.clear(); m_snap.reset(); } - void clear() - { - // clear out old state - clear_snap(); - m_req = decltype(m_req)(); - m_adbd_client.reset(); - } - - void restart() - { - clear(); - - // set a new 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_req = req; - maybe_snap(); - } - ); - } - void maybe_snap() { // only prompt if there's something to prompt about - if (m_req.public_key.empty()) + if (!m_req) return; // only prompt in an unlocked session @@ -110,19 +105,25 @@ private: void snap() { - m_snap = std::make_shared(m_req.fingerprint); + m_snap = std::make_shared(m_req->fingerprint); m_snap_connections.insert((*m_snap).on_user_response().connect( [this](AdbdClient::PKResponse response, bool remember_choice){ - g_debug("%s user responded! response %d, remember %d", G_STRLOC, int(response), int(remember_choice)); - m_req.respond(response); + if (remember_choice && (response == AdbdClient::PKResponse::ALLOW)) - write_public_key(m_req.public_key); - m_restart_idle_tag = g_idle_add([](gpointer gself){ - auto self = static_cast(gself); - self->m_restart_idle_tag = 0; - self->restart(); - 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(gself); + self->m_request_complete_idle_tag = 0; + self->m_req.reset(); + return G_SOURCE_REMOVE; + }, this); + } } )); } @@ -163,12 +164,13 @@ private: const std::shared_ptr m_usb_monitor; const std::shared_ptr m_greeter; - unsigned int m_restart_idle_tag {}; + unsigned int m_request_complete_idle_tag {}; std::shared_ptr m_adbd_client; - AdbdClient::PKRequest m_req; std::shared_ptr m_snap; std::set m_snap_connections; + AdbdClient::PKResponse m_response {AdbdClient::PKResponse::DENY}; + std::shared_ptr m_req; }; /*** -- cgit v1.2.3