Properly handle disconnected server from libwebservandroid-n-preview-1

The DBus back end implemented fault tolerance, so that libwebserv could
ignore the disappearance of the server. We didn't carry this forward.

This actually breaks if a user tries to connect, grab protocol handlers,
and register all in one go, as when we connect, we don't actually
establish the connection until the next main loop iteration.

Change-Id: Ia8b7efb5f6b024eb3668759a53a03061c6dc136c
Test: Sanity test now successfully registers request handlers.
Bug: 27204884

2 files changed, 80 insertions, 51 deletions

diff --git a/libwebserv/binder_request_handler.h b/libwebserv/binder_request_handler.h
index b3f38e6..b42ddd2 100644
--- a/libwebserv/binder_request_handler.h
+++ b/libwebserv/binder_request_handler.h
@@ -23,14 +23,14 @@ namespace libwebserv {
 class BinderRequestHandler : public android::webservd::BnRequestHandler {
  public:
   explicit BinderRequestHandler(
-      std::unique_ptr<libwebserv::RequestHandlerInterface> handler)
-      : handler_(std::move(handler)) {}
+      libwebserv::RequestHandlerInterface* handler)
+      : handler_(handler) {}
 
   android::binder::Status ProcessRequest(
       const android::sp<android::webservd::IHttpRequest>& request) override;
 
  private:
-  std::unique_ptr<libwebserv::RequestHandlerInterface> handler_;
+  libwebserv::RequestHandlerInterface* handler_;
 
   DISALLOW_COPY_AND_ASSIGN(BinderRequestHandler);
 };
