[CRD iOS] Implementing save to keychain and user defaults for login info. Refactor remoting service.

remoting/client/ios/facade/remoting_service.h

Index: remoting/client/ios/facade/remoting_service.h
diff --git a/remoting/client/ios/facade/remoting_service.h b/remoting/client/ios/facade/remoting_service.h
index 2f27fba1ad2b340a1dda0355a130ded17d34b75e..b70d43a579b3c65ca476231db4c07e7078176271 100644
--- a/remoting/client/ios/facade/remoting_service.h
+++ b/remoting/client/ios/facade/remoting_service.h
@@ -6,29 +6,22 @@
 #define REMOTING_CLIENT_IOS_FACADE_REMOTING_SERVICE_H_
 
 #import "remoting/client/chromoting_client_runtime.h"
-#import "remoting/client/ios/domain/host_info.h"
-#import "remoting/client/ios/domain/user_info.h"
 
 #include "base/memory/weak_ptr.h"
 #include "remoting/base/oauth_token_getter.h"
 
-// |RemotingAuthenticationDelegate|s are interested in authentication related
-// notifications.
-@protocol RemotingAuthenticationDelegate<NSObject>
+@class HostInfo;
+@class UserInfo;
+@class RemotingAuthentication;
 
-// Notifies the delegate that the authentication status of the current user has
-// changed to a new state.
-- (void)nowAuthenticated:(BOOL)authenticated;
+// Eventing related keys:
 
-@end
-
-// |RemotingHostListDelegate|s are interested in notifications related to host
-// list.
-@protocol RemotingHostListDelegate<NSObject>
-
-- (void)hostListUpdated;
-
-@end
+// Hosts did update event.
+extern NSString* const kHostsDidUpdate;
+// User did update event name.
+extern NSString* const kUserDidUpdate;
+// Map key for UserInfo object.
+extern NSString* const kUserInfo;
 
 // |RemotingService| is the centralized place to ask for information about
 // authentication or query the remote services. It also helps deal with the
@@ -39,38 +32,24 @@
 // Access to the singleton shared instance from this method.
 + (RemotingService*)SharedInstance;
 
-// Access to the current |ChromotingClientRuntime| from this method.
-- (remoting::ChromotingClientRuntime*)runtime;
-
-// Register to be a |RemotingAuthenticationDelegate|.
-- (void)setAuthenticationDelegate:(id<RemotingAuthenticationDelegate>)delegate;
-
-// A cached answer if there is a currently authenticated user.
-- (BOOL)isAuthenticated;
-
-// Provide an |authorizationCode| to authenticate a user as the first time user
-// of the application or OAuth Flow.
-- (void)authenticateWithAuthorizationCode:(NSString*)authorizationCode;
+// Logout of the current account and clear data.

[Yuwei, 2017/05/04 05:10:14]

It's kind of confusing that this class provides methods similar to those in
RemotingAuthentication but they don't work exactly the same. One could be
tempted to call logout in |authentication| and found that the host list doesn't
get cleared.

I would prefer either completely encapulating |authentication| or handle the
user == nil case in the userDidUpdate callback (if it still gets called in user
== nil case, otherwise you may add a userDidLogOut).

[nicholss, 2017/05/08 17:08:10]

I like the fact that authentication object is exposed. I moved logout to be only
in authentication. Service is notified if user goes nil and acts on the info.

+- (void)logout;
 
-// Provide the |refreshToken| and |email| to authenticate a user as a returning
-// user of the application.
-- (void)authenticateWithRefreshToken:(NSString*)refreshToken
-                               email:(NSString*)email;
+// Start a request to fetch the host list. This will produce an notification on
+// |kHostsDidUpdate| when a new host is ready.
+- (void)requestHostListFetch;
 
-// Returns the currently logged in user info from cache, or nil if no
-// currently authenticated user.
-- (UserInfo*)getUser;
+@property(nonatomic, readonly) RemotingAuthentication* authentication;
 
-// Register to be a |RemotingHostListDelegate|. Side effect of setting this
-// delegate is the application will attempt to fetch a fresh host list.
-- (void)setHostListDelegate:(id<RemotingHostListDelegate>)delegate;
+// Returns the currently logged in user or nil.
+@property(nonatomic, readonly) UserInfo* user;

[Yuwei, 2017/05/04 05:10:14]

I'm not sure how useful this can be given that it's just a shortcut to
authentication.user. One may hesitate to choose user vs authentication.user
without looking at the implementation.

[nicholss, 2017/05/08 17:08:10]

I was not 100% sure how the api would end up but I settled on the user living in
the auth object and this was not needed. Removed.

 
-// Returns the currently cached host list or nil if none exist.
-- (NSArray<HostInfo*>*)getHosts;
+// Returns the current host list.
+@property(nonatomic, readonly) NSArray<HostInfo*>* hosts;
 
-// Fetches an OAuth Access Token and passes it back to the callback.
-- (void)callbackWithAccessToken:
-    (const remoting::OAuthTokenGetter::TokenCallback&)onAccessToken;
+// The Chromoting Client Runtime, this holds the threads and other shared
+// resources used by the Chromoting clients
+@property(nonatomic, readonly) remoting::ChromotingClientRuntime* runtime;
 
 @end