Don't clear brand for regular cros sessions.

If the brand code persisted in local prefs is empty, reinitialize to correct.

Bug: 846033, 893451
Change-Id: I49cc299374d8530d3a46aee93cb7cb2518ff1dee
Reviewed-on: https://chromium-review.googlesource.com/c/1272447Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Commit-Queue: Roger Tawa <rogerta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599645}

chrome/browser/chromeos/login/session/chrome_session_manager_browsertest.cc

@@ -21,9 +21,11 @@
 #include "chrome/browser/chromeos/settings/stub_install_attributes.h"
 #include "chrome/browser/google/google_brand_chromeos.h"
 #include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h"
+#include "chrome/common/chrome_switches.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/system/fake_statistics_provider.h"
 #include "chromeos/system/statistics_provider.h"
+#include "components/user_manager/user_names.h"
 #include "content/public/test/test_utils.h"
 #include "google_apis/gaia/fake_gaia.h"
 #include "rlz/buildflags/buildflags.h"
@@ -235,13 +237,69 @@ IN_PROC_BROWSER_TEST_F(ChromeSessionManagerRlzTest, DeviceIsLocked) {
 }
 
 IN_PROC_BROWSER_TEST_F(ChromeSessionManagerRlzTest, DeviceIsUnlocked) {
-  // When the device is unlocked, the brand should be cleared after session
-  // start.
+  // When the device is unlocked, the brand should still stick after a
+  // regular session start.
   stub_install_attributes()->set_device_locked(false);
   StartUserSession();
-  EXPECT_EQ("", google_brand::chromeos::GetBrand());
+  EXPECT_EQ("TEST", google_brand::chromeos::GetBrand());
+}
+
+class GuestSessionRlzTest : public InProcessBrowserTest,
+                            public ::testing::WithParamInterface<bool> {
+ public:
+  GuestSessionRlzTest() : is_locked_(GetParam()) {}
+
+ protected:
+  StubInstallAttributes* stub_install_attributes() {
+    return scoped_stub_install_attributes_->Get();
+  }
+
+ private:
+  void SetUpInProcessBrowserTestFixture() override {
+    // Set the default brand code to a known value.
+    scoped_fake_statistics_provider_.reset(
+        new system::ScopedFakeStatisticsProvider());
+    scoped_fake_statistics_provider_->SetMachineStatistic(
+        system::kRlzBrandCodeKey, "TEST");
+
+    // Lock the device as needed for this test.
+    scoped_stub_install_attributes_ =
+        std::make_unique<ScopedStubInstallAttributes>(
+            StubInstallAttributes::CreateUnset());
+    scoped_stub_install_attributes_->Get()->set_device_locked(is_locked_);
+
+    InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(chromeos::switches::kGuestSession);
+    command_line->AppendSwitch(::switches::kIncognito);
+    command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile, "hash");
+    command_line->AppendSwitchASCII(
+        chromeos::switches::kLoginUser,
+        user_manager::GuestAccountId().GetUserEmail());
+  }
+
+  // Test instance parameters.
+  const bool is_locked_;
+
+  std::unique_ptr<system::ScopedFakeStatisticsProvider>
+      scoped_fake_statistics_provider_;
+  std::unique_ptr<ScopedStubInstallAttributes> scoped_stub_install_attributes_;
+
+  DISALLOW_COPY_AND_ASSIGN(GuestSessionRlzTest);
+};
+
+IN_PROC_BROWSER_TEST_P(GuestSessionRlzTest, DeviceIsLocked) {
+  const char* const expected_brand =
+      stub_install_attributes()->IsDeviceLocked() ? "TEST" : "";
+  EXPECT_EQ(expected_brand, google_brand::chromeos::GetBrand());
 }
 
+INSTANTIATE_TEST_CASE_P(GuestSessionRlzTest,
+                        GuestSessionRlzTest,
+                        ::testing::Values(false, true));
+
 #endif  // BUILDFLAG(ENABLE_RLZ)
 
 }  // namespace chromeos

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -683,7 +683,15 @@ bool UserSessionManager::UserSessionsRestoreInProgress() const {
 
 void UserSessionManager::InitRlz(Profile* profile) {
 #if BUILDFLAG(ENABLE_RLZ)
-  if (!g_browser_process->local_state()->HasPrefPath(prefs::kRLZBrand)) {
+  // Initialize the brand code in the local prefs if it does not exist yet or
+  // if it is empty.  The latter is to correct a problem in older builds where
+  // an empty brand code would be persisted if the first login after OOBE was
+  // a guest session.
+  if (!g_browser_process->local_state()->HasPrefPath(prefs::kRLZBrand) ||
+      g_browser_process->local_state()
+          ->Get(prefs::kRLZBrand)
+          ->GetString()
+          .empty()) {
     // Read brand code asynchronously from an OEM data and repost ourselves.
     google_brand::chromeos::InitBrand(
         base::Bind(&UserSessionManager::InitRlz, AsWeakPtr(), profile));
@@ -1682,9 +1690,30 @@ void UserSessionManager::RestoreAuthSessionImpl(
 void UserSessionManager::InitRlzImpl(Profile* profile,
                                      const RlzInitParams& params) {
 #if BUILDFLAG(ENABLE_RLZ)
-  // RLZ is disabled if disabled explicitly or if the device is not yet locked.
+  // If RLZ is disabled then clear the brand for the session.
+  //
+  // RLZ is disabled if disabled explicitly OR if the device's enrollment
+  // state is not yet known. The device's enrollment state is definitively
+  // known once the device is locked. Note that for enrolled devices, the
+  // enrollment login locks the device.
+  //
+  // There the following cases to consider when a session starts:
+  //
+  // 1) This is a regular session.
+  // 1a) The device is LOCKED. Thus, the enrollment state is KNOWN.
+  // 1b) The device is NOT LOCKED. This should only happen on the first
+  //     regular login (due to lock race condition with this code) if the
+  //     device is NOT enrolled; thus, the enrollment state is also KNOWN.
+  //
+  // 2) This is a guest session.
+  // 2a) The device is LOCKED. Thus, the enrollment state is KNOWN.
+  // 2b) The device is NOT locked. This should happen if ONLY Guest mode
+  //     sessions have ever been used on this device. This is the only
+  //     situation where the enrollment state is NOT KNOWN at this point.
+
   PrefService* local_state = g_browser_process->local_state();
-  if (params.disabled || !InstallAttributes::Get()->IsDeviceLocked()) {
+  if (params.disabled || (profile->IsGuestSession() &&
+                          !InstallAttributes::Get()->IsDeviceLocked())) {
     // Empty brand code means an organic install (no RLZ pings are sent).
     google_brand::chromeos::ClearBrandForCurrentSession();
   }