Merge lp:~thomas-voss/location-service/fix-match-rule-leak into lp:location-service/15.04

[Thomas Voß]

Ensure that event connections are cleaned up on destruction.

Ensure that event connections are cleaned up on destruction.

[lp:~thomas-voss/location-service/fix-match-rule-leak]

199. By Thomas Voß on 2015-11-12

Instrument on_access_point_{added, removed} with log messages.

200. By Thomas Voß on 2015-11-12

One connection is more than enough.

201. By Thomas Voß on 2015-11-12

Don't add, just use an existing object.

202. By Thomas Voß on 2015-11-12

Log destruction of CachedWirelessNetwork instances.

203. By Thomas Voß on 2015-11-17

Remove temporary log output.

204. By Thomas Voß on 2015-11-17

Revert accidental change to debian/source/format.

[Alberto Mardegan (mardy)]

The last chunk of changes looks unrelated and it's not obvious whether what it's solving. But if that's not an accidental change, it all looks good to me.

review: Approve

[lp:~thomas-voss/location-service/fix-match-rule-leak]

205. By Thomas Voß on 2015-12-02

[ Alberto Mardegan ]
* Send last known position on session start
[ CI Train Bot ]
* New rebuild forced.
[ Thomas Voß ]
* Factor out service::Runtime from daemon.cpp into its own .h/.cpp
  pair of files. Add test cases around correct operation of
  service::Runtime. added:
  src/location_service/com/ubuntu/location/service/runtime.cpp
  src/location_service/com/ubuntu/location/service/runtime.h
  tests/runtime_test.cpp
[ thomas-voss ]
* Factor out service::Runtime from daemon.cpp into its own .h/.cpp
  pair of files. Add test cases around correct operation of
  service::Runtime. added:
  src/location_service/com/ubuntu/location/service/runtime.cpp
  src/location_service/com/ubuntu/location/service/runtime.h
  tests/runtime_test.cpp
* Adjust default timeout for downloading GPS XTRA data.
[ Alberto Mardegan ]
* Make sure that injected time is given in milliseconds
[ Thomas Voß ]
* Cherry-pick rev. 196 and 199 from lp:location-service. The changes
  got accidentally removed by merging the outstanding documentation
  branch.
* Handle responses of clients to updates asynchronously. Rely on
  dummy::ConnectivityManager as harvesting is disabled anyway. (LP:
  #1462664, #1387643)

206. By Thomas Voß on 2015-12-02

Revert accidental changes.

=== modified file 'src/location_service/com/ubuntu/location/connectivity/cached_wireless_network.cpp'
--- src/location_service/com/ubuntu/location/connectivity/cached_wireless_network.cpp	2015-05-28 10:57:24 +0000
+++ src/location_service/com/ubuntu/location/connectivity/cached_wireless_network.cpp	2015-12-02 12:11:29 +0000
@@ -123,7 +123,14 @@
         const org::freedesktop::NetworkManager::Device& device,
         const org::freedesktop::NetworkManager::AccessPoint& ap)
     : device_(device),
-      access_point_(ap)
+      access_point_(ap),
+      connections
+      {
+          access_point_.properties_changed->connect([this](const std::map<std::string, core::dbus::types::Variant>& dict)
+          {
+              on_access_point_properties_changed(dict);
+          })
+      }
 {
     last_seen_ = translate_time_stamp(access_point_.last_seen->get());
 
@@ -138,12 +145,11 @@
     {
         static_cast<int>(access_point_.strength->get())
     };
+}
 
-    // Wire up all the connections
-    access_point_.properties_changed->connect([this](const std::map<std::string, core::dbus::types::Variant>& dict)
-    {
-        on_access_point_properties_changed(dict);
-    });
+detail::CachedWirelessNetwork::~CachedWirelessNetwork()
+{
+    access_point_.properties_changed->disconnect(connections.ap_properties_changed);
 }
 
 void detail::CachedWirelessNetwork::on_access_point_properties_changed(const std::map<std::string, core::dbus::types::Variant>& dict)

=== modified file 'src/location_service/com/ubuntu/location/connectivity/cached_wireless_network.h'
--- src/location_service/com/ubuntu/location/connectivity/cached_wireless_network.h	2014-08-14 20:25:22 +0000
+++ src/location_service/com/ubuntu/location/connectivity/cached_wireless_network.h	2015-12-02 12:11:29 +0000
@@ -41,6 +41,8 @@
             const org::freedesktop::NetworkManager::Device& device,
             const org::freedesktop::NetworkManager::AccessPoint& ap);
 
+    ~CachedWirelessNetwork();
+
     // Timestamp when the network became visible.
     const core::Property<std::chrono::system_clock::time_point>& last_seen() const override;
 
@@ -67,6 +69,16 @@
     // The actual access point stub.
     org::freedesktop::NetworkManager::AccessPoint access_point_;
 
+    // Encapsulates all event connections that have to be cut on destruction.
+    struct
+    {
+        core::dbus::Signal
+        <
+            org::freedesktop::NetworkManager::AccessPoint::PropertiesChanged,
+            org::freedesktop::NetworkManager::AccessPoint::PropertiesChanged::ArgumentType
+        >::SubscriptionToken ap_properties_changed;
+    } connections;
+
     core::Property<std::chrono::system_clock::time_point> last_seen_;
     core::Property<std::string> bssid_;
     core::Property<std::string> ssid_;

=== modified file 'src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.cpp'
--- src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.cpp	2015-05-27 18:40:37 +0000
+++ src/location_service/com/ubuntu/location/connectivity/ofono_nm_connectivity_manager.cpp	2015-12-02 12:11:29 +0000
@@ -595,7 +595,7 @@
 
     xdg::NetworkManager::AccessPoint ap
     {
-        network_manager->service->add_object_for_path(ap_path)
+        network_manager->service->object_for_path(ap_path)
     };
 
     auto wifi = std::make_shared<detail::CachedWirelessNetwork>(itd->second, ap);

=== modified file 'src/location_service/com/ubuntu/location/service/provider_daemon.cpp'
--- src/location_service/com/ubuntu/location/service/provider_daemon.cpp	2015-11-25 09:20:12 +0000
+++ src/location_service/com/ubuntu/location/service/provider_daemon.cpp	2015-12-02 12:11:29 +0000
@@ -108,6 +108,14 @@
     return result;
 }
 
+namespace
+{
+std::shared_ptr<location::service::Runtime> runtime()
+{
+    static const auto inst = location::service::Runtime::create(1);
+    return inst;
+}
+}
 int location::service::ProviderDaemon::main(const location::service::ProviderDaemon::Configuration& config)
 {
     auto trap = core::posix::trap_signals_for_all_subsequent_threads(
@@ -121,9 +129,7 @@
         trap->stop();
     });
 
-    auto runtime = location::service::Runtime::create(1);
-
-    config.connection->install_executor(core::dbus::asio::make_executor(config.connection, runtime->service()));
+    config.connection->install_executor(core::dbus::asio::make_executor(config.connection, runtime()->service()));
 
     auto skeleton = location::providers::remote::skeleton::create_with_configuration(location::providers::remote::skeleton::Configuration
     {
@@ -132,7 +138,7 @@
         config.provider
     });
 
-    runtime->start();
+    runtime()->start();
 
     trap->run();