Commit 1e9598fa authored by Eric Willigers's avatar Eric Willigers Committed by Chromium LUCI CQ

Web Share for Chrome OS: Delete shared files after delay

To avoid the accumulation of shared files on Chromebooks that
are restarted infrequently, we delete shared files 10 minutes
after the share dialog completes.


Design doc:
https://docs.google.com/document/d/1EjpgseTbBhT9ogQv6HV6sLlBl4hpb2viOcmPtFJorMQ/


Bug: 1153215
Change-Id: Iff221b79b7e6d9e92e47081bdea4ebbf30f37776
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2581704
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Auto-Submit: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: default avatarAlexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835964}
parent adedb829
...@@ -34,6 +34,7 @@ source_set("unit_tests") { ...@@ -34,6 +34,7 @@ source_set("unit_tests") {
"//base/test:test_support", "//base/test:test_support",
"//build:chromeos_buildflags", "//build:chromeos_buildflags",
"//chrome/browser", "//chrome/browser",
"//chrome/browser/chromeos:chromeos",
"//chrome/common", "//chrome/common",
"//chrome/test:test_support", "//chrome/test:test_support",
"//content/public/browser", "//content/public/browser",
......
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
#include "base/files/file.h" #include "base/files/file.h"
#include "base/files/file_enumerator.h" #include "base/files/file_enumerator.h"
#include "base/files/file_util.h"
#include "base/system/sys_info.h" #include "base/system/sys_info.h"
#include "base/task/task_traits.h" #include "base/task/task_traits.h"
#include "base/task/thread_pool.h" #include "base/task/thread_pool.h"
...@@ -16,6 +15,18 @@ ...@@ -16,6 +15,18 @@
using content::BrowserThread; using content::BrowserThread;
namespace {
void DeleteSharedFiles(std::vector<base::FilePath> file_paths) {
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::WILL_BLOCK);
for (const base::FilePath& name : file_paths) {
base::DeleteFile(name);
}
}
} // namespace
namespace webshare { namespace webshare {
constexpr base::TimeDelta PrepareDirectoryTask::kSharedFileLifetime; constexpr base::TimeDelta PrepareDirectoryTask::kSharedFileLifetime;
...@@ -40,6 +51,15 @@ void PrepareDirectoryTask::Start() { ...@@ -40,6 +51,15 @@ void PrepareDirectoryTask::Start() {
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
} }
// static
void PrepareDirectoryTask::ScheduleSharedFileDeletion(
std::vector<base::FilePath> file_paths,
base::TimeDelta delay) {
base::ThreadPool::PostDelayedTask(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&DeleteSharedFiles, std::move(file_paths)), delay);
}
// static // static
base::File::Error PrepareDirectoryTask::PrepareDirectory( base::File::Error PrepareDirectoryTask::PrepareDirectory(
base::FilePath directory, base::FilePath directory,
......
...@@ -27,6 +27,10 @@ class PrepareDirectoryTask { ...@@ -27,6 +27,10 @@ class PrepareDirectoryTask {
PrepareDirectoryTask& operator=(const PrepareDirectoryTask&) = delete; PrepareDirectoryTask& operator=(const PrepareDirectoryTask&) = delete;
~PrepareDirectoryTask(); ~PrepareDirectoryTask();
// Deletes specified |file_paths| after waiting |delay|.
static void ScheduleSharedFileDeletion(std::vector<base::FilePath> file_paths,
base::TimeDelta delay);
// Launches the task. |callback_| will be called on the original (UI) thread // Launches the task. |callback_| will be called on the original (UI) thread
// when the task completes. // when the task completes.
void Start(); void Start();
......
...@@ -157,6 +157,8 @@ void SharesheetClient::SetSharesheetCallbackForTesting( ...@@ -157,6 +157,8 @@ void SharesheetClient::SetSharesheetCallbackForTesting(
void SharesheetClient::OnPrepareDirectory(blink::mojom::ShareError error) { void SharesheetClient::OnPrepareDirectory(blink::mojom::ShareError error) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (!current_share_.has_value())
return;
if (!web_contents() || error != blink::mojom::ShareError::OK) { if (!web_contents() || error != blink::mojom::ShareError::OK) {
std::move(current_share_->callback).Run(error); std::move(current_share_->callback).Run(error);
...@@ -183,9 +185,13 @@ void SharesheetClient::OnPrepareDirectory(blink::mojom::ShareError error) { ...@@ -183,9 +185,13 @@ void SharesheetClient::OnPrepareDirectory(blink::mojom::ShareError error) {
void SharesheetClient::OnStoreFiles(blink::mojom::ShareError error) { void SharesheetClient::OnStoreFiles(blink::mojom::ShareError error) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (!current_share_.has_value())
return;
if (!web_contents() || error != blink::mojom::ShareError::OK) { if (!web_contents() || error != blink::mojom::ShareError::OK) {
std::move(current_share_->callback).Run(error); std::move(current_share_->callback).Run(error);
PrepareDirectoryTask::ScheduleSharedFileDeletion(
std::move(current_share_->file_paths), base::TimeDelta::FromMinutes(0));
current_share_ = base::nullopt; current_share_ = base::nullopt;
return; return;
} }
...@@ -198,7 +204,13 @@ void SharesheetClient::OnStoreFiles(blink::mojom::ShareError error) { ...@@ -198,7 +204,13 @@ void SharesheetClient::OnStoreFiles(blink::mojom::ShareError error) {
} }
void SharesheetClient::OnShowSharesheet(sharesheet::SharesheetResult result) { void SharesheetClient::OnShowSharesheet(sharesheet::SharesheetResult result) {
if (!current_share_.has_value())
return;
std::move(current_share_->callback).Run(SharesheetResultToShareError(result)); std::move(current_share_->callback).Run(SharesheetResultToShareError(result));
PrepareDirectoryTask::ScheduleSharedFileDeletion(
std::move(current_share_->file_paths),
PrepareDirectoryTask::kSharedFileLifetime);
current_share_ = base::nullopt; current_share_ = base::nullopt;
} }
......
...@@ -9,12 +9,16 @@ ...@@ -9,12 +9,16 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/bind.h" #include "base/test/bind.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "chrome/browser/chromeos/file_manager/path_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sharesheet/sharesheet_types.h" #include "chrome/browser/sharesheet/sharesheet_types.h"
#include "chrome/browser/webshare/chromeos/prepare_directory_task.h"
#include "chrome/browser/webshare/chromeos/store_file_task.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "content/public/browser/site_instance.h" #include "content/public/browser/site_instance.h"
#include "content/public/test/web_contents_tester.h" #include "content/public/test/web_contents_tester.h"
...@@ -103,4 +107,47 @@ TEST_F(SharesheetClientUnitTest, TestWithoutFilesInIncognito) { ...@@ -103,4 +107,47 @@ TEST_F(SharesheetClientUnitTest, TestWithoutFilesInIncognito) {
EXPECT_EQ(error, blink::mojom::ShareError::OK); EXPECT_EQ(error, blink::mojom::ShareError::OK);
} }
TEST_F(SharesheetClientUnitTest, DeleteAfterShare) {
SharesheetClient sharesheet_client(web_contents());
const base::FilePath my_files =
file_manager::util::GetMyFilesFolderForProfile(profile());
const base::FilePath first_file =
my_files.AppendASCII(".WebShare/share1.txt");
const base::FilePath second_file =
my_files.AppendASCII(".WebShare/share2.txt");
const std::string title = "Subject";
const std::string text = "Message";
const GURL share_url("https://example.com/");
std::vector<blink::mojom::SharedFilePtr> files;
files.push_back(blink::mojom::SharedFile::New(
first_file.AsUTF8Unsafe(), blink::mojom::SerializedBlob::New()));
files.push_back(blink::mojom::SharedFile::New(
second_file.AsUTF8Unsafe(), blink::mojom::SerializedBlob::New()));
base::RunLoop run_loop;
blink::mojom::ShareError error = blink::mojom::ShareError::INTERNAL_ERROR;
StoreFileTask::SkipCopyingForTesting();
sharesheet_client.Share(
title, text, share_url, std::move(files),
base::BindLambdaForTesting(
[&run_loop, &error](blink::mojom::ShareError in_error) {
error = in_error;
run_loop.Quit();
}));
run_loop.Run();
EXPECT_EQ(error, blink::mojom::ShareError::OK);
task_environment()->FastForwardBy(PrepareDirectoryTask::kSharedFileLifetime /
2);
EXPECT_TRUE(base::PathExists(first_file));
EXPECT_TRUE(base::PathExists(second_file));
task_environment()->FastForwardBy(PrepareDirectoryTask::kSharedFileLifetime *
2);
EXPECT_FALSE(base::PathExists(first_file));
EXPECT_FALSE(base::PathExists(second_file));
}
} // namespace webshare } // namespace webshare
...@@ -27,6 +27,17 @@ StoreFileTask::StoreFileTask(base::FilePath filename, ...@@ -27,6 +27,17 @@ StoreFileTask::StoreFileTask(base::FilePath filename,
StoreFileTask::~StoreFileTask() = default; StoreFileTask::~StoreFileTask() = default;
void StoreFileTask::Start() { void StoreFileTask::Start() {
{
base::ScopedBlockingCall scoped_blocking_call(
FROM_HERE, base::BlockingType::WILL_BLOCK);
output_file_.Initialize(
filename_, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
}
(this->*GetStartReadFunc())();
}
void StoreFileTask::StartRead() {
constexpr size_t kDataPipeCapacity = 65536; constexpr size_t kDataPipeCapacity = 65536;
MojoCreateDataPipeOptions options; MojoCreateDataPipeOptions options;
...@@ -42,18 +53,16 @@ void StoreFileTask::Start() { ...@@ -42,18 +53,16 @@ void StoreFileTask::Start() {
return; return;
} }
{
base::ScopedBlockingCall scoped_blocking_call(
FROM_HERE, base::BlockingType::WILL_BLOCK);
output_file_.Initialize(
filename_, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
}
mojo::Remote<blink::mojom::Blob> blob(std::move(file_->blob->blob)); mojo::Remote<blink::mojom::Blob> blob(std::move(file_->blob->blob));
blob->ReadAll(std::move(producer_handle), blob->ReadAll(std::move(producer_handle),
receiver_.BindNewPipeAndPassRemote()); receiver_.BindNewPipeAndPassRemote());
} }
// static
void StoreFileTask::SkipCopyingForTesting() {
GetStartReadFunc() = &StoreFileTask::OnSuccess;
}
void StoreFileTask::OnDataPipeReadable(MojoResult result) { void StoreFileTask::OnDataPipeReadable(MojoResult result) {
if (result != MOJO_RESULT_OK) { if (result != MOJO_RESULT_OK) {
if (!received_all_data_) { if (!received_all_data_) {
...@@ -144,4 +153,11 @@ void StoreFileTask::OnComplete(int32_t status, uint64_t data_length) { ...@@ -144,4 +153,11 @@ void StoreFileTask::OnComplete(int32_t status, uint64_t data_length) {
OnSuccess(); OnSuccess();
} }
// static
StoreFileTask::StartReadFunc& StoreFileTask::GetStartReadFunc() {
static StartReadFunc start_read_func = &StoreFileTask::StartRead;
return start_read_func;
}
} // namespace webshare } // namespace webshare
...@@ -31,7 +31,13 @@ class StoreFileTask : public blink::mojom::BlobReaderClient { ...@@ -31,7 +31,13 @@ class StoreFileTask : public blink::mojom::BlobReaderClient {
// Must be called on a thread that allows blocking IO. // Must be called on a thread that allows blocking IO.
void Start(); void Start();
// Create empty files instead of copying from the SharedFilePtr.
static void SkipCopyingForTesting();
private: private:
using StartReadFunc = void (StoreFileTask::*)();
void StartRead();
void OnDataPipeReadable(MojoResult result); void OnDataPipeReadable(MojoResult result);
void OnSuccess(); void OnSuccess();
...@@ -40,6 +46,8 @@ class StoreFileTask : public blink::mojom::BlobReaderClient { ...@@ -40,6 +46,8 @@ class StoreFileTask : public blink::mojom::BlobReaderClient {
uint64_t expected_content_size) override; uint64_t expected_content_size) override;
void OnComplete(int32_t status, uint64_t data_length) override; void OnComplete(int32_t status, uint64_t data_length) override;
static StartReadFunc& GetStartReadFunc();
base::FilePath filename_; base::FilePath filename_;
blink::mojom::SharedFilePtr file_; blink::mojom::SharedFilePtr file_;
uint64_t& available_space_; uint64_t& available_space_;
......
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