diagnostics: add ResetAndInitializeLogWriters to controller

- Add public ResetAndInitializeLogWriters function called when flag active and Diagnostics app launched by factory method to update log_base_path_.
- Update DEPs.
- Add helper to configure log_base_path_ based on the SessionState and LoginStatus.
- Test log path updates correctly for Guest, Sign-in, KioskApp, and Regular users.

Bug: b:225191282
Test: ./out/Default/ash_unittests --gtest_filter=*Diagnostics*. Manually deploy to confirm path using debug log output.
Change-Id: I9cb1906b735970e686daf959f9af27c7187f86d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3553589
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Gavin Williams <gavinwill@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Ashley Prasad <ashleydp@google.com>
Auto-Submit: Ashley Prasad <ashleydp@google.com>
Cr-Commit-Position: refs/heads/main@{#994405}

ash/system/diagnostics/diagnostics_log_controller.cc

@@ -4,24 +4,42 @@
 
 #include "ash/system/diagnostics/diagnostics_log_controller.h"
 
+#include "ash/public/cpp/session/session_types.h"
+#include "ash/session/session_controller_impl.h"
+#include "ash/shell.h"
 #include "ash/system/diagnostics/diagnostics_browser_delegate.h"
 #include "base/check_op.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
+#include "base/logging.h"
+#include "components/session_manager/session_manager_types.h"
 
 namespace ash {
 namespace diagnostics {
 
 namespace {
 
 DiagnosticsLogController* g_instance = nullptr;
+// Default path for storing logs.
+const char kDiaganosticsTmpDir[] = "/tmp/diagnostics";
+const char kDiaganosticsDirName[] = "diagnostics";
+
+// Determines if profile should be accessed with current session state.  If at
+// sign-in screen, guest user, kiosk app, or before the profile has
+// successfully loaded temporary path should be used for storing logs.
+bool ShouldUseActiveUserProfileDir(session_manager::SessionState state,
+                                   LoginStatus status) {
+  return state == session_manager::SessionState::ACTIVE &&
+         status == ash::LoginStatus::USER;
+}
 
 // Placeholder session log contents.
 const char kLogFileContents[] = "Diagnostics Log";
 
 }  // namespace
 
-DiagnosticsLogController::DiagnosticsLogController() {
+DiagnosticsLogController::DiagnosticsLogController()
+    : log_base_path_(kDiaganosticsTmpDir) {
   DCHECK_EQ(nullptr, g_instance);
   g_instance = this;
 }
@@ -46,6 +64,7 @@ void DiagnosticsLogController::Initialize(
     std::unique_ptr<DiagnosticsBrowserDelegate> delegate) {
   DCHECK(g_instance);
   g_instance->delegate_ = std::move(delegate);
+  g_instance->ResetLogBasePath();
 }
 
 bool DiagnosticsLogController::GenerateSessionLogOnBlockingPool(
@@ -57,5 +76,36 @@ bool DiagnosticsLogController::GenerateSessionLogOnBlockingPool(
   return base::WriteFile(save_file_path, kLogFileContents);
 }
 
+void DiagnosticsLogController::ResetAndInitializeLogWriters() {
+  if (!DiagnosticsLogController::IsInitialized()) {
+    return;
+  }
+
+  ResetLogBasePath();
+}
+
+void DiagnosticsLogController::ResetLogBasePath() {
+  const session_manager::SessionState state =
+      ash::Shell::Get()->session_controller()->GetSessionState();
+  const LoginStatus status =
+      ash::Shell::Get()->session_controller()->login_status();
+
+  // Check if there is an active user and profile is ready based on session and
+  // login state.
+  if (ShouldUseActiveUserProfileDir(state, status)) {
+    base::FilePath user_dir = g_instance->delegate_->GetActiveUserProfileDir();
+
+    // Update |log_base_path_| when path is non-empty. Otherwise fallback to
+    // |kDiaganosticsTmpDir|.
+    if (!user_dir.empty()) {
+      g_instance->log_base_path_ = user_dir.Append(kDiaganosticsDirName);
+      return;
+    }
+  }
+
+  // Use diagnostics temporary path for Guest, KioskApp, and no user states.
+  g_instance->log_base_path_ = base::FilePath(kDiaganosticsTmpDir);
+}
+
 }  // namespace diagnostics
 }  // namespace ash

ash/system/diagnostics/diagnostics_log_controller.h

@@ -37,8 +37,19 @@ class ASH_EXPORT DiagnosticsLogController {
   // whether file creation is successful.
   bool GenerateSessionLogOnBlockingPool(const base::FilePath& save_file_path);
 
+  // Ensures DiagnosticsLogController is configured to match the current
+  // environment. To be called from DiagnosticsDialog::ShowDialog prior to the
+  // Diagnostics app being shown.
+  void ResetAndInitializeLogWriters();
+
  private:
+  friend class DiagnosticsLogControllerTest;
+
+  // Ensure log_base_path_ set based on current session and ash::LoginStatus.
+  void ResetLogBasePath();
+
   std::unique_ptr<DiagnosticsBrowserDelegate> delegate_;
+  base::FilePath log_base_path_;
 };
 
 }  // namespace diagnostics

ash/system/diagnostics/diagnostics_log_controller_unittest.cc

@@ -8,11 +8,13 @@
 
 #include "ash/constants/ash_features.h"
 #include "ash/shell.h"
+#include "ash/system/diagnostics/diagnostics_browser_delegate.h"
 #include "ash/test/ash_test_base.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/test/scoped_feature_list.h"
+#include "components/user_manager/user_type.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace ash {
@@ -22,14 +24,23 @@ namespace {
 
 const char kLogFileContents[] = "Diagnostics Log";
 const char kTestSessionLogFileName[] = "test_session_log.txt";
+const char kDiangosticsDirName[] = "diagnostics";
+const char kDefaultUserDir[] = "/fake/user-dir";
+const char kTmpDiagnosticsDir[] = "/tmp/diagnostics";
+const char kTestUserEmail[] = "test-user@gmail.com";
 
 // Fake delegate used to set the expected user directory path.
 class FakeDiagnosticsBrowserDelegate : public DiagnosticsBrowserDelegate {
  public:
-  FakeDiagnosticsBrowserDelegate() = default;
+  explicit FakeDiagnosticsBrowserDelegate(
+      const base::FilePath path = base::FilePath(kDefaultUserDir))
+      : active_user_dir_(path) {}
   ~FakeDiagnosticsBrowserDelegate() override = default;
 
-  base::FilePath GetActiveUserProfileDir() override { return base::FilePath(); }
+  base::FilePath GetActiveUserProfileDir() override { return active_user_dir_; }
+
+ private:
+  base::FilePath active_user_dir_;
 };
 
 }  // namespace
@@ -55,6 +66,25 @@ class DiagnosticsLogControllerTest : public NoSessionAshTestBase {
     return save_dir_.GetPath().Append(kTestSessionLogFileName);
   }
 
+  void ResetLogBasePath() {
+    return DiagnosticsLogController::Get()->ResetLogBasePath();
+  }
+
+  base::FilePath log_base_path() {
+    return DiagnosticsLogController::Get()->log_base_path_;
+  }
+
+  void SetBrowserDelegate(
+      std::unique_ptr<DiagnosticsBrowserDelegate> delegate) {
+    DiagnosticsLogController::Get()->delegate_ = std::move(delegate);
+  }
+
+  void InitializeWithFakeDelegate() {
+    std::unique_ptr<DiagnosticsBrowserDelegate> delegate =
+        std::make_unique<FakeDiagnosticsBrowserDelegate>();
+    DiagnosticsLogController::Initialize(std::move(delegate));
+  }
+
  private:
   base::test::ScopedFeatureList feature_list_;
   base::ScopedTempDir save_dir_;
@@ -69,8 +99,8 @@ TEST_F(DiagnosticsLogControllerTest,
 TEST_F(DiagnosticsLogControllerTest, IsInitializedAfterDelegateProvided) {
   EXPECT_NE(nullptr, DiagnosticsLogController::Get());
   EXPECT_FALSE(DiagnosticsLogController::IsInitialized());
-  DiagnosticsLogController::Initialize(
-      std::make_unique<FakeDiagnosticsBrowserDelegate>());
+
+  InitializeWithFakeDelegate();
   EXPECT_TRUE(DiagnosticsLogController::IsInitialized());
 }
 
@@ -85,5 +115,55 @@ TEST_F(DiagnosticsLogControllerTest, GenerateSessionLogOnBlockingPoolFile) {
   EXPECT_EQ(kLogFileContents, contents);
 }
 
+TEST_F(DiagnosticsLogControllerTest,
+       ResetAndInitializeShouldNotLookupProfilePath) {
+  const base::FilePath expected_path_not_regular_user =
+      base::FilePath(kTmpDiagnosticsDir);
+  // Simulate called before delegate configured.
+  DiagnosticsLogController::Get()->ResetAndInitializeLogWriters();
+  EXPECT_EQ(expected_path_not_regular_user, log_base_path());
+  InitializeWithFakeDelegate();
+
+  // Simulate sign-in user.
+  ClearLogin();
+  DiagnosticsLogController::Get()->ResetAndInitializeLogWriters();
+  EXPECT_EQ(expected_path_not_regular_user, log_base_path());
+
+  SimulateGuestLogin();
+  DiagnosticsLogController::Get()->ResetAndInitializeLogWriters();
+  EXPECT_EQ(expected_path_not_regular_user, log_base_path());
+
+  SimulateKioskMode(user_manager::UserType::USER_TYPE_KIOSK_APP);
+  DiagnosticsLogController::Get()->ResetAndInitializeLogWriters();
+  EXPECT_EQ(expected_path_not_regular_user, log_base_path());
+
+  SimulateKioskMode(user_manager::UserType::USER_TYPE_ARC_KIOSK_APP);
+  DiagnosticsLogController::Get()->ResetAndInitializeLogWriters();
+  EXPECT_EQ(expected_path_not_regular_user, log_base_path());
+}
+
+TEST_F(DiagnosticsLogControllerTest,
+       ResetAndInitializeShouldLookupProfileUserEmptyPath) {
+  const base::FilePath expected_path_not_regular_user =
+      base::FilePath(kTmpDiagnosticsDir);
+  // Simulate DiagnosticsBrowserDelegate returning empty path.
+  std::unique_ptr<DiagnosticsBrowserDelegate> delegate_with_empty_file_path =
+      std::make_unique<FakeDiagnosticsBrowserDelegate>(base::FilePath());
+  SetBrowserDelegate(std::move(delegate_with_empty_file_path));
+  SimulateUserLogin(kTestUserEmail);
+  DiagnosticsLogController::Get()->ResetAndInitializeLogWriters();
+  EXPECT_EQ(expected_path_not_regular_user, log_base_path());
+}
+
+TEST_F(DiagnosticsLogControllerTest,
+       ResetAndInitializeForShouldLookupProfileUserNonEmptyPath) {
+  InitializeWithFakeDelegate();
+  const base::FilePath expected_path_regular_user =
+      base::FilePath(kDefaultUserDir).Append(kDiangosticsDirName);
+  SimulateUserLogin(kTestUserEmail);
+  DiagnosticsLogController::Get()->ResetAndInitializeLogWriters();
+  EXPECT_EQ(expected_path_regular_user, log_base_path());
+}
+
 }  // namespace diagnostics
 }  // namespace ash

chrome/browser/ui/webui/chromeos/DEPS

@@ -1,6 +1,7 @@
 include_rules = [
   "+ash/utility",
   "+ash/system/hps",
+  "+ash/system/diagnostics",
 
   # Chrome OS depends on views, so allow code in this directory to depend on
   # ui/views.

chrome/browser/ui/webui/chromeos/diagnostics_dialog.cc

@@ -6,6 +6,8 @@
 
 #include <string>
 
+#include "ash/constants/ash_features.h"
+#include "ash/system/diagnostics/diagnostics_log_controller.h"
 #include "ash/webui/diagnostics_ui/url_constants.h"
 #include "base/strings/strcat.h"
 #include "ui/display/display.h"
@@ -36,6 +38,13 @@ const float kDiagnosticsDialogScale = .8;
 void DiagnosticsDialog::ShowDialog(DiagnosticsDialog::DiagnosticsPage page,
                                    gfx::NativeWindow parent) {
   DiagnosticsDialog* dialog = new DiagnosticsDialog(page);
+
+  // Ensure log controller configuration matches current session.
+  if (ash::features::IsLogControllerForDiagnosticsAppEnabled()) {
+    ash::diagnostics::DiagnosticsLogController::Get()
+        ->ResetAndInitializeLogWriters();
+  }
+
   dialog->ShowSystemDialog(parent);
 }