Minor cleanup of EnrollmentHandlerChromeOS.

chrome/browser/chromeos/login/enrollment/enrollment_screen.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/login/enrollment/enrollment_screen.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 103 matching lines @@
     case GoogleServiceAuthError::ACCOUNT_DISABLED:
       UMAFailure(policy::kMetricEnrollmentNotSupported);
       LOG(ERROR) << "Account error " << error.state();
       return;
     case GoogleServiceAuthError::CONNECTION_FAILED:
     case GoogleServiceAuthError::SERVICE_UNAVAILABLE:
       UMAFailure(policy::kMetricEnrollmentNetworkFailed);
       LOG(WARNING) << "Network error " << error.state();
       return;
     case GoogleServiceAuthError::NUM_STATES:
-      break;
+      NOTREACHED();
+      return;
   }
-
   NOTREACHED();
-  UMAFailure(policy::kMetricEnrollmentOtherFailed);

[Joao da Silva, 2014/08/14 12:11:10]

Why was this sample removed? Even though it's not specific it's still better
than nothing; without it we won't learn that devices out there may be going
through the NOTREACHED flows.

[Thiemo Nagel, 2014/08/14 14:52:37]

It's after a switch on an enum.  The compiler will throw an error if a case has
been missed.  It will not be reached.  Anyways, I've refactored the switch to
use break instead of return.

 }
 
 void EnrollmentScreen::OnOAuthTokenAvailable(const std::string& token) {
   RegisterForDevicePolicy(token);
 }
 
 void EnrollmentScreen::OnRetry() {
   actor_->ResetAuth(base::Bind(&EnrollmentScreen::ShowSigninScreen,
                                weak_ptr_factory_.GetWeakPtr()));
 }
@@ skipping 152 matching lines @@
       return;
     case policy::EnrollmentStatus::STATUS_ROBOT_REFRESH_STORE_FAILED:
       UMAFailure(policy::kMetricEnrollmentRobotRefreshTokenStoreFailed);
       return;
     case policy::EnrollmentStatus::STATUS_STORE_TOKEN_AND_ID_FAILED:
       // This error should not happen for enterprise enrollment.
       UMAFailure(policy::kMetricEnrollmentStoreTokenAndIdFailed);
       NOTREACHED();
       return;
     case policy::EnrollmentStatus::STATUS_SUCCESS:
       NOTREACHED();

[Joao da Silva, 2014/08/14 12:11:10]

Would be nice to have a sample here too, as this is a bug

[Thiemo Nagel, 2014/08/14 14:52:37]

This case is treated in line 221.  I've refactored the switch to make it less
confusing.

       return;
   }
-
   NOTREACHED();
-  UMAFailure(policy::kMetricEnrollmentOtherFailed);

[Joao da Silva, 2014/08/14 12:11:10]

Same here

[Thiemo Nagel, 2014/08/14 14:52:37]

Same as above.

 }
 
 void EnrollmentScreen::UMA(policy::MetricEnrollment sample) {
   if (enrollment_mode_ == EnrollmentScreenActor::ENROLLMENT_MODE_RECOVERY) {
     UMA_HISTOGRAM_ENUMERATION(policy::kMetricEnrollmentRecovery, sample,
                               policy::kMetricEnrollmentSize);
   } else {
     UMA_HISTOGRAM_ENUMERATION(policy::kMetricEnrollment, sample,
                               policy::kMetricEnrollmentSize);
   }
 }
 
 void EnrollmentScreen::UMAFailure(policy::MetricEnrollment sample) {
   if (is_auto_enrollment())
     sample = policy::kMetricEnrollmentAutoFailed;
   UMA(sample);
 }
 
 void EnrollmentScreen::ShowSigninScreen() {
   actor_->Show();
   actor_->ShowSigninScreen();
 }
 
 }  // namespace chromeos