Refactor weaved to use Avahi client library

am: 494b72473a

* commit '494b72473a1c436751003c249e1e4a9f81102329':
  Refactor weaved to use Avahi client library

6 files changed, 83 insertions, 356 deletions

diff --git a/Android.mk b/Android.mk
index 7ebdf2b..7f8ac06 100644
--- a/Android.mk
+++ b/Android.mk
@@ -37,6 +37,7 @@ buffetCommonCIncludes := \
 buffetSharedLibraries := \
 	libapmanager-client \
 	libavahi-common \
+	libavahi-client \
 	libbrillo \
 	libbrillo-dbus \
 	libbrillo-http \
diff --git a/buffet/avahi_mdns_client.cc b/buffet/avahi_mdns_client.cc
index 6bf8c5d..852f59a 100644
--- a/buffet/avahi_mdns_client.cc
+++ b/buffet/avahi_mdns_client.cc
@@ -17,349 +17,112 @@
 #include <vector>
 
 #include "buffet/avahi_mdns_client.h"
-#include "buffet/dbus_constants.h"
 
-#include <avahi-common/defs.h>
 #include <avahi-common/address.h>
+#include <avahi-common/defs.h>
+#include <avahi-common/error.h>
+
 #include <base/guid.h>
-#include <brillo/dbus/async_event_sequencer.h>
-#include <brillo/dbus/dbus_signal_handler.h>
-#include <brillo/dbus/dbus_method_invoker.h>
 #include <brillo/errors/error.h>
-#include <dbus/object_path.h>
-#include <dbus/object_proxy.h>
 
 using brillo::ErrorPtr;
