CrOS OOBE - Skip marketing screen for enterprise

The marketing opt-in screen does not show an option to subscribe
to enterprise and supervised users. Since it does not provide any
value and requires the user to click on 'Get Started' there was a
request to skip it altogether.

Bug: 1128918
Change-Id: Iaf40322824b8d75d738e3f58b9a8cce0316c8349
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414228
Commit-Queue: Renato Silva <rrsilva@google.com>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Auto-Submit: Renato Silva <rrsilva@google.com>
Cr-Commit-Position: refs/heads/master@{#810553}

chrome/browser/chromeos/login/active_directory_login_browsertest.cc

@@ -107,7 +107,6 @@ IN_PROC_BROWSER_TEST_F(ActiveDirectoryLoginTest, LoginSuccess) {
   ad_login_.TestNoError();
   ad_login_.TestDomainHidden();
   ad_login_.SubmitActiveDirectoryCredentials(test_user_, kPassword);
-  test::WaitForLastScreenAndTapGetStarted();
   test::WaitForPrimaryUserSessionStart();
 }
 
@@ -117,7 +116,6 @@ IN_PROC_BROWSER_TEST_F(ActiveDirectoryLoginTest, KerberosVarsCopied) {
   ad_login_.TestNoError();
   ad_login_.TestDomainHidden();
   ad_login_.SubmitActiveDirectoryCredentials(test_user_, kPassword);
-  test::WaitForLastScreenAndTapGetStarted();
   test::WaitForPrimaryUserSessionStart();
 
   base::FilePath dir;
@@ -182,7 +180,6 @@ IN_PROC_BROWSER_TEST_F(ActiveDirectoryLoginTest, PasswordChange_LoginSuccess) {
   fake_authpolicy_client()->set_auth_error(authpolicy::ERROR_NONE);
   ad_login_.SubmitActiveDirectoryPasswordChangeCredentials(
       kPassword, kNewPassword, kNewPassword);
-  test::WaitForLastScreenAndTapGetStarted();
   test::WaitForPrimaryUserSessionStart();
 }
 
@@ -246,7 +243,6 @@ IN_PROC_BROWSER_TEST_F(ActiveDirectoryLoginAutocompleteTest, LoginSuccess) {
 
   ad_login_.SubmitActiveDirectoryCredentials(kTestActiveDirectoryUser,
                                              kPassword);
-  test::WaitForLastScreenAndTapGetStarted();
   test::WaitForPrimaryUserSessionStart();
 }
 

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

@@ -57,10 +57,8 @@ class FamilyLinkNoticeScreenTest : public OobeBaseTest {
   }
 
   void ExpectHelpAppPrefValue(bool expected) {
-    WizardController::default_controller()->PrepareFirstRunPrefs();
-    bool value = ProfileManager::GetActiveUserProfile()->GetPrefs()->GetBoolean(
-        prefs::kHelpAppShouldShowParentalControl);
-    EXPECT_EQ(value, expected);
+    EXPECT_TRUE(help_app_pref_fal_.has_value());
+    EXPECT_EQ(help_app_pref_fal_.value(), expected);
   }
 
   void ClickContinueButtonOnFamilyLinkScreen() {
@@ -87,12 +85,21 @@ class FamilyLinkNoticeScreenTest : public OobeBaseTest {
     ASSERT_FALSE(screen_exited_);
     screen_exited_ = true;
     screen_result_ = result;
+
+    // Fetch the values before OOBE is eventually destroyed after the exit
+    // callback.
+    WizardController::default_controller()->PrepareFirstRunPrefs();
+    help_app_pref_fal_ =
+        ProfileManager::GetActiveUserProfile()->GetPrefs()->GetBoolean(
+            prefs::kHelpAppShouldShowParentalControl);
+
     original_callback_.Run(result);
     if (screen_exit_callback_)
       std::move(screen_exit_callback_).Run();
   }
 
   bool screen_exited_ = false;
+  base::Optional<bool> help_app_pref_fal_;
   base::RepeatingClosure screen_exit_callback_;
   FamilyLinkNoticeScreen::ScreenExitCallback original_callback_;
 

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

@@ -107,7 +107,8 @@ MarketingOptInScreen* MarketingOptInScreen::Get(ScreenManager* manager) {
 
 bool MarketingOptInScreen::MaybeSkip(WizardContext* context) {
   if (!base::FeatureList::IsEnabled(features::kOobeMarketingScreen) ||
-      chrome_user_manager_util::IsPublicSessionOrEphemeralLogin()) {
+      chrome_user_manager_util::IsPublicSessionOrEphemeralLogin() ||
+      IsCurrentUserManaged() /*skip for enterprise and supervised users*/) {
     exit_callback_.Run(Result::NOT_APPLICABLE);
     return true;
   }

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

@@ -21,12 +21,15 @@
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/login/marketing_backend_connector.h"
 #include "chrome/browser/chromeos/login/screen_manager.h"
+#include "chrome/browser/chromeos/login/test/fake_gaia_mixin.h"
 #include "chrome/browser/chromeos/login/test/js_checker.h"
+#include "chrome/browser/chromeos/login/test/local_policy_test_server_mixin.h"
 #include "chrome/browser/chromeos/login/test/local_state_mixin.h"
 #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
 #include "chrome/browser/chromeos/login/test/oobe_base_test.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_exit_waiter.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
+#include "chrome/browser/chromeos/login/test/user_policy_mixin.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -122,11 +125,16 @@ class MarketingOptInScreenTest : public OobeBaseTest,
   void WaitForScreenExit();
   void SetUpLocalState() override {}
 
+  // Logs in as a normal user. Overridden by subclasses.
+  virtual void PerformLogin();
+
   base::Optional<MarketingOptInScreen::Result> screen_result_;
   base::HistogramTester histogram_tester_;
 
  protected:
   base::test::ScopedFeatureList feature_list_;
+  LoginManagerMixin login_manager_mixin_{&mixin_host_, {}, &fake_gaia_};
+
  private:
   void HandleScreenExit(MarketingOptInScreen::Result result);
 
@@ -134,8 +142,8 @@ class MarketingOptInScreenTest : public OobeBaseTest,
   base::RepeatingClosure screen_exit_callback_;
   MarketingOptInScreen::ScreenExitCallback original_callback_;
 
+  FakeGaiaMixin fake_gaia_{&mixin_host_, embedded_test_server()};
   LocalStateMixin local_state_mixin_{&mixin_host_, this};
-  LoginManagerMixin login_manager_mixin_{&mixin_host_};
 };
 
 /**
@@ -177,7 +185,7 @@ void MarketingOptInScreenTest::SetUpOnMainThread() {
       &MarketingOptInScreenTest::HandleScreenExit, base::Unretained(this)));
 
   OobeBaseTest::SetUpOnMainThread();
-  login_manager_mixin_.LoginAsNewRegularUser();
+  PerformLogin();
   OobeScreenExitWaiter(GetFirstSigninScreen()).Wait();
   ProfileManager::GetActiveUserProfile()->GetPrefs()->SetBoolean(
       ash::prefs::kGestureEducationNotificationShown, true);
@@ -254,6 +262,10 @@ void MarketingOptInScreenTest::WaitForScreenExit() {
   run_loop.Run();
 }
 
+void MarketingOptInScreenTest::PerformLogin() {
+  login_manager_mixin_.LoginAsNewRegularUser();
+}
+
 void MarketingOptInScreenTest::HandleScreenExit(
     MarketingOptInScreen::Result result) {
   ASSERT_FALSE(screen_exited_);
@@ -493,4 +505,33 @@ IN_PROC_BROWSER_TEST_F(MarketingOptInScreenTestDisabled, FeatureDisabled) {
                                      0);
 }
 
+class MarketingOptInScreenTestChildUser : public MarketingOptInScreenTest {
+ protected:
+  void SetUpInProcessBrowserTestFixture() override {
+    // Child users require a user policy, set up an empty one so the user can
+    // get through login.
+    ASSERT_TRUE(user_policy_mixin_.RequestPolicyUpdate());
+    OobeBaseTest::SetUpInProcessBrowserTestFixture();
+  }
+  void PerformLogin() override { login_manager_mixin_.LoginAsNewChildUser(); }
+
+ private:
+  LocalPolicyTestServerMixin policy_server_mixin_{&mixin_host_};
+  UserPolicyMixin user_policy_mixin_{
+      &mixin_host_,
+      AccountId::FromUserEmailGaiaId(test::kTestEmail, test::kTestGaiaId),
+      &policy_server_mixin_};
+};
+
+IN_PROC_BROWSER_TEST_F(MarketingOptInScreenTestChildUser, DisabledForChild) {
+  ShowMarketingOptInScreen();
+  WaitForScreenExit();
+  EXPECT_EQ(screen_result_.value(),
+            MarketingOptInScreen::Result::NOT_APPLICABLE);
+  histogram_tester_.ExpectTotalCount(
+      "OOBE.StepCompletionTimeByExitReason.Marketing-opt-in.Next", 0);
+  histogram_tester_.ExpectTotalCount("OOBE.StepCompletionTime.Marketing-opt-in",
+                                     0);
+}
+
 }  // namespace chromeos

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

@@ -17,6 +17,7 @@
 #include "chrome/browser/chromeos/login/screens/recommend_apps/recommend_apps_fetcher.h"
 #include "chrome/browser/chromeos/login/screens/recommend_apps/recommend_apps_fetcher_delegate.h"
 #include "chrome/browser/chromeos/login/screens/recommend_apps/scoped_test_recommend_apps_fetcher_factory.h"
+#include "chrome/browser/chromeos/login/screens/sync_consent_screen.h"
 #include "chrome/browser/chromeos/login/test/js_checker.h"
 #include "chrome/browser/chromeos/login/test/local_policy_test_server_mixin.h"
 #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
@@ -551,6 +552,10 @@ class RecommendAppsScreenManagedTest : public RecommendAppsScreenTest {
 };
 
 IN_PROC_BROWSER_TEST_F(RecommendAppsScreenManagedTest, SkipDueToManagedUser) {
+  // Force the sync screen to be shown so that OOBE isn't destroyed
+  // right after login due to all screens being skipped.
+  auto autoreset = SyncConsentScreen::ForceBrandedBuildForTesting(true);
+
   // Mark user as managed.
   user_policy_mixin_.RequestPolicyUpdate();
 

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

@@ -658,7 +658,6 @@ IN_PROC_BROWSER_TEST_F(SyncConsentActiveDirectoryTest, LoginDoesNotStartSync) {
   ad_login_.TestLoginVisible();
   ad_login_.SubmitActiveDirectoryCredentials(
       "test-user@locally-managed.localhost", "password");
-  test::WaitForLastScreenAndTapGetStarted();
   test::WaitForPrimaryUserSessionStart();
 
   // OS sync is off.

chrome/browser/chromeos/login/test/oobe_screens_utils.cc

@@ -143,12 +143,6 @@ void TapUserCreationNext() {
   test::OobeJS().TapOnPath({"user-creation", "nextButton"});
 }
 
-void WaitForLastScreenAndTapGetStarted() {
-  WaitFor(MarketingOptInScreenView::kScreenId);
-  test::OobeJS().TapOnPath(
-      {"marketing-opt-in", "marketing-opt-in-next-button"});
-}
-
 void WaitForEulaScreen() {
   if (!WizardController::IsBrandedBuildForTesting())
     return;

chrome/browser/chromeos/login/test/oobe_screens_utils.h

@@ -31,7 +31,6 @@ void WaitForEulaScreen();
 void TapEulaAccept();
 void WaitForSyncConsentScreen();
 void ExitScreenSyncConsent();
-void WaitForLastScreenAndTapGetStarted();
 
 class LanguageReloadObserver : public WelcomeScreen::Observer {
  public: