Addressing feedback from ValidatingAuthenticator CL

remoting/protocol/validating_authenticator.cc

Index: remoting/protocol/validating_authenticator.cc
diff --git a/remoting/protocol/validating_authenticator.cc b/remoting/protocol/validating_authenticator.cc
index d3da78d12427b741dfdc5359bbb02aebd98ee93c..0546c3d4b144b160e1db6590b9e8a8f6e8e35418 100644
--- a/remoting/protocol/validating_authenticator.cc
+++ b/remoting/protocol/validating_authenticator.cc
@@ -91,18 +91,16 @@ void ValidatingAuthenticator::OnValidateComplete(
     const buzz::XmlElement* message,
     const base::Closure& resume_callback,
     Result validation_result) {
-  if (validation_result == Result::SUCCESS) {
-    current_authenticator_->ProcessMessage(
-        message, base::Bind(&ValidatingAuthenticator::UpdateState,
-                            weak_factory_.GetWeakPtr(), resume_callback));
-    return;
-  }
-
-  // |validation_result| represents a rejected state so map the result to a
-  // rejection reason and call the callback to let the caller know the result.
-  state_ = Authenticator::REJECTED;
-
+  // Process the original message in the success case, otherwise map
+  // |rejection_reason_| to a known reason, set |state_| to REJECTED and notify
+  // the listener of the connection error via the callback.
   switch (validation_result) {
+    case Result::SUCCESS:
+      current_authenticator_->ProcessMessage(
+          message, base::Bind(&ValidatingAuthenticator::UpdateState,
+                              weak_factory_.GetWeakPtr(), resume_callback));
+      return;
+
     case Result::ERROR_INVALID_CREDENTIALS:
       rejection_reason_ = Authenticator::INVALID_CREDENTIALS;
       break;
@@ -115,12 +113,11 @@ void ValidatingAuthenticator::OnValidateComplete(
       rejection_reason_ = Authenticator::REJECTED_BY_USER;
       break;
 
-    default:
-      // Log an error and fail to prevent logging a misleading error value.
-      CHECK(false) << "Unknown validation result value: "
-                   << static_cast<unsigned int>(validation_result);
+      // No default statement exists as we want the compiler to error out if a

[Sergey Ulanov, 2016/09/02 23:13:39]

Don't really need this comment - we don't have similar comments in other switch
statements.

+      // new value is added to the enum but not added here.
   }
 
+  state_ = Authenticator::REJECTED;
   resume_callback.Run();
 }