-using brillo::dbus_utils::AsyncEventSequencer;
-using brillo::dbus_utils::CallMethodAndBlock;
-using brillo::dbus_utils::ExtractMethodCallResults;
-using CompletionAction =
-    brillo::dbus_utils::AsyncEventSequencer::CompletionAction;
 
 namespace buffet {
 
-AvahiMdnsClient::AvahiMdnsClient(const scoped_refptr<dbus::Bus> &bus)
-    : bus_(bus), service_name_(base::GenerateGUID()) {
-}
+std::unique_ptr<MdnsClient> MdnsClient::CreateInstance() {
+  return std::unique_ptr<MdnsClient>{new AvahiMdnsClient()};
 
-AvahiMdnsClient::~AvahiMdnsClient() {
 }
 
-// NB: This should be the one-and-only definition of this MdnsClient static
-// method.
-std::unique_ptr<MdnsClient> MdnsClient::CreateInstance(
-    const scoped_refptr<dbus::Bus> &bus) {
-  return std::unique_ptr<MdnsClient>{new AvahiMdnsClient(bus)};
-}
+namespace {
 
-// TODO(rginda): Report errors back to the caller.
-// TODO(rginda): Support publishing more than one service.
-void AvahiMdnsClient::PublishService(
-    const std::string& service_type, uint16_t port,
-    const std::vector<std::string>& txt) {
-
-  CHECK_EQ("_privet._tcp", service_type);
-
-  if (service_state_ == READY) {
-    if (service_type_ != service_type || port_ != port) {
-      // If the type or port of a service changes we have to re-publish
-      // rather than just update the txt record.
-      StopPublishing(service_type_);
-      if (service_state_ != UNDEF) {
-        LOG(ERROR) << "Failed to disable existing service.";
-        return;
-      }
-    }
-  }
-
-  service_type_ = service_type;
-  port_ = port;
-  txt_ = GetTxtRecord(txt);
-
-  if (avahi_state_ == UNDEF || avahi_state_ == ERROR) {
-    ConnectToAvahi();
-  } else if (service_state_ == READY) {
-    UpdateServiceTxt();
-  } else if (avahi_state_ == READY) {
-    CreateEntryGroup();
-  } else {
-    CHECK(avahi_state_ == PENDING);
+void HandleGroupStateChanged(AvahiEntryGroup* g,
+                             AvahiEntryGroupState state,
+                             AVAHI_GCC_UNUSED void* userdata) {
+  if (state == AVAHI_ENTRY_GROUP_COLLISION ||
+      state == AVAHI_ENTRY_GROUP_FAILURE) {
+    LOG(ERROR) << "Avahi service group error: " << state;
   }
 }
 
-// TODO(rginda): If we support publishing more than one service then we
-// may need a less ambiguous way of unpublishing them.
-void AvahiMdnsClient::StopPublishing(const std::string& service_type) {
-  if (service_type_ != service_type) {
-    LOG(ERROR) << "Unknown service type: " << service_type;
-    return;
-  }
+}  // namespace
 
-  if (service_state_ != READY) {
-    LOG(ERROR) << "Service is not published.";
-  }
+AvahiMdnsClient::AvahiMdnsClient()
+    : service_name_(base::GenerateGUID()) {
+  thread_pool_.reset(avahi_threaded_poll_new());
+  CHECK(thread_pool_);
 
-  service_type_.clear();
-  port_ = 0;
+  int ret = 0;
 
-  FreeEntryGroup();
-}
+  client_.reset(avahi_client_new(avahi_threaded_poll_get(thread_pool_.get()),
+                                 {}, nullptr, this, &ret));
+  CHECK(client_) << avahi_strerror(ret);
 
-// Transform a service_info to a mDNS compatible TXT record value.
-// Concretely, a TXT record consists of a list of strings in the format
-// "key=value".  Each string must be less than 256 bytes long, since they are
-// length/value encoded.  Keys may not contain '=' characters, but are
-// otherwise unconstrained.
-//
-// We need a DBus type of "aay", which is a vector<vector<uint8_t>> in our
-// bindings.
-AvahiMdnsClient::TxtRecord AvahiMdnsClient::GetTxtRecord(
-    const std::vector<std::string>& txt) {
-  TxtRecord result;
-  result.reserve(txt.size());
-  for (const std::string& s : txt) {
-    result.emplace_back();
-    std::vector<uint8_t>& record = result.back();
-    record.insert(record.end(), s.begin(), s.end());
-  }
-  return result;
-}
-
-void AvahiMdnsClient::ConnectToAvahi() {
-  avahi_state_ = PENDING;
-
-  avahi_ = bus_->GetObjectProxy(
-      dbus_constants::avahi::kServiceName,
-      dbus::ObjectPath(dbus_constants::avahi::kServerPath));
-
-  // This callback lives for the lifetime of the ObjectProxy.
-  avahi_->SetNameOwnerChangedCallback(
-      base::Bind(&AvahiMdnsClient::OnAvahiOwnerChanged,
-                 weak_ptr_factory_.GetWeakPtr()));
+  avahi_threaded_poll_start(thread_pool_.get());
 
-  // Reconnect to our signals on a new Avahi instance.
-  scoped_refptr<AsyncEventSequencer> sequencer(new AsyncEventSequencer());
-  brillo::dbus_utils::ConnectToSignal(
-      avahi_,
-      dbus_constants::avahi::kServerInterface,
-      dbus_constants::avahi::kServerSignalStateChanged,
-      base::Bind(&AvahiMdnsClient::OnAvahiStateChanged,
-                 weak_ptr_factory_.GetWeakPtr()),
-      sequencer->GetExportHandler(
-          dbus_constants::avahi::kServerInterface,
-          dbus_constants::avahi::kServerSignalStateChanged,
-          "Failed to subscribe to Avahi state change.",
-          true));
-  sequencer->OnAllTasksCompletedCall(
-      {// Get a onetime callback with the initial state of Avahi.
-       AsyncEventSequencer::WrapCompletionTask(
-            base::Bind(&dbus::ObjectProxy::WaitForServiceToBeAvailable,
-                       avahi_,
-                       base::Bind(&AvahiMdnsClient::OnAvahiAvailable,
-                                  weak_ptr_factory_.GetWeakPtr()))),
-      });
+  group_.reset(avahi_entry_group_new(client_.get(), HandleGroupStateChanged,
+                                     nullptr));
+  CHECK(group_) << avahi_strerror(avahi_client_errno(client_.get()))
+                << ". Check avahi-daemon configuration";
 }
 
-void AvahiMdnsClient::CreateEntryGroup() {
-  ErrorPtr error;
-
-  service_state_ = PENDING;
-
-  auto resp = CallMethodAndBlock(
-      avahi_, dbus_constants::avahi::kServerInterface,
-      dbus_constants::avahi::kServerMethodEntryGroupNew,
-      &error);
-
-  dbus::ObjectPath group_path;
-  if (!resp || !ExtractMethodCallResults(resp.get(), &error, &group_path)) {
-    service_state_ = ERROR;
-    LOG(ERROR) << "Error creating group.";
-    return;
-  }
-  entry_group_ = bus_->GetObjectProxy(dbus_constants::avahi::kServiceName,
-                                      group_path);
-
-  // If we fail to connect to the StateChange signal for this group, just
-  // report that the whole thing has failed.
-  auto on_connect_cb = [](const std::string& interface_name,
-                          const std::string& signal_name,
-                          bool success) {
-    if (!success) {
-      LOG(ERROR) << "Failed to connect to StateChange signal "
-        "from EntryGroup.";
-      return;
-    }
-  };
-
-  brillo::dbus_utils::ConnectToSignal(
-      entry_group_,
-      dbus_constants::avahi::kGroupInterface,
-      dbus_constants::avahi::kGroupSignalStateChanged,
-      base::Bind(&AvahiMdnsClient::HandleGroupStateChanged,
-                 weak_ptr_factory_.GetWeakPtr()),
-      base::Bind(on_connect_cb));
-
-  CreateService();
+AvahiMdnsClient::~AvahiMdnsClient() {
+  if (thread_pool_)
+    avahi_threaded_poll_stop(thread_pool_.get());
 }
 
-void AvahiMdnsClient::FreeEntryGroup() {
-  if (!entry_group_) {
-    LOG(ERROR) << "No group to free.";
-    return;
-  }
-
-  ErrorPtr error;
-  auto resp = CallMethodAndBlock(entry_group_,
-                                 dbus_constants::avahi::kGroupInterface,
-                                 dbus_constants::avahi::kGroupMethodFree,
-                                 &error);
-  // Extract and log relevant errors.
-  bool success = resp && ExtractMethodCallResults(resp.get(), &error);
-  if (!success) {
-    LOG(ERROR) << "Error freeing service group";
-  }
+// TODO(rginda): Report errors back to the caller.
+// TODO(rginda): Support publishing more than one service.
+void AvahiMdnsClient::PublishService(const std::string& service_type,
+                                     uint16_t port,
+                                     const std::vector<std::string>& txt) {
+  CHECK(group_);
+  CHECK_EQ("_privet._tcp", service_type);
 
-  // Ignore any signals we may have registered for from this proxy.
-  entry_group_->Detach();
-  entry_group_ = nullptr;
+  // Create txt record.
+  std::unique_ptr<AvahiStringList, decltype(&avahi_string_list_free)> txt_list{
+      nullptr, &avahi_string_list_free};
 
-  service_state_ = UNDEF;
-}
+  if (!txt.empty()) {
+    std::vector<const char*> txt_vector_ptr;
 
-void AvahiMdnsClient::CreateService() {
-  ErrorPtr error;
+    for (const auto& i : txt)
+      txt_vector_ptr.push_back(i.c_str());
 
-  VLOG(1) << "CreateService: name: " << service_name_ << ", type: " <<
-      service_type_ << ", port: " << port_;
-  auto resp = CallMethodAndBlock(
-      entry_group_,
-      dbus_constants::avahi::kGroupInterface,
-      dbus_constants::avahi::kGroupMethodAddService,
-      &error,
-      int32_t{AVAHI_IF_UNSPEC},
-      int32_t{AVAHI_PROTO_UNSPEC},
-      uint32_t{0},  // No flags.
-      service_name_,
-      std::string{"_privet._tcp"},
-      std::string{},  // domain.
-      std::string{},  // hostname
-      port_,
-      txt_);
-  if (!resp || !ExtractMethodCallResults(resp.get(), &error)) {
-    LOG(ERROR) << "Error creating service";
-    service_state_ = ERROR;
-    return;
+    txt_list.reset(avahi_string_list_new_from_array(txt_vector_ptr.data(),
+                                                    txt_vector_ptr.size()));
+    CHECK(txt_list);
   }
 
-  resp = CallMethodAndBlock(entry_group_,
-                            dbus_constants::avahi::kGroupInterface,
-                            dbus_constants::avahi::kGroupMethodCommit,
-                            &error);
-  if (!resp || !ExtractMethodCallResults(resp.get(), &error)) {
-    LOG(ERROR) << "Error committing service.";
-    service_state_ = ERROR;
-    return;
-  }
-
-  service_state_ = READY;
-}
-
-void AvahiMdnsClient::UpdateServiceTxt() {
-  ErrorPtr error;
+  int ret = 0;
 
-  CHECK_EQ(READY, service_state_);
+  if (prev_port_ == port && prev_type_ == service_type) {
+    ret = avahi_entry_group_update_service_txt_strlst(
+        group_.get(), AVAHI_IF_UNSPEC, AVAHI_PROTO_UNSPEC, {},
+        service_name_.c_str(), service_type.c_str(), nullptr, txt_list.get());
 
-  VLOG(1) << "UpdateServiceTxt: name " << service_name_ << ", type: " <<
-      service_type_ << ", port: " << port_;
-  auto resp = CallMethodAndBlock(
-      entry_group_,
-      dbus_constants::avahi::kGroupInterface,
-      dbus_constants::avahi::kGroupMethodUpdateServiceTxt,
-      &error,
-      int32_t{AVAHI_IF_UNSPEC},
-      int32_t{AVAHI_PROTO_UNSPEC},
-      uint32_t{0},  // No flags.
-      service_name_,
-      std::string{"_privet._tcp"},
-      std::string{},  // domain.
-      txt_);
-  if (!resp || !ExtractMethodCallResults(resp.get(), &error)) {
-    LOG(ERROR) << "Error creating service";
-    service_state_ = ERROR;
-    return;
-  }
-};
+    CHECK_GE(ret, 0) << avahi_strerror(ret);
+  } else {
+    prev_port_ = port;
+    prev_type_ = service_type;
 
-void AvahiMdnsClient::OnAvahiOwnerChanged(const std::string& old_owner,
-                                          const std::string& new_owner) {
-  if (new_owner.empty()) {
-    OnAvahiAvailable(false);
-    return;
-  }
-  OnAvahiAvailable(true);
-}
+    avahi_entry_group_reset(group_.get());
+    CHECK(avahi_entry_group_is_empty(group_.get()));
 
-void AvahiMdnsClient::OnAvahiStateChanged(int32_t state,
-                                          const std::string& error) {
-  HandleAvahiStateChange(state);
-}
+    ret = avahi_entry_group_add_service_strlst(
+        group_.get(), AVAHI_IF_UNSPEC, AVAHI_PROTO_UNSPEC, {},
+        service_name_.c_str(), service_type.c_str(), nullptr, nullptr, port,
+        txt_list.get());
+    CHECK_GE(ret, 0) << avahi_strerror(ret);
 
-void AvahiMdnsClient::OnAvahiAvailable(bool avahi_is_on_dbus) {
-  VLOG(1) << "Avahi is "  << (avahi_is_on_dbus ? "up." : "down.");
-  int32_t state = AVAHI_SERVER_FAILURE;
-  if (avahi_is_on_dbus) {
-    auto resp = CallMethodAndBlock(
-        avahi_, dbus_constants::avahi::kServerInterface,
-        dbus_constants::avahi::kServerMethodGetState,
-        nullptr);
-    if (!resp || !ExtractMethodCallResults(resp.get(), nullptr, &state)) {
-      LOG(WARNING) << "Failed to get avahi initial state.  Relying on signal.";
-    }
+    ret = avahi_entry_group_commit(group_.get());
+    CHECK_GE(ret, 0) << avahi_strerror(ret);
   }
-  VLOG(1) << "Initial Avahi state=" << state << ".";
-  HandleAvahiStateChange(state);
 }
 
-void AvahiMdnsClient::HandleAvahiStateChange(int32_t state) {
-  switch (state) {
-    case AVAHI_SERVER_RUNNING: {
-      // All host RRs have been established.
-      VLOG(1) << "Avahi ready for action.";
-      if (avahi_state_ == READY) {
-        LOG(INFO) << "Ignoring redundant Avahi up event.";
-        return;
-      }
-      avahi_state_ = READY;
-      CreateEntryGroup();
-    } break;
-    case AVAHI_SERVER_INVALID:
-      // Invalid state (initial).
-    case AVAHI_SERVER_REGISTERING:
-      // Host RRs are being registered.
-    case AVAHI_SERVER_COLLISION:
-      // There is a collision with a host RR. All host RRs have been withdrawn,
-      // the user should set a new host name via avahi_server_set_host_name().
-    case AVAHI_SERVER_FAILURE:
-      // Some fatal failure happened, the server is unable to proceed.
-      avahi_state_ = ERROR;
-      if (service_state_ != UNDEF)
-        service_state_ = ERROR;
-
-      LOG(ERROR) << "Avahi changed to error state: " << state;
-      break;
-    default:
-      LOG(ERROR) << "Unknown Avahi server state change to " << state;
-      break;
-  }
-}
-
-void AvahiMdnsClient::HandleGroupStateChanged(
-    int32_t state,
-    const std::string& error_message) {
-  if (state == AVAHI_ENTRY_GROUP_COLLISION ||
-      state == AVAHI_ENTRY_GROUP_FAILURE) {
-    LOG(ERROR) << "Avahi service group error: " << state;
-  }
+void AvahiMdnsClient::StopPublishing(const std::string& service_type) {
+  CHECK(group_);
+  avahi_entry_group_reset(group_.get());
 }
 
 }  // namespace buffet
diff --git a/buffet/avahi_mdns_client.h b/buffet/avahi_mdns_client.h
index e689cac..405180d 100644
--- a/buffet/avahi_mdns_client.h
+++ b/buffet/avahi_mdns_client.h
@@ -20,8 +20,9 @@
 #include <map>
 #include <string>
 
-#include <base/memory/weak_ptr.h>
-#include <dbus/bus.h>
+#include <avahi-client/client.h>
+#include <avahi-client/publish.h>
+#include <avahi-common/thread-watch.h>
 
 #include "buffet/mdns_client.h"
 
@@ -30,7 +31,7 @@ namespace buffet {
 // Publishes privet service on mDns using Avahi.
 class AvahiMdnsClient : public MdnsClient {
  public:
-  explicit AvahiMdnsClient(const scoped_refptr<dbus::Bus>& bus);
+  explicit AvahiMdnsClient();
   ~AvahiMdnsClient() override;
 
   // weave::provider::DnsServiceDiscovery implementation.
@@ -39,52 +40,16 @@ class AvahiMdnsClient : public MdnsClient {
   void StopPublishing(const std::string& service_type) override;
 
  private:
-  using TxtRecord = std::vector<std::vector<uint8_t>>;
-
-  // States used to track progress of our asynchronous dbus operations.
-  enum AsyncState {
-    UNDEF,
-    PENDING,
-    READY,
-    ERROR
-  };
-
-  scoped_refptr<dbus::Bus> bus_;
-  dbus::ObjectProxy* avahi_{nullptr};
-  // The avahi interface we use to add/remove mdns services.
-  dbus::ObjectProxy* entry_group_{nullptr};
-
-  // State of our dbus connection to avahi.
-  AsyncState avahi_state_{UNDEF};
-  // State of the group/service publish operation.
-  AsyncState service_state_{UNDEF};
-
+  uint16_t prev_port_{0};
+  std::string prev_type_;
   std::string service_name_;
-  std::string service_type_;
-  uint16_t port_{0};
-  TxtRecord txt_;
-
-  // Must be last member to invalidate pointers before actual destruction.
-  base::WeakPtrFactory<AvahiMdnsClient> weak_ptr_factory_{this};
-
-  // Convert a {string:string} text record into something we can send over
-  // dbus.
-  static TxtRecord GetTxtRecord(const std::vector<std::string>& txt);
-
-  void ConnectToAvahi();
-  void CreateEntryGroup();
-  void FreeEntryGroup();
-  void CreateService();
-  void UpdateServiceTxt();
+  std::unique_ptr<AvahiThreadedPoll, decltype(&avahi_threaded_poll_free)>
+      thread_pool_{nullptr, &avahi_threaded_poll_free};
+  std::unique_ptr< ::AvahiClient, decltype(&avahi_client_free)> client_{
+      nullptr, &avahi_client_free};
+  std::unique_ptr<AvahiEntryGroup, decltype(&avahi_entry_group_free)> group_{
+      nullptr, &avahi_entry_group_free};
 
-  void OnAvahiOwnerChanged(const std::string& old_owner,
-                           const std::string& new_owner);
-  void OnAvahiStateChanged(int32_t state,
-                           const std::string& error);
-  void OnAvahiAvailable(bool avahi_is_on_dbus);
-  void HandleAvahiStateChange(int32_t state);
-  void HandleGroupStateChanged(int32_t state,
-                               const std::string& error_message);
   DISALLOW_COPY_AND_ASSIGN(AvahiMdnsClient);
 };
 
diff --git a/buffet/manager.cc b/buffet/manager.cc
index 4cfa902..2e53c62 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -193,7 +193,7 @@ void Manager::RestartWeave(AsyncEventSequencer* sequencer) {
   weave::provider::HttpServer* http_server{nullptr};
 #ifdef BUFFET_USE_WIFI_BOOTSTRAPPING
   if (!options_.disable_privet) {
-    mdns_client_ = MdnsClient::CreateInstance(dbus_object_.GetBus());
+    mdns_client_ = MdnsClient::CreateInstance();
     web_serv_client_.reset(new WebServClient{
         dbus_object_.GetBus(), sequencer,
         base::Bind(&Manager::CreateDevice, weak_ptr_factory_.GetWeakPtr())});
diff --git a/buffet/mdns_client.h b/buffet/mdns_client.h
index f9bc229..6a04882 100644
--- a/buffet/mdns_client.h
+++ b/buffet/mdns_client.h
@@ -39,8 +39,7 @@ class MdnsClient : public weave::provider::DnsServiceDiscovery {
                       const std::vector<std::string>& txt) override {}
   void StopPublishing(const std::string& service_type) override {}
 
-  static std::unique_ptr<MdnsClient> CreateInstance(
-      const scoped_refptr<dbus::Bus>& bus);
+  static std::unique_ptr<MdnsClient> CreateInstance();
 
  protected:
   DISALLOW_COPY_AND_ASSIGN(MdnsClient);
diff --git a/buffet/stub_mdns_client.cc b/buffet/stub_mdns_client.cc
index 860f016..e535014 100644
--- a/buffet/stub_mdns_client.cc
+++ b/buffet/stub_mdns_client.cc
@@ -18,8 +18,7 @@
 
 namespace buffet {
 
-std::unique_ptr<MdnsClient> MdnsClient::CreateInstance(
-    const scoped_refptr<dbus::Bus> &bus) {
+std::unique_ptr<MdnsClient> MdnsClient::CreateInstance() {
   return std::unique_ptr<MdnsClient>{new MdnsClient};
 }