[Remoting Mobile] Rewire connection state reporting logic for telemetry

This CL improves the connection state report accuracy such that:
* If session is disconnected before the CONNECTED state, report a
  CANCEL event. Previously nothing is reported in this state.
* If session is disconnected during the CONNECTED state, report a CLOSE
  event.
* On iOS, report an AUTHENTICATION_FAILED error if the user tries to do
  a third party auth, which is not supported. Previously we don't
  report anything and when this happens.

Bug: 793995
Change-Id: I9155c0efe43892149179e70f52ecee17434ff88e
Reviewed-on: https://chromium-review.googlesource.com/869457
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530249}

remoting/client/chromoting_session.cc

@@ -114,21 +114,37 @@ void ChromotingSession::Connect() {
 }
 
 void ChromotingSession::Disconnect() {
+  DisconnectForReason(protocol::ErrorCode::OK);
+}
+
+void ChromotingSession::DisconnectForReason(protocol::ErrorCode error) {
   if (!runtime_->network_task_runner()->BelongsToCurrentThread()) {
     runtime_->network_task_runner()->PostTask(
-        FROM_HERE, base::Bind(&ChromotingSession::Disconnect, GetWeakPtr()));
+        FROM_HERE, base::BindOnce(&ChromotingSession::DisconnectForReason,
+                                  GetWeakPtr(), error));
     return;
   }
 
-  stats_logging_enabled_ = false;
-
-  // User disconnection will not trigger OnConnectionState(Closed, OK).
-  // Remote disconnection will trigger OnConnectionState(...) and later trigger
-  // Disconnect().
-  if (connected_) {
-    logger_->LogSessionStateChange(ChromotingEvent::SessionState::CLOSED,
-                                   ChromotingEvent::ConnectionError::NONE);
-    connected_ = false;
+  EnableStatsLogging(false);
+
+  // Do not log session state change if the connection is never started or is
+  // already closed.
+  if (session_state_ != protocol::ConnectionToHost::INITIALIZING &&
+      session_state_ != protocol::ConnectionToHost::FAILED &&
+      session_state_ != protocol::ConnectionToHost::CLOSED) {
+    ChromotingEvent::SessionState session_state_to_log;
+    if (error != protocol::ErrorCode::OK) {
+      session_state_to_log = ChromotingEvent::SessionState::CONNECTION_FAILED;
+    } else if (session_state_ == protocol::ConnectionToHost::CONNECTED) {
+      session_state_to_log = ChromotingEvent::SessionState::CLOSED;
+    } else {
+      session_state_to_log = ChromotingEvent::SessionState::CONNECTION_CANCELED;
+    }
+    logger_->LogSessionStateChange(
+        session_state_to_log, ClientTelemetryLogger::TranslateError(error));
+    session_state_ = (error == protocol::ErrorCode::OK)
+                         ? protocol::ConnectionToHost::CLOSED
+                         : protocol::ConnectionToHost::FAILED;
   }
 
   ReleaseResources();
@@ -317,8 +333,8 @@ void ChromotingSession::OnConnectionState(
 
   // This code assumes no intermediate connection state between CONNECTED and
   // CLOSED/FAILED.
-  connected_ = state == protocol::ConnectionToHost::CONNECTED;
-  EnableStatsLogging(connected_);
+  session_state_ = state;
+  EnableStatsLogging(session_state_ == protocol::ConnectionToHost::CONNECTED);
 
   logger_->LogSessionStateChange(ClientTelemetryLogger::TranslateState(state),
                                  ClientTelemetryLogger::TranslateError(error));

remoting/client/chromoting_session.h

@@ -90,14 +90,18 @@ class ChromotingSession : public ClientUserInterface,
   // Must be called before destruction.
   void Disconnect();
 
-  // Requests the android app to fetch a third-party token.
+  // Similar to Disconnect(), except that this method allows you to specify the
+  // reason to disconnect, which will be reported to telemetry.
+  void DisconnectForReason(protocol::ErrorCode error);
+
+  // Requests the client to fetch a third-party token.
   void FetchThirdPartyToken(
       const std::string& host_public_key,
       const std::string& token_url,
       const std::string& scope,
       const protocol::ThirdPartyTokenFetchedCallback& token_fetched_callback);
 
-  // Called by the android app when the token is fetched.
+  // Called by the client when the token is fetched.
   void HandleOnThirdPartyTokenFetched(const std::string& token,
                                       const std::string& shared_secret);
 
@@ -211,9 +215,9 @@ class ChromotingSession : public ClientUserInterface,
   // set of capabilities for this remoting session.
   std::string capabilities_;
 
-  // Indicates whether the client is connected to the host. Used on network
-  // thread.
-  bool connected_ = false;
+  // The current session state. Used on network thread.
+  protocol::ConnectionToHost::State session_state_ =
+      protocol::ConnectionToHost::INITIALIZING;
 
   std::unique_ptr<ClientTelemetryLogger> logger_;
 

remoting/ios/session/remoting_client.mm

@@ -327,6 +327,8 @@ - (void)fetchThirdPartyTokenForUrl:(NSString*)tokenUrl
   // Not supported for iOS yet.
   _sessionDetails.state = SessionFailed;
   _sessionDetails.error = SessionErrorThirdPartyAuthNotSupported;
+  _session->DisconnectForReason(
+      remoting::protocol::ErrorCode::AUTHENTICATION_FAILED);
   [[NSNotificationCenter defaultCenter]
       postNotificationName:kHostSessionStatusChanged
                     object:self