Skip to content

Commit

Permalink
Diagnostics: Change location of session log files
Browse files Browse the repository at this point in the history
-  As a result of this change, the routine and network event logs
   are no longer stored in "tmp/diagnostics" and instead, stored in
   a distinct place for each use case (guest mode, multiple users of
   a device).

Bug: 1197335
Change-Id: If134cd6a19bcb6887a47d83ed65c7865476a9d00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3219566
Commit-Queue: Michael Checo <michaelcheco@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/main@{#931106}
  • Loading branch information
Michael Checo authored and Chromium LUCI CQ committed Oct 13, 2021
1 parent 05d7350 commit 082d895
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 13 deletions.
15 changes: 7 additions & 8 deletions ash/webui/diagnostics_ui/backend/session_log_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const char kNetworkingLogSectionHeader[] = "=== Networking === \n";
const char kNoRoutinesRun[] =
"No routines of this type were run in the session.\n";
const char kDefaultSessionLogFileName[] = "session_log.txt";
const char kRoutineLogBasePath[] = "/tmp/diagnostics/";

std::string GetRoutineResultsString(const std::string& results) {
const std::string section_header =
Expand All @@ -47,13 +46,13 @@ std::string GetRoutineResultsString(const std::string& results) {

SessionLogHandler::SessionLogHandler(
const SelectFilePolicyCreator& select_file_policy_creator,
ash::HoldingSpaceClient* holding_space_client)
: SessionLogHandler(
select_file_policy_creator,
std::make_unique<TelemetryLog>(),
std::make_unique<RoutineLog>(base::FilePath(kRoutineLogBasePath)),
std::make_unique<NetworkingLog>(base::FilePath(kRoutineLogBasePath)),
holding_space_client) {}
ash::HoldingSpaceClient* holding_space_client,
const base::FilePath& log_directory_path)
: SessionLogHandler(select_file_policy_creator,
std::make_unique<TelemetryLog>(),
std::make_unique<RoutineLog>(log_directory_path),
std::make_unique<NetworkingLog>(log_directory_path),
holding_space_client) {}

SessionLogHandler::SessionLogHandler(
const SelectFilePolicyCreator& select_file_policy_creator,
Expand Down
3 changes: 2 additions & 1 deletion ash/webui/diagnostics_ui/backend/session_log_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ class SessionLogHandler : public content::WebUIMessageHandler,
base::RepeatingCallback<std::unique_ptr<ui::SelectFilePolicy>(
content::WebContents*)>;
SessionLogHandler(const SelectFilePolicyCreator& select_file_policy_creator,
ash::HoldingSpaceClient* holding_space_client);
ash::HoldingSpaceClient* holding_space_client,
const base::FilePath& log_directory_path);

// Constructor for testing. Should not be called outside of tests.
SessionLogHandler(const SelectFilePolicyCreator& select_file_policy_creator,
Expand Down
6 changes: 4 additions & 2 deletions ash/webui/diagnostics_ui/diagnostics_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "ash/webui/diagnostics_ui/mojom/system_data_provider.mojom.h"
#include "ash/webui/diagnostics_ui/url_constants.h"
#include "base/containers/span.h"
#include "base/files/file_path.h"
#include "base/memory/ptr_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/values.h"
Expand Down Expand Up @@ -328,7 +329,8 @@ DiagnosticsDialogUI::DiagnosticsDialogUI(
content::WebUI* web_ui,
const diagnostics::SessionLogHandler::SelectFilePolicyCreator&
select_file_policy_creator,
HoldingSpaceClient* holding_space_client)
HoldingSpaceClient* holding_space_client,
const base::FilePath& log_directory_path)
: ui::MojoWebDialogUI(web_ui) {
auto html_source = base::WrapUnique(
content::WebUIDataSource::Create(kChromeUIDiagnosticsAppHost));
Expand All @@ -345,7 +347,7 @@ DiagnosticsDialogUI::DiagnosticsDialogUI(
SetUpPluralStringHandler(web_ui);

auto session_log_handler = std::make_unique<diagnostics::SessionLogHandler>(
select_file_policy_creator, holding_space_client);
select_file_policy_creator, holding_space_client, log_directory_path);
diagnostics_manager_ = std::make_unique<diagnostics::DiagnosticsManager>(
session_log_handler.get());
web_ui->AddMessageHandler(std::move(session_log_handler));
Expand Down
7 changes: 6 additions & 1 deletion ash/webui/diagnostics_ui/diagnostics_ui.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "ui/web_dialogs/web_dialog_ui.h"

namespace base {
class FilePath;
} // namespace base

namespace ash {

class HoldingSpaceClient;
Expand All @@ -31,7 +35,8 @@ class DiagnosticsDialogUI : public ui::MojoWebDialogUI {
content::WebUI* web_ui,
const diagnostics::SessionLogHandler::SelectFilePolicyCreator&
select_file_policy_creator,
HoldingSpaceClient* holding_space_client);
HoldingSpaceClient* holding_space_client,
const base::FilePath& log_directory_path);
~DiagnosticsDialogUI() override;

DiagnosticsDialogUI(const DiagnosticsDialogUI&) = delete;
Expand Down
8 changes: 7 additions & 1 deletion chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -492,9 +492,15 @@ WebUIController* NewWebUI<ash::DiagnosticsDialogUI>(WebUI* web_ui,
ash::HoldingSpaceKeyedService* holding_space_keyed_service =
ash::HoldingSpaceKeyedServiceFactory::GetInstance()->GetService(
web_ui->GetWebContents()->GetBrowserContext());
// This directory stores routine and network event logs for a given
// |profile|.
static constexpr base::FilePath::CharType kDiagnosticsLogDirectoryName[] =
FILE_PATH_LITERAL("diagnostics");
return new ash::DiagnosticsDialogUI(
web_ui, base::BindRepeating(&CreateChromeSelectFilePolicy),
holding_space_keyed_service->client());
holding_space_keyed_service->client(),
Profile::FromWebUI(web_ui)->GetPath().Append(
kDiagnosticsLogDirectoryName));
}

void BindMultiDeviceSetup(
Expand Down

0 comments on commit 082d895

Please sign in to comment.