diff --git a/libwebserv/binder_server.cc b/libwebserv/binder_server.cc
index c7a7491..e4abd7f 100644
--- a/libwebserv/binder_server.cc
+++ b/libwebserv/binder_server.cc
@@ -38,6 +38,58 @@ using std::vector;
 
 namespace libwebserv {
 
+class RequestHandlerRegistration {
+ public:
+  RequestHandlerRegistration(const string& url,
+                             const string& method,
+                             unique_ptr<RequestHandlerInterface> handler)
+      : url_(url),
+        method_(method),
+        handler_(std::move(handler)),
+        binder_handler_(new BinderRequestHandler(handler_.get())) {}
+
+  ~RequestHandlerRegistration() { DropRemotes(); }
+
+  void AddRemote(sp<IProtocolHandler> handler) {
+    string guid;
+
+    if (!handler->AddRequestHandler(url_,
+                                    method_,
+                                    binder_handler_,
+                                    &guid).isOk()) {
+      LOG(WARNING) << "Remote protocol handler failed to "
+                   << "register new request handler.";
+    }
+
+    protos_.emplace_back(handler, guid);
+  }
+
+  void DropRemotes() {
+    for (auto& proto : protos_) {
+      if (!proto.first->RemoveRequestHandler(proto.second).isOk()) {
+        LOG(WARNING) << "Could not remove request handler from remote.";
+      }
+    }
+  }
+
+ private:
+  string url_;
+  string method_;
+  unique_ptr<RequestHandlerInterface> handler_;
+  sp<IRequestHandler> binder_handler_;
+  vector<pair<sp<IProtocolHandler>, string>> protos_;
+
+  DISALLOW_COPY_AND_ASSIGN(RequestHandlerRegistration);
+};
+
+// This is libwebserv's binder based process local protocol
+// handler.  It wraps around a group of remote protocol handlers
+// that share a common name.  It exists independently of the
+// lifetime of the remote protocol handlers and is created on
+// either of:
+//   1) The local process requests a protocol handler by name
+//   2) We connect to the remote webservd process and it has a protocol
+//      handler with the given name.
 class BinderProtocolHandler : public ProtocolHandler {
  public:
   explicit BinderProtocolHandler(const string& name)
@@ -73,34 +125,27 @@ class BinderProtocolHandler : public ProtocolHandler {
     protocols_.insert(protocol);
     remote_handlers_.push_back(handler);
 
+    for (auto& request_handler : request_handler_ids_) {
+      request_handler.second->AddRemote(handler);
+    }
+
     return true;
   }
 
   int AddHandler(const string& url,
                  const string& method,
                  unique_ptr<RequestHandlerInterface> handler) override {
-    sp<IRequestHandler> binder_handler(
-      new BinderRequestHandler(std::move(handler)));
-    set<pair<sp<IProtocolHandler>, string>> guids;
-
-    // Add this local request handler to each remote protocol handler
-    // associated with this local protocol handler.
-    for (sp<IProtocolHandler> handler : remote_handlers_) {
-      string guid;
-
-      if (!handler->AddRequestHandler(url,
-                                      method,
-                                      binder_handler,
-                                      &guid).isOk()) {
-        LOG(WARNING) << "Remote protocol handler failed to "
-                     << "register new request handler.";
-      } else {
-        guids.emplace(handler, guid);
-      }
+    unique_ptr<RequestHandlerRegistration> registration(
+        new RequestHandlerRegistration(url, method, std::move(handler)));
+
+    for (sp<IProtocolHandler> remote : remote_handlers_) {
+      registration->AddRemote(remote);
     }
 
     int id = last_handler_id_++;
-    handler_ids_.emplace(id, guids);
+
+    request_handler_ids_.emplace(id, std::move(registration));
+
     return id;
   }
 
@@ -115,31 +160,14 @@ class BinderProtocolHandler : public ProtocolHandler {
   }
 
   bool RemoveHandler(int handler_id) override {
-    auto it = handler_ids_.find(handler_id);
-
-    if (it == handler_ids_.end()) {
-      return false;
-    }
-
-    for (const pair<sp<IProtocolHandler>,string>& remote : it->second) {
-      if (!remote.first->RemoveRequestHandler(remote.second).isOk()) {
-        LOG(WARNING) << "Could not remove request handler from remote.";
-      }
-    }
-
-    return true;
+    return request_handler_ids_.erase(handler_id) > 0;
   }
 
   void ResetRemoteProtocolHandlers() {
-    for (auto it : handler_ids_) {
-      for (auto remote : it.second) {
-        if (!remote.first->RemoveRequestHandler(remote.second).isOk()) {
-          LOG(INFO) << "Could not remove request handler from remote.";
-        }
-      }
+    for (auto& it : request_handler_ids_) {
+      it.second->DropRemotes();
     }
 
-    handler_ids_.clear();
     remote_handlers_.clear();
   }
 
@@ -171,10 +199,9 @@ class BinderProtocolHandler : public ProtocolHandler {
   string name_;
   vector<sp<IProtocolHandler>> remote_handlers_;
 
-  // Map of handler ID numbers as returned by AddHandler to pairs of
-  // IProtocolHandler pointers and string handler GUIDs as returned by the
-  // daemon.
-  map<int,set<pair<sp<IProtocolHandler>,string>>> handler_ids_;
+  // Map of local handler IDs to objects containing the various bits of IPC
+  // state we need to maintain those handlers on the remote end.
+  map<int,unique_ptr<RequestHandlerRegistration>> request_handler_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(BinderProtocolHandler);
 };
@@ -221,8 +248,7 @@ void BinderServer::ClearLocalState() {
 bool BinderServer::BuildLocalState(sp<IBinder> server) {
   remote_server_ = android::interface_cast<RemoteServer>(server);
   vector<sp<IBinder>> remote_raw_binders;
-  if (!remote_server_->GetProtocolHandlers(nullptr,
-                                           &remote_raw_binders).isOk()) {
+  if (!remote_server_->GetProtocolHandlers("", &remote_raw_binders).isOk()) {
     // Possibly the server died, this is not necessarily an error.
     LOG(INFO) << "Webservd failed to tell us about protocol handlers.";
     return false;
@@ -243,8 +269,8 @@ bool BinderServer::BuildLocalState(sp<IBinder> server) {
     if (it == local_protocol_handlers_.end()) {
       unique_ptr<BinderProtocolHandler> local_handler(
           new BinderProtocolHandler(name));
-      local_protocol_handlers_[name] = std::move(local_handler);
-      it = local_protocol_handlers_.find(name);
+      it = local_protocol_handlers_.emplace(name,
+                                            std::move(local_handler)).first;
     }
 
     it->second->AddRemote(remote_handler);
@@ -264,7 +290,10 @@ ProtocolHandler* BinderServer::GetProtocolHandler(const string& name) {
   auto it = local_protocol_handlers_.find(name);
 
   if (it == local_protocol_handlers_.end()) {
-    return nullptr;
+    unique_ptr<BinderProtocolHandler> local_handler(
+        new BinderProtocolHandler(name));
+    it = local_protocol_handlers_.emplace(name,
+                                          std::move(local_handler)).first;
   }
 
   return it->second.get();