arc: Differentiate restart and logout for app push logger.

We need to distinguish logout and restart in app push install logger.
Currently there is no easy way to get this information. At this moment
only BootTimesRecorder is explicitly notified. This CL makes this
information public and let log collector use it.

Bug: b/73277923
Test: Manual + unit test
Change-Id: I64eea3b748e4ad089365fe3453e9c5fc0e6c3162
Reviewed-on: https://chromium-review.googlesource.com/949826Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bartosz Fabianowski <bartfab@chromium.org>
Commit-Queue: Yury Khmel <khmel@google.com>
Cr-Commit-Position: refs/heads/master@{#541722}

chrome/browser/chromeos/policy/app_install_event_log_collector.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/chromeos/policy/app_install_event_log_collector.h"
 #include "base/command_line.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/common/pref_names.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/network/network_handler.h"
@@ -11,6 +13,7 @@
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_type_pattern.h"
 #include "components/policy/proto/device_management_backend.pb.h"
+#include "components/prefs/pref_service.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace em = enterprise_management;
@@ -80,6 +83,10 @@ void AppInstallEventLogCollector::AddLoginEvent() {
 }
 
 void AppInstallEventLogCollector::AddLogoutEvent() {
+  // Don't log in case session is restared.
+  if (g_browser_process->local_state()->GetBoolean(prefs::kWasRestarted))
+    return;
+
   delegate_->AddForAllPackages(
       CreateSessionChangeEvent(em::AppInstallReportLogEvent::LOGOUT));
 }

chrome/browser/chromeos/policy/app_install_event_log_collector_unittest.cc

@@ -8,6 +8,10 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/prefs/browser_prefs.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -15,6 +19,8 @@
 #include "chromeos/dbus/shill_service_client.h"
 #include "chromeos/network/network_handler.h"
 #include "components/policy/proto/device_management_backend.pb.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/testing_pref_service.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
@@ -71,6 +77,8 @@ class AppInstallEventLogCollectorTest : public testing::Test {
   ~AppInstallEventLogCollectorTest() override = default;
 
   void SetUp() override {
+    RegisterLocalState(pref_service_.registry());
+    TestingBrowserProcess::GetGlobal()->SetLocalState(&pref_service_);
     std::unique_ptr<chromeos::FakePowerManagerClient> power_manager_client =
         std::make_unique<chromeos::FakePowerManagerClient>();
     power_manager_client_ = power_manager_client.get();
@@ -99,6 +107,7 @@ class AppInstallEventLogCollectorTest : public testing::Test {
     profile_.reset();
     chromeos::NetworkHandler::Shutdown();
     chromeos::DBusThreadManager::Shutdown();
+    TestingBrowserProcess::GetGlobal()->SetLocalState(nullptr);
   }
 
   void SetNetworkState(const std::string& service_path,
@@ -140,6 +149,7 @@ class AppInstallEventLogCollectorTest : public testing::Test {
   content::TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<TestingProfile> profile_;
   FakeAppInstallEventLogCollectorDelegate delegate_;
+  TestingPrefServiceSimple pref_service_;
   chromeos::FakePowerManagerClient* power_manager_client_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(AppInstallEventLogCollectorTest);
@@ -202,9 +212,19 @@ TEST_F(AppInstallEventLogCollectorTest, LoginTypes) {
   }
 
   {
-    // Check restart. No log is expected.
+    // Check login after restart. No log is expected.
+    AppInstallEventLogCollector collector(delegate(), profile(), packages_);
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         chromeos::switches::kLoginUser);
+    collector.AddLoginEvent();
+    EXPECT_EQ(1, delegate()->add_for_all_count());
+  }
+
+  {
+    // Check logout on restart. No log is expected.
+    AppInstallEventLogCollector collector(delegate(), profile(), packages_);
+    g_browser_process->local_state()->SetBoolean(prefs::kWasRestarted, true);
+    collector.AddLogoutEvent();
     EXPECT_EQ(1, delegate()->add_for_all_count());
   }
 

chrome/browser/ui/startup/startup_browser_creator.cc

@@ -402,8 +402,11 @@ SessionStartupPref StartupBrowserCreator::GetSessionStartupPref(
       user_manager::UserManager::Get()->IsCurrentUserNew();
   // On ChromeOS restarts force the user to login again. The expectation is that
   // after a login the user gets clean state. For this reason we ignore
-  // StartupBrowserCreator::WasRestarted().
+  // StartupBrowserCreator::WasRestarted(). However
+  // StartupBrowserCreator::WasRestarted has to be called in order to correctly
+  // update pref values.
   const bool did_restart = false;
+  StartupBrowserCreator::WasRestarted();
 #else
   const bool is_first_run = first_run::IsChromeFirstRun();
   const bool did_restart = StartupBrowserCreator::WasRestarted();

chrome/browser/ui/startup/startup_browser_creator_browsertest.cc

@@ -10,8 +10,10 @@
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/json/json_writer.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/histogram_tester.h"
 #include "base/threading/thread_restrictions.h"
@@ -44,6 +46,7 @@
 #include "chrome/browser/ui/startup/startup_tab_provider.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/buildflags.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
@@ -1393,3 +1396,36 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorWelcomeBackTest,
                    PolicyVariant(policy::POLICY_LEVEL_RECOMMENDED)));
   ExpectUrlInBrowserAtPosition(GURL("http://managed.site.com/"), 0);
 }
+
+// Validates that prefs::kWasRestarted is automatically reset after next browser
+// start.
+class StartupBrowserCreatorWasRestartedFlag : public InProcessBrowserTest {
+ public:
+  StartupBrowserCreatorWasRestartedFlag() = default;
+  ~StartupBrowserCreatorWasRestartedFlag() override = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    InProcessBrowserTest::SetUpCommandLine(command_line);
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    command_line->AppendSwitchPath(switches::kUserDataDir, temp_dir_.GetPath());
+    std::string json;
+    base::DictionaryValue local_state;
+    local_state.SetBoolean(prefs::kWasRestarted, true);
+    base::JSONWriter::Write(local_state, &json);
+    ASSERT_EQ(json.length(),
+              static_cast<size_t>(base::WriteFile(
+                  temp_dir_.GetPath().Append(chrome::kLocalStateFilename),
+                  json.c_str(), json.length())));
+  }
+
+ private:
+  base::ScopedTempDir temp_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(StartupBrowserCreatorWasRestartedFlag);
+};
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorWasRestartedFlag, Test) {
+  EXPECT_TRUE(StartupBrowserCreator::WasRestarted());
+  EXPECT_FALSE(
+      g_browser_process->local_state()->GetBoolean(prefs::kWasRestarted));
+}