Deprecate HOSTED_NOT_ALLOWED auth error

chrome/browser/signin/signin_global_error_unittest.cc

 // Copyright (c) 2013 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/signin/signin_global_error.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <string>
@@ skipping 121 matching lines @@
     { GoogleServiceAuthError::NONE, false },
     { GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS, true },
     { GoogleServiceAuthError::USER_NOT_SIGNED_UP, true },
     { GoogleServiceAuthError::CONNECTION_FAILED, false },
     { GoogleServiceAuthError::CAPTCHA_REQUIRED, true },
     { GoogleServiceAuthError::ACCOUNT_DELETED, true },
     { GoogleServiceAuthError::ACCOUNT_DISABLED, true },
     { GoogleServiceAuthError::SERVICE_UNAVAILABLE, true },
     { GoogleServiceAuthError::TWO_FACTOR, true },
     { GoogleServiceAuthError::REQUEST_CANCELED, true },
-    { GoogleServiceAuthError::HOSTED_NOT_ALLOWED, true },
+    { GoogleServiceAuthError::HOSTED_NOT_ALLOWED_DEPRECATED, true },
     { GoogleServiceAuthError::UNEXPECTED_SERVICE_RESPONSE, true },
     { GoogleServiceAuthError::SERVICE_ERROR, true },
     { GoogleServiceAuthError::WEB_LOGIN_REQUIRED, true },
   };
   static_assert(arraysize(table) == GoogleServiceAuthError::NUM_STATES,
       "table size should match number of auth error types");
 
   // Mark the profile with an active timestamp so profile_metrics logs it.
   testing_profile_manager()->UpdateLastUser(profile());
 
   for (size_t i = 0; i < arraysize(table); ++i) {
+    if (table[i].error_state ==
+        GoogleServiceAuthError::HOSTED_NOT_ALLOWED_DEPRECATED)
+      continue;

[Greg Levin, 2016/09/08 19:56:35]

I couldn't remove HOSTED_NOT_ALLOWED from this table, because the
histogram_tester requires it to align with the enum in histograms.xml (see test
failures in Patch #2).

This code to skip testing HOSTED_NOT_ALLOWED is crude, but simple seemed okay in
this one-off instance.  I also considered adding a |is_deprecated| member to the
struct, or putting HOSTED_NOT_ALLOWED into a singleton set of entries to be
skipped.  Let me know if one of these, or some other approach, sounds better.

[Roger Tawa OOO till Jul 10th, 2016/09/08 21:05:58]

Got it, I see what you mean.  I guess the problem is at line 174 below.  I
wonder if there should be a method like:

static bool GoogleServiceAuthError::IsDeprecated(State state);

to use in the if statement here and the other test file?

     base::HistogramTester histogram_tester;
     FakeAuthStatusProvider provider(error_controller());
     provider.SetAuthError(kTestAccountId,
                           GoogleServiceAuthError(table[i].error_state));
 
     EXPECT_EQ(global_error()->HasMenuItem(), table[i].is_error);
     EXPECT_EQ(global_error()->MenuItemLabel().empty(), !table[i].is_error);
     EXPECT_EQ(global_error()->GetBubbleViewMessages().empty(),
               !table[i].is_error);
     EXPECT_FALSE(global_error()->GetBubbleViewTitle().empty());
     EXPECT_FALSE(global_error()->GetBubbleViewAcceptButtonLabel().empty());
     EXPECT_TRUE(global_error()->GetBubbleViewCancelButtonLabel().empty());
 
     ProfileMetrics::LogNumberOfProfiles(
         testing_profile_manager()->profile_manager());
 
     if (table[i].is_error)
       histogram_tester.ExpectBucketCount("Signin.AuthError", i, 1);
     histogram_tester.ExpectBucketCount(
         "Profile.NumberOfProfilesWithAuthErrors", table[i].is_error, 1);
   }
 }