Readability fixes for server_connection_manager.

Review URL: http://codereview.chromium.org/267029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@28988 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 93 insertions, 67 deletions

diff --git a/chrome/browser/sync/engine/net/server_connection_manager.cc b/chrome/browser/sync/engine/net/server_connection_manager.cc
index e7d6dbd..c8165a9 100644
--- a/chrome/browser/sync/engine/net/server_connection_manager.cc
+++ b/chrome/browser/sync/engine/net/server_connection_manager.cc
@@ -39,7 +39,9 @@ static const ServerConnectionEvent shutdown_event =
     false };
 
 bool ServerConnectionManager::Post::ReadBufferResponse(
-    string* buffer_out, HttpResponse* response, bool require_response) {
+    string* buffer_out,
+    HttpResponse* response,
+    bool require_response) {
   if (RC_REQUEST_OK != response->response_code) {
     response->server_status = HttpResponse::SYNC_SERVER_ERROR;
     return false;
@@ -58,7 +60,8 @@ bool ServerConnectionManager::Post::ReadBufferResponse(
 }
 
 bool ServerConnectionManager::Post::ReadDownloadResponse(
-    HttpResponse* response, string* buffer_out) {
+    HttpResponse* response,
+    string* buffer_out) {
   const int64 bytes_read = ReadResponse(buffer_out,
       static_cast<int>(response->content_length));
 
@@ -72,19 +75,22 @@ bool ServerConnectionManager::Post::ReadDownloadResponse(
 }
 
 namespace {
-  string StripTrailingSlash(const string& s) {
-    int stripped_end_pos = s.size();
-    if (s.at(stripped_end_pos - 1) == '/') {
-      stripped_end_pos = stripped_end_pos - 1;
-    }
 
-    return s.substr(0, stripped_end_pos);
+string StripTrailingSlash(const string& s) {
+  int stripped_end_pos = s.size();
+  if (s.at(stripped_end_pos - 1) == '/') {
+    stripped_end_pos = stripped_end_pos - 1;
   }
+
+  return s.substr(0, stripped_end_pos);
+}
+
 }  // namespace
 
 // TODO(chron): Use a GURL instead of string concatenation.
-  string ServerConnectionManager::Post::MakeConnectionURL(
-    const string& sync_server, const string& path,
+string ServerConnectionManager::Post::MakeConnectionURL(
+    const string& sync_server,
+    const string& path,
     bool use_ssl) const {
   string connection_url = (use_ssl ? "https://" : "http://");
   connection_url += sync_server;
@@ -103,11 +109,13 @@ int ServerConnectionManager::Post::ReadResponse(string* out_buffer,
 }
 
 // A helper class that automatically notifies when the status changes.
-struct WatchServerStatus {
+class WatchServerStatus {
+ public:
   WatchServerStatus(ServerConnectionManager* conn_mgr, HttpResponse* response)
-    : conn_mgr_(conn_mgr), response_(response),
-      reset_count_(conn_mgr->reset_count_),
-      server_reachable_(conn_mgr->server_reachable_) {
+      : conn_mgr_(conn_mgr),
+        response_(response),
+        reset_count_(conn_mgr->reset_count_),
+        server_reachable_(conn_mgr->server_reachable_) {
     response->server_status = conn_mgr->server_status_;
   }
   ~WatchServerStatus() {
@@ -124,6 +132,8 @@ struct WatchServerStatus {
     if (server_reachable_ != conn_mgr_->server_reachable_)
       conn_mgr_->NotifyStatusChanged();
   }
+
+ private:
   ServerConnectionManager* const conn_mgr_;
   HttpResponse* const response_;
   // TODO(timsteele): Should this be Barrier:AtomicIncrement?
@@ -132,7 +142,10 @@ struct WatchServerStatus {
 };
 
 ServerConnectionManager::ServerConnectionManager(
-    const string& server, int port, bool use_ssl, const string& user_agent,
+    const string& server,
+    int port,
+    bool use_ssl,
+    const string& user_agent,
     const string& client_id)
     : sync_server_(server),
       sync_server_port_(port),
@@ -279,7 +292,7 @@ void ServerConnectionManager::ResetConnection() {
 }
 
 bool ServerConnectionManager::IncrementErrorCount() {
-#ifdef OS_WIN
+#if defined(OS_WIN)
   error_count_mutex_.Acquire();
   error_count_++;
 
@@ -303,14 +316,15 @@ bool ServerConnectionManager::IncrementErrorCount() {
 
   error_count_mutex_.Release();
   return true;
-#endif
+#endif  // defined(OS_WIN)
   return true;
 }
 
 void ServerConnectionManager::SetServerParameters(const string& server_url,
-                                                  int port, bool use_ssl) {
+                                                  int port,
+                                                  bool use_ssl) {
   {
-    ParametersLock lock(server_parameters_mutex_);
+    AutoLock lock(server_parameters_mutex_);
     sync_server_ = server_url;
     sync_server_port_ = port;
     use_ssl_ = use_ssl;
@@ -319,8 +333,9 @@ void ServerConnectionManager::SetServerParameters(const string& server_url,
 
 // Returns the current server parameters in server_url and port.
 void ServerConnectionManager::GetServerParameters(string* server_url,
-    int* port, bool* use_ssl) const {
-  ParametersLock lock(server_parameters_mutex_);
+                                                  int* port,
+                                                  bool* use_ssl) const {
+  AutoLock lock(server_parameters_mutex_);
   if (server_url != NULL)
     *server_url = sync_server_;
   if (port != NULL)
diff --git a/chrome/browser/sync/engine/net/server_connection_manager.h b/chrome/browser/sync/engine/net/server_connection_manager.h
index 878c582..5773a44 100644
--- a/chrome/browser/sync/engine/net/server_connection_manager.h
+++ b/chrome/browser/sync/engine/net/server_connection_manager.h
@@ -27,7 +27,7 @@ class DirectoryManager;
 
 namespace sync_pb {
 class ClientToServerMessage;
-};
+}
 
 struct RequestTimingInfo;
 
@@ -94,18 +94,23 @@ inline bool IsGoodReplyFromServer(HttpResponse::ServerConnectionCode code) {
 }
 
 struct ServerConnectionEvent {
-  enum { SHUTDOWN, STATUS_CHANGED } what_happened;
-  HttpResponse::ServerConnectionCode connection_code;
-  bool server_reachable;
-
   // Traits.
   typedef ServerConnectionEvent EventType;
+  enum WhatHappened {
+    SHUTDOWN,
+    STATUS_CHANGED
+  };
+
   static inline bool IsChannelShutdownEvent(const EventType& event) {
     return SHUTDOWN == event.what_happened;
   }
+
+  WhatHappened what_happened;
+  HttpResponse::ServerConnectionCode connection_code;
+  bool server_reachable;
 };
 
-struct WatchServerStatus;
+class WatchServerStatus;
 
 // Use this class to interact with the sync server.
 // The ServerConnectionManager currently supports POSTing protocol buffers.
@@ -113,19 +118,9 @@ struct WatchServerStatus;
 //  *** This class is thread safe. In fact, you should consider creating only
 //  one instance for every server that you need to talk to.
 class ServerConnectionManager {
-  friend class Post;
-  friend struct WatchServerStatus;
  public:
   typedef EventChannel<ServerConnectionEvent, Lock> Channel;
 
-  // The lifetime of the GaiaAuthenticator must be longer than the instance
-  // of the ServerConnectionManager that you're creating.
-  ServerConnectionManager(const std::string& server, int port, bool use_ssl,
-                          const std::string& user_agent,
-                          const std::string& client_id);
-
-  virtual ~ServerConnectionManager();
-
   // buffer_in - will be POSTed
   // buffer_out - string will be overwritten with response
   struct PostBufferParams {
@@ -145,7 +140,8 @@ class ServerConnectionManager {
     virtual ~Post() { }
 
     // Called to initialize and perform an HTTP POST.
-    virtual bool Init(const char* path, const std::string& auth_token,
+    virtual bool Init(const char* path,
+                      const std::string& auth_token,
                       const std::string& payload,
                       HttpResponse* response) = 0;
 
@@ -160,12 +156,13 @@ class ServerConnectionManager {
 
    protected:
     std::string MakeConnectionURL(const std::string& sync_server,
-                                  const std::string& path, bool use_ssl) const;
+                                  const std::string& path,
+                                  bool use_ssl) const;
 
-    void GetServerParams(std::string* server, int* server_port,
+    void GetServerParams(std::string* server,
+                         int* server_port,
                          bool* use_ssl) const {
-      ServerConnectionManager::ParametersLock lock(
-          scm_->server_parameters_mutex_);
+      AutoLock lock(scm_->server_parameters_mutex_);
       server->assign(scm_->sync_server_);
       *server_port = scm_->sync_server_port_;
       *use_ssl = scm_->use_ssl_;
@@ -180,6 +177,16 @@ class ServerConnectionManager {
     RequestTimingInfo* timing_info_;
   };
 
+  // The lifetime of the GaiaAuthenticator must be longer than the instance
+  // of the ServerConnectionManager that you're creating.
+  ServerConnectionManager(const std::string& server,
+                          int port,
+                          bool use_ssl,
+                          const std::string& user_agent,
+                          const std::string& client_id);
+
+  virtual ~ServerConnectionManager();
+
   // POSTS buffer_in and reads a response into buffer_out. Uses our currently
   // set auth token in our headers.
   //
@@ -235,12 +242,14 @@ class ServerConnectionManager {
   // typically called during / after authentication so that the server url
   // can be a function of the user's login id. A side effect of this call is
   // that ResetConnection is called.
-  void SetServerParameters(const std::string& server_url, int port,
+  void SetServerParameters(const std::string& server_url,
+                           int port,
                            bool use_ssl);
 
   // Returns the current server parameters in server_url, port and use_ssl.
   void GetServerParameters(std::string* server_url,
-                           int* port, bool* use_ssl) const;
+                           int* port,
+                           bool* use_ssl) const;
 
   bool terminate_all_io() const {
     AutoLock lock(terminate_all_io_mutex_);
@@ -262,9 +271,29 @@ class ServerConnectionManager {
   }
 
  protected:
+  inline std::string proto_sync_path() const {
+    AutoLock lock(path_mutex_);
+    return proto_sync_path_;
+  }
+
+  std::string get_time_path() const {
+    AutoLock lock(path_mutex_);
+    return get_time_path_;
+  }
+
+  // Called wherever a failure should be taken as an indication that we may
+  // be experiencing connection difficulties.
+  virtual bool IncrementErrorCount();
+
+  // NOTE: Tests rely on this protected function being virtual.
+  //
+  // Internal PostBuffer base function.
+  virtual bool PostBufferToPath(const PostBufferParams*,
+                                const std::string& path,
+                                const std::string& auth_token);
+
   // Protects access to sync_server_, sync_server_port_ and use_ssl_:
   mutable Lock server_parameters_mutex_;
-  typedef AutoLock ParametersLock;
 
   // The sync_server_ is the server that requests will be made to.
   std::string sync_server_;
@@ -283,31 +312,17 @@ class ServerConnectionManager {
 
   // The paths we post to.
   mutable Lock path_mutex_;
-  typedef AutoLock ScopedPathLock;
-
   std::string proto_sync_path_;
   std::string get_time_path_;
 
   // The auth token to use in authenticated requests. Set by the AuthWatcher.
   std::string auth_token_;
 
-  inline std::string proto_sync_path() const {
-    ScopedPathLock lock(path_mutex_);
-    return proto_sync_path_;
-  }
-  std::string get_time_path() const {
-    ScopedPathLock lock(path_mutex_);
-    return get_time_path_;
-  }
-
-  // Called wherever a failure should be taken as an indication that we may
-  // be experiencing connection difficulties.
-  virtual bool IncrementErrorCount();
   Lock error_count_mutex_;  // Protects error_count_
   int error_count_;  // Tracks the number of connection errors.
 
- protected:
   Channel* const channel_;
+
   // Volatile so various threads can call server_status() without
   // synchronization.
   volatile HttpResponse::ServerConnectionCode server_status_;
@@ -316,14 +331,10 @@ class ServerConnectionManager {
   // A counter that is incremented everytime ResetAuthStatus() is called.
   volatile base::subtle::AtomicWord reset_count_;
 
-  // NOTE: Tests rely on this protected function being virtual.
-  //
-  // Internal PostBuffer base function.
-  virtual bool PostBufferToPath(const PostBufferParams*,
-                                const std::string& path,
-                                const std::string& auth_token);
-
  private:
+  friend class Post;
+  friend class WatchServerStatus;
+
   mutable Lock terminate_all_io_mutex_;
   bool terminate_all_io_;  // When set to true, terminate all connections asap.
   DISALLOW_COPY_AND_ASSIGN(ServerConnectionManager);