Add UMA metric for CBCM unenrollment

The metric tracks whether the invalid DM token was stored successfully.

Bug: 1039399
Change-Id: I8e1cf72b65353e378841dfc1642d9176638a748a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1988674Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Tien Mai <tienmai@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729942}

chrome/browser/policy/chrome_browser_cloud_management_controller.cc

@@ -314,12 +314,16 @@ void ChromeBrowserCloudManagementController::InvalidatePolicies() {
 
 void ChromeBrowserCloudManagementController::InvalidateDMTokenCallback(
     bool success) {
+  UMA_HISTOGRAM_BOOLEAN(
+      "Enterprise.MachineLevelUserCloudPolicyEnrollment.UnenrollSuccess",
+      success);
   if (success) {
     DVLOG(1) << "Successfully invalidated the DM token";
     InvalidatePolicies();
   } else {
     DVLOG(1) << "Failed to invalidate the DM token";
   }
+  NotifyBrowserUnenrolled(success);
 }
 
 void ChromeBrowserCloudManagementController::OnPolicyFetched(
@@ -347,6 +351,12 @@ void ChromeBrowserCloudManagementController::NotifyPolicyRegisterFinished(
   }
 }
 
+void ChromeBrowserCloudManagementController::NotifyBrowserUnenrolled(
+    bool succeeded) {
+  for (auto& observer : observers_)
+    observer.OnBrowserUnenrolled(succeeded);
+}
+
 bool ChromeBrowserCloudManagementController::GetEnrollmentTokenAndClientId(
     std::string* enrollment_token,
     std::string* client_id) {

chrome/browser/policy/chrome_browser_cloud_management_controller.h

@@ -63,6 +63,9 @@ class ChromeBrowserCloudManagementController
     // Called when policy enrollment is finished.
     // |succeeded| is true if |dm_token| is returned from the server.
     virtual void OnPolicyRegisterFinished(bool succeeded) {}
+
+    // Called when the browser has been unenrolled.
+    virtual void OnBrowserUnenrolled(bool succeeded) {}
   };
 
   // Directory name under the user-data-dir where the policy data is stored.
@@ -99,6 +102,7 @@ class ChromeBrowserCloudManagementController
 
  protected:
   void NotifyPolicyRegisterFinished(bool succeeded);
+  void NotifyBrowserUnenrolled(bool succeeded);
 
  private:
   bool GetEnrollmentTokenAndClientId(std::string* enrollment_token,

chrome/browser/policy/cloud/chrome_browser_cloud_management_browsertest.cc

@@ -74,6 +74,8 @@ const char kDMToken[] = "fake_dm_token";
 const char kInvalidDMToken[] = "invalid_dm_token";
 const char kEnrollmentResultMetrics[] =
     "Enterprise.MachineLevelUserCloudPolicyEnrollment.Result";
+const char kUnenrollmentSuccessMetrics[] =
+    "Enterprise.MachineLevelUserCloudPolicyEnrollment.UnenrollSuccess";
 const char kTestPolicyConfig[] = R"(
 {
   "google/chrome/machine-level-user" : {
@@ -546,17 +548,42 @@ INSTANTIATE_TEST_SUITE_P(ChromeBrowserCloudManagementEnrollmentTest,
                                             ::testing::Bool()));
 
 class MachineLevelUserCloudPolicyPolicyFetchTest
-    : public InProcessBrowserTest,
-      public ::testing::WithParamInterface<std::string> {
+    : public ChromeBrowserCloudManagementControllerObserver,
+      public InProcessBrowserTest,
+      public ::testing::WithParamInterface<std::tuple<std::string, bool>> {
  public:
   MachineLevelUserCloudPolicyPolicyFetchTest() {
     BrowserDMTokenStorage::SetForTesting(&storage_);
     storage_.SetEnrollmentToken(kEnrollmentToken);
     storage_.SetClientId(kClientID);
+    storage_.EnableStorage(storage_enabled());
     if (!dm_token().empty())
       storage_.SetDMToken(dm_token());
   }
 
+  void QuitOnUnenroll(base::RepeatingClosure quit_closure) {
+    quit_closure_ = std::move(quit_closure);
+  }
+
+  void OnBrowserUnenrolled(bool succeeded) override {
+    if (!quit_closure_.is_null()) {
+      EXPECT_FALSE(succeeded);
+      std::move(quit_closure_).Run();
+    }
+  }
+
+  void SetUpOnMainThread() override {
+    g_browser_process->browser_policy_connector()
+        ->chrome_browser_cloud_management_controller()
+        ->AddObserver(this);
+  }
+
+  void TearDownOnMainThread() override {
+    g_browser_process->browser_policy_connector()
+        ->chrome_browser_cloud_management_controller()
+        ->RemoveObserver(this);
+  }
+
   void SetUpInProcessBrowserTestFixture() override {
     SetUpTestServer();
     ASSERT_TRUE(test_server_->Start());
@@ -584,12 +611,17 @@ class MachineLevelUserCloudPolicyPolicyFetchTest
 
   DMToken retrieve_dm_token() { return storage_.RetrieveDMToken(); }
 
-  const std::string dm_token() const { return GetParam(); }
+  const std::string dm_token() const { return std::get<0>(GetParam()); }
+  bool storage_enabled() const { return std::get<1>(GetParam()); }
+
+ protected:
+  base::HistogramTester histogram_tester_;
 
  private:
   std::unique_ptr<LocalPolicyTestServer> test_server_;
   FakeBrowserDMTokenStorage storage_;
   base::ScopedTempDir temp_dir_;
+  base::RepeatingClosure quit_closure_;
 
   DISALLOW_COPY_AND_ASSIGN(MachineLevelUserCloudPolicyPolicyFetchTest);
 };
@@ -608,8 +640,17 @@ IN_PROC_BROWSER_TEST_P(MachineLevelUserCloudPolicyPolicyFetchTest, Test) {
     std::unique_ptr<PolicyFetchCoreObserver> core_observer;
     std::unique_ptr<PolicyFetchStoreObserver> store_observer;
     if (dm_token() == kInvalidDMToken) {
-      core_observer = std::make_unique<PolicyFetchCoreObserver>(
-          manager->core(), run_loop.QuitClosure());
+      if (storage_enabled()) {
+        // |run_loop|'s QuitClosure will be called after the core is
+        // disconnected following unenrollment.
+        core_observer = std::make_unique<PolicyFetchCoreObserver>(
+            manager->core(), run_loop.QuitClosure());
+      } else {
+        // |run_loop|'s QuitClosure will be called after the browser attempts to
+        // unenroll from CBCM. This is necessary to quit the loop in the case
+        // the storage fails since the core is not disconnected.
+        QuitOnUnenroll(run_loop.QuitClosure());
+      }
     } else {
       store_observer = std::make_unique<PolicyFetchStoreObserver>(
           manager->store(), run_loop.QuitClosure());
@@ -637,24 +678,32 @@ IN_PROC_BROWSER_TEST_P(MachineLevelUserCloudPolicyPolicyFetchTest, Test) {
       EXPECT_EQ(token.value(), "fake_device_management_token");
     else
       EXPECT_EQ(token.value(), kDMToken);
+
+    histogram_tester_.ExpectTotalCount(kUnenrollmentSuccessMetrics, 0);
   } else {
     EXPECT_EQ(0u, policy_map.size());
 
     // The token in storage should be invalid.
     DMToken token = retrieve_dm_token();
     EXPECT_TRUE(token.is_invalid());
+
+    histogram_tester_.ExpectUniqueSample(kUnenrollmentSuccessMetrics,
+                                         storage_enabled(), 1);
   }
 }
 
-// The tests here cover three cases:
+// The tests here cover three DM token cases combined with the storage
+// succeeding or failing:
 //  1) Start Chrome with a valid DM token but no policy cache. Chrome will
 //  load the policy from the DM server.
 //  2) Start Chrome with an invalid DM token. Chrome will hit the DM server and
 //  get an error. There should be no more cloud policy applied.
 //  3) Start Chrome without DM token. Chrome will register itself and fetch
 //  policy after it.
-INSTANTIATE_TEST_SUITE_P(MachineLevelUserCloudPolicyPolicyFetchTest,
-                         MachineLevelUserCloudPolicyPolicyFetchTest,
-                         ::testing::Values(kDMToken, kInvalidDMToken, ""));
+INSTANTIATE_TEST_SUITE_P(
+    MachineLevelUserCloudPolicyPolicyFetchTest,
+    MachineLevelUserCloudPolicyPolicyFetchTest,
+    ::testing::Combine(::testing::Values(kDMToken, kInvalidDMToken, ""),
+                       ::testing::Bool()));
 
 }  // namespace policy

tools/metrics/histograms/histograms.xml

@@ -41444,6 +41444,17 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram
+    name="Enterprise.MachineLevelUserCloudPolicyEnrollment.UnenrollSuccess"
+    enum="BooleanSuccess" expires_after="2020-10-01">
+  <owner>domfc@chromium.org</owner>
+  <owner>zmin@chromium.org</owner>
+  <summary>
+    Records whether a browser unenrollment was completed succcessfully by
+    writing an invalid DM token to storage or not.
+  </summary>
+</histogram>
+
 <histogram name="Enterprise.ONC.PolicyValidation" enum="BooleanSuccess">
   <owner>mnissler@chromium.org</owner>
   <summary>Result of the OpenNetworkConfiguration policy validation.</summary>