Commit 4a4f9a56 authored by tby's avatar tby Committed by Commit Bot

[Hindsight] Remove teamfood URL logging and wipe existing URLs.

After more investigation of which devices logged URLs will be coming
from, it's unlikely we'll be getting useful signal from the chromebook
itself. So let's disable URL logging altogether.

This CL:
 1. Disables URL navigation logging.
 2. Redacts any existing URLs in saved protos.

Bug: 1012936
Change-Id: Ie96191ea6b0be47f7a2adbb29b80cc3b693f7844
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1947614Reviewed-by: default avatarJia Meng <jiameng@chromium.org>
Commit-Queue: Tony Yeoman <tby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721707}
parent 36a3ff6f
......@@ -4,8 +4,11 @@
#include "chrome/browser/ui/app_list/search/cros_action_history/cros_action_recorder.h"
#include <utility>
#include "ash/public/cpp/app_list/app_list_switches.h"
#include "base/command_line.h"
#include "base/files/file_enumerator.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/files/important_file_writer.h"
......@@ -13,6 +16,7 @@
#include "base/metrics/metrics_hashes.h"
#include "base/no_destructor.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "base/task/post_task.h"
#include "base/threading/scoped_blocking_call.h"
#include "chrome/browser/profiles/profile_manager.h"
......@@ -94,6 +98,45 @@ void SaveToDiskOnWorkerThread(const CrOSActionHistoryProto actions,
}
}
// Redact all URLs in saved protos within |proto_dir| if the urls_redacted file
// doesn't exist.
void RedactURLsInExistingLogs(const base::FilePath proto_dir) {
// If |proto_dir| doesn't exist, no redaction is needed.
if (!base::DirectoryExists(proto_dir))
return;
// Use {proto_dir}/urls_deleted as a flag to indicate whether we've already
// done the redaction.
if (base::PathExists(proto_dir.Append("urls_deleted")))
return;
base::FileEnumerator protos(proto_dir, false, base::FileEnumerator::FILES);
for (base::FilePath proto_path = protos.Next(); !proto_path.empty();
proto_path = protos.Next()) {
std::string proto_str;
if (!base::ReadFileToString(proto_path, &proto_str))
continue;
CrOSActionHistoryProto proto;
if (!proto.ParseFromString(proto_str))
continue;
for (auto& action : *proto.mutable_actions()) {
if (base::StartsWith(action.action_name(), "URLVisited-",
base::CompareCase::SENSITIVE)) {
action.set_action_name("URLVisited-deleted");
}
}
const std::string proto_str_to_write = proto.SerializeAsString();
base::ImportantFileWriter::WriteFileAtomically(
proto_path, proto.SerializeAsString(), "CrOSActionHistory");
}
base::ImportantFileWriter::WriteFileAtomically(
proto_dir.Append("urls_deleted"), "done", "CrOSActionHistory");
}
} // namespace
constexpr char CrOSActionRecorder::kActionHistoryDir[];
......@@ -118,6 +161,11 @@ CrOSActionRecorder::CrOSActionRecorder()
task_runner_ = base::CreateSequencedTaskRunner(
{base::ThreadPool(), base::TaskPriority::BEST_EFFORT, base::MayBlock(),
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
task_runner_->PostTask(
FROM_HERE, base::BindOnce(&RedactURLsInExistingLogs,
profile_path_.Append(
CrOSActionRecorder::kActionHistoryDir)));
}
CrOSActionRecorder::~CrOSActionRecorder() = default;
......
......@@ -30,6 +30,7 @@ class CrOSActionRecorderTest : public testing::Test {
Test::SetUp();
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
profile_path_ = temp_dir_.GetPath();
ASSERT_TRUE(base::IsDirectoryEmpty(profile_path_));
actions_ = {"Action0", "Action1", "Action2"};
conditions_ = {"Condition0", "Condition1", "Condition2"};
......@@ -45,6 +46,7 @@ class CrOSActionRecorderTest : public testing::Test {
CrOSActionRecorder::GetCrosActionRecorder()->should_log_ = false;
CrOSActionRecorder::GetCrosActionRecorder()->should_hash_ = true;
CrOSActionRecorder::GetCrosActionRecorder()->profile_path_ = profile_path_;
Wait();
}
CrOSActionHistoryProto GetCrOSActionHistory() {
......
......@@ -620,17 +620,6 @@ void SearchResultRanker::OnFilesOpened(
}
}
void SearchResultRanker::OnURLVisited(history::HistoryService* history_service,
ui::PageTransition transition,
const history::URLRow& row,
const history::RedirectList& redirects,
base::Time visit_time) {
// TODO(chareszhao): move this outside of SearchResultRanker.
CrOSActionRecorder::GetCrosActionRecorder()->RecordAction(
{base::StrCat({"URLVisited-", row.url().spec()})},
{{"PageTransition", static_cast<int>(transition)}});
}
void SearchResultRanker::OnURLsDeleted(
history::HistoryService* history_service,
const history::DeletionInfo& deletion_info) {
......
......@@ -70,13 +70,6 @@ class SearchResultRanker : file_manager::file_tasks::FileTasksObserver,
// file_manager::file_tasks::FileTaskObserver:
void OnFilesOpened(const std::vector<FileOpenEvent>& file_opens) override;
// history::HistoryService::HistoryServiceObserver:
void OnURLVisited(history::HistoryService* history_service,
ui::PageTransition transition,
const history::URLRow& row,
const history::RedirectList& redirects,
base::Time visit_time) override;
// history::HistoryServiceObserver:
void OnURLsDeleted(history::HistoryService* history_service,
const history::DeletionInfo& deletion_info) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment