PackagedLicenseScreen: Skip screen properly.

Use new method for skipping screen. Add tests for UMA stats.

Bug: 1064561
Change-Id: I2dd5ec7255fc748c33935c201d29384fbbc3ada2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2151445
Commit-Queue: Roman Aleksandrov <raleksandrov@google.com>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760083}

chrome/browser/chromeos/login/screens/packaged_license_screen.cc

@@ -4,6 +4,10 @@
 
 #include "chrome/browser/chromeos/login/screens/packaged_license_screen.h"
 
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_platform_part_chromeos.h"
+#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
+#include "chrome/browser/chromeos/policy/enrollment_config.h"
 #include "chrome/browser/ui/webui/chromeos/login/packaged_license_screen_handler.h"
 
 namespace {
@@ -22,6 +26,8 @@ std::string PackagedLicenseScreen::GetResultString(Result result) {
       return "DontEnroll";
     case Result::ENROLL:
       return "Enroll";
+    case Result::NOT_APPLICABLE:
+      return BaseScreen::kNotApplicable;
   }
 }
 
@@ -40,6 +46,21 @@ PackagedLicenseScreen::~PackagedLicenseScreen() {
     view_->Unbind();
 }
 
+bool PackagedLicenseScreen::MaybeSkip() {
+  policy::EnrollmentConfig enrollment_config =
+      g_browser_process->platform_part()
+          ->browser_policy_connector_chromeos()
+          ->GetPrescribedEnrollmentConfig();
+  // License screen should be shown when device packed with license and other
+  // enrollment flows are not triggered by the device state.
+  if (enrollment_config.is_license_packaged_with_device &&
+      !enrollment_config.should_enroll()) {
+    return false;
+  }
+  exit_callback_.Run(Result::NOT_APPLICABLE);
+  return true;
+}
+
 void PackagedLicenseScreen::ShowImpl() {
   if (view_)
     view_->Show();

chrome/browser/chromeos/login/screens/packaged_license_screen.h

@@ -23,7 +23,9 @@ class PackagedLicenseScreen : public BaseScreen {
     // Show login screen
     DONT_ENROLL,
     // Show enterprise enrollment screen
-    ENROLL
+    ENROLL,
+    // No information about license in the |enrollment_config_|
+    NOT_APPLICABLE
   };
 
   static std::string GetResultString(Result result);
@@ -35,10 +37,19 @@ class PackagedLicenseScreen : public BaseScreen {
   PackagedLicenseScreen& operator=(const PackagedLicenseScreen&) = delete;
   ~PackagedLicenseScreen() override;
 
-  void set_exit_callback_for_testing(const ScreenExitCallback& exit_callback) {
-    exit_callback_ = exit_callback;
+  void AddExitCallbackForTesting(const ScreenExitCallback& testing_callback) {
+    exit_callback_ = base::BindRepeating(
+        [](const ScreenExitCallback& original_callback,
+           const ScreenExitCallback& testing_callback, Result result) {
+          original_callback.Run(result);
+          testing_callback.Run(result);
+        },
+        exit_callback_, testing_callback);
   }
 
+  // BaseScreen
+  bool MaybeSkip() override;
+
  protected:
   // BaseScreen
   void ShowImpl() override;

chrome/browser/chromeos/login/screens/packaged_license_screen_browsertest.cc

@@ -6,13 +6,18 @@
 
 #include "base/bind.h"
 #include "base/run_loop.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/login/screen_manager.h"
 #include "chrome/browser/chromeos/login/test/js_checker.h"
 #include "chrome/browser/chromeos/login/test/oobe_base_test.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/browser/chromeos/policy/enrollment_config.h"
+#include "chrome/browser/chromeos/policy/server_backed_device_state.h"
 #include "chrome/browser/ui/webui/chromeos/login/packaged_license_screen_handler.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
 
 namespace chromeos {
 
@@ -25,19 +30,28 @@ class PackagedLicenseScreenTest : public OobeBaseTest {
     PackagedLicenseScreen* screen = static_cast<PackagedLicenseScreen*>(
         WizardController::default_controller()->screen_manager()->GetScreen(
             PackagedLicenseView::kScreenId));
-    screen->set_exit_callback_for_testing(base::BindRepeating(
+    screen->AddExitCallbackForTesting(base::BindRepeating(
         &PackagedLicenseScreenTest::HandleScreenExit, base::Unretained(this)));
-    policy::EnrollmentConfig config;
-    config.is_license_packaged_with_device = true;
-    WizardController::default_controller()
-        ->set_prescribed_enrollment_config_for_testing(config);
 
     OobeBaseTest::SetUpOnMainThread();
   }
 
+  void SetUpLicense(bool value) {
+    DictionaryPrefUpdate dict(local_state(), prefs::kServerBackedDeviceState);
+    if (value) {
+      dict.Get()->Set(policy::kDeviceStatePackagedLicense,
+                      std::make_unique<base::Value>(true));
+    } else {
+      dict.Get()->Remove(policy::kDeviceStatePackagedLicense, nullptr);
+    }
+  }
+
   void ShowPackagedLicenseScreen() {
     WizardController::default_controller()->AdvanceToScreen(
         PackagedLicenseView::kScreenId);
+  }
+
+  void WaitForScreenShown() {
     OobeScreenWaiter(PackagedLicenseView::kScreenId).Wait();
   }
 
@@ -53,6 +67,10 @@ class PackagedLicenseScreenTest : public OobeBaseTest {
     EXPECT_EQ(result_, result);
   }
 
+  PrefService* local_state() { return g_browser_process->local_state(); }
+
+  base::HistogramTester histogram_tester_;
+
  private:
   void HandleScreenExit(PackagedLicenseScreen::Result result) {
     screen_exited_ = true;
@@ -67,21 +85,51 @@ class PackagedLicenseScreenTest : public OobeBaseTest {
 };
 
 IN_PROC_BROWSER_TEST_F(PackagedLicenseScreenTest, DontEnroll) {
+  SetUpLicense(true);
   ShowPackagedLicenseScreen();
+  WaitForScreenShown();
 
   test::OobeJS().TapOnPath({"packaged-license", "dont-enroll-button"});
 
   WaitForScreenExit();
   CheckResult(PackagedLicenseScreen::Result::DONT_ENROLL);
+  histogram_tester_.ExpectTotalCount(
+      "OOBE.StepCompletionTimeByExitReason.Packaged-license.Enroll", 0);
+  histogram_tester_.ExpectTotalCount(
+      "OOBE.StepCompletionTimeByExitReason.Packaged-license.DontEnroll", 1);
+  histogram_tester_.ExpectTotalCount("OOBE.StepCompletionTime.Packaged-license",
+                                     1);
 }
 
 IN_PROC_BROWSER_TEST_F(PackagedLicenseScreenTest, Enroll) {
+  SetUpLicense(true);
   ShowPackagedLicenseScreen();
+  WaitForScreenShown();
 
   test::OobeJS().TapOnPath({"packaged-license", "enroll-button"});
 
   WaitForScreenExit();
   CheckResult(PackagedLicenseScreen::Result::ENROLL);
+  histogram_tester_.ExpectTotalCount(
+      "OOBE.StepCompletionTimeByExitReason.Packaged-license.Enroll", 1);
+  histogram_tester_.ExpectTotalCount(
+      "OOBE.StepCompletionTimeByExitReason.Packaged-license.DontEnroll", 0);
+  histogram_tester_.ExpectTotalCount("OOBE.StepCompletionTime.Packaged-license",
+                                     1);
+}
+
+IN_PROC_BROWSER_TEST_F(PackagedLicenseScreenTest, NoLicense) {
+  SetUpLicense(false);
+  ShowPackagedLicenseScreen();
+
+  WaitForScreenExit();
+  CheckResult(PackagedLicenseScreen::Result::NOT_APPLICABLE);
+  histogram_tester_.ExpectTotalCount(
+      "OOBE.StepCompletionTimeByExitReason.Packaged-license.Enroll", 0);
+  histogram_tester_.ExpectTotalCount(
+      "OOBE.StepCompletionTimeByExitReason.Packaged-license.DontEnroll", 0);
+  histogram_tester_.ExpectTotalCount("OOBE.StepCompletionTime.Packaged-license",
+                                     0);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/login/wizard_controller.cc

@@ -777,10 +777,7 @@ void WizardController::ShowDiscoverScreen() {
 }
 
 void WizardController::ShowPackagedLicenseScreen() {
-  if (should_show_packaged_license_screen())
-    SetCurrentScreen(GetScreen(PackagedLicenseView::kScreenId));
-  else
-    ShowLoginScreen();
+  SetCurrentScreen(GetScreen(PackagedLicenseView::kScreenId));
 }
 
 void WizardController::SkipToLoginForTesting() {
@@ -1256,6 +1253,7 @@ void WizardController::OnPackagedLicenseScreenExit(
                PackagedLicenseScreen::GetResultString(result));
   switch (result) {
     case PackagedLicenseScreen::Result::DONT_ENROLL:
+    case PackagedLicenseScreen::Result::NOT_APPLICABLE:
       ShowLoginScreen();
       break;
     case PackagedLicenseScreen::Result::ENROLL:

chrome/browser/chromeos/login/wizard_controller.h

@@ -136,19 +136,6 @@ class WizardController {
   // Returns true if the current wizard instance has reached the login screen.
   bool login_screen_started() const { return login_screen_started_; }
 
-  // Returns true if packaged license screen should be shown.
-  // License screen should be shown when device packed with license and other
-  // enrollment flows are not triggered by the device state.
-  bool should_show_packaged_license_screen() const {
-    return prescribed_enrollment_config_.is_license_packaged_with_device &&
-           !prescribed_enrollment_config_.should_enroll();
-  }
-
-  void set_prescribed_enrollment_config_for_testing(
-      policy::EnrollmentConfig config) {
-    prescribed_enrollment_config_ = config;
-  }
-
   // Returns true if a given screen exists.
   bool HasScreen(OobeScreenId screen_id);