Skip to content

Commit

Permalink
[M110] dlp: switch to IO thread to access class members
Browse files Browse the repository at this point in the history 
(cherry picked from commit da543cb)

Bug: 1402113
Change-Id: Iab0201f1f761c34af3973757525cc6f573745787
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4117069
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Commit-Queue: Daniel Brinkers <brinky@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1085416}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4134065
Auto-Submit: Daniel Brinkers <brinky@google.com>
Reviewed-by: Aya Elsayed <ayaelattar@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#122}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Daniel Brinkers authored and Chromium LUCI CQ committed Jan 4, 2023
1 parent d0d66fb commit 55afa35
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 40 deletions.
81 changes: 45 additions & 36 deletions chrome/browser/chromeos/policy/dlp/dlp_copy_or_move_hook_delegate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,16 @@ void GotAccess(base::WeakPtr<DlpCopyOrMoveHookDelegate> hook_delegate,
const storage::FileSystemURL& destination,
DlpCopyOrMoveHookDelegate::StatusCallback callback,
std::unique_ptr<file_access::ScopedFileAccess> access) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
bool is_allowed = access->is_allowed();
if (hook_delegate.MaybeValid()) {
hook_delegate->GotAccess(source, destination, std::move(access));
content::GetIOThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce(&DlpCopyOrMoveHookDelegate::GotAccess, hook_delegate,
source, destination, std::move(access)));
}
// The `callback` was bound to the calling thread in OnBeginProcessFile and
// will be executed on the IO thread.
if (is_allowed) {
std::move(callback).Run(base::File::FILE_OK);
} else {
Expand All @@ -46,6 +52,36 @@ void GotAccess(base::WeakPtr<DlpCopyOrMoveHookDelegate> hook_delegate,
}
#endif

void RequestCopyAccess(base::WeakPtr<DlpCopyOrMoveHookDelegate> hook_delegate,
const storage::FileSystemURL& source,
const storage::FileSystemURL& destination,
DlpCopyOrMoveHookDelegate::StatusCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

// TODO(http://b/259183766): We might need to consider the lacros case,
// too.
#if BUILDFLAG(IS_CHROMEOS_ASH)
DlpRulesManager* dlp_rules_manager =
DlpRulesManagerFactory::GetForPrimaryProfile();
if (!dlp_rules_manager) {
std::move(callback).Run(base::File::FILE_OK);
return;
}
DlpFilesController* dlp_files_controller =
dlp_rules_manager->GetDlpFilesController();
if (!dlp_files_controller) {
std::move(callback).Run(base::File::FILE_OK);
return;
}
dlp_files_controller->RequestCopyAccess(
source, destination,
base::BindOnce(&policy::GotAccess, hook_delegate, source, destination,
std::move(callback)));
#else
NOTREACHED();
#endif
}

} // namespace

DlpCopyOrMoveHookDelegate::DlpCopyOrMoveHookDelegate(bool isComposite)
Expand All @@ -57,6 +93,7 @@ void DlpCopyOrMoveHookDelegate::GotAccess(
const storage::FileSystemURL& source,
const storage::FileSystemURL& destination,
std::unique_ptr<file_access::ScopedFileAccess> access) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
current_access_map_.emplace(std::make_pair(source.path(), destination.path()),
std::move(access));
}
Expand All @@ -65,12 +102,15 @@ void DlpCopyOrMoveHookDelegate::OnBeginProcessFile(
const storage::FileSystemURL& source_url,
const storage::FileSystemURL& destination_url,
StatusCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
StatusCallback continuation = base::BindPostTask(
base::SequencedTaskRunner::GetCurrentDefault(), std::move(callback));
content::GetUIThreadTaskRunner({})->PostTask(
FROM_HERE, base::BindOnce(&DlpCopyOrMoveHookDelegate::RequestCopyAccess,
base::Unretained(this), source_url,
destination_url, std::move(continuation)));
FROM_HERE,
base::BindOnce(
&RequestCopyAccess,
base::SupportsWeakPtr<DlpCopyOrMoveHookDelegate>::AsWeakPtr(),
source_url, destination_url, std::move(continuation)));
}

void DlpCopyOrMoveHookDelegate::OnEndCopy(
Expand All @@ -95,40 +135,9 @@ void DlpCopyOrMoveHookDelegate::OnError(
void DlpCopyOrMoveHookDelegate::OnEnd(
const storage::FileSystemURL& source_url,
const storage::FileSystemURL& destination_url) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
current_access_map_.erase(
std::make_pair(source_url.path(), destination_url.path()));
}

void DlpCopyOrMoveHookDelegate::RequestCopyAccess(
const storage::FileSystemURL& source,
const storage::FileSystemURL& destination,
DlpCopyOrMoveHookDelegate::StatusCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

// TODO(http://b/259183766): We might need to consider the lacros case,
// too.
#if BUILDFLAG(IS_CHROMEOS_ASH)
DlpRulesManager* dlp_rules_manager =
DlpRulesManagerFactory::GetForPrimaryProfile();
if (!dlp_rules_manager) {
std::move(callback).Run(base::File::FILE_OK);
return;
}
DlpFilesController* dlp_files_controller =
dlp_rules_manager->GetDlpFilesController();
if (!dlp_files_controller) {
std::move(callback).Run(base::File::FILE_OK);
return;
}
dlp_files_controller->RequestCopyAccess(
source, destination,
base::BindOnce(
&policy::GotAccess,
base::SupportsWeakPtr<DlpCopyOrMoveHookDelegate>::AsWeakPtr(), source,
destination, std::move(callback)));
#else
NOTREACHED();
#endif
}

} // namespace policy
4 changes: 0 additions & 4 deletions chrome/browser/chromeos/policy/dlp/dlp_copy_or_move_hook_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ class DlpCopyOrMoveHookDelegate
void OnEnd(const storage::FileSystemURL& source_url,
const storage::FileSystemURL& destination_url);

void RequestCopyAccess(const storage::FileSystemURL& source,
const storage::FileSystemURL& destination,
DlpCopyOrMoveHookDelegate::StatusCallback callback);

friend DlpCopyOrMoveHookDelegateTest;

// Keeps the ScopedFileAccess object for the whole copy operation between the
Expand Down
41 changes: 41 additions & 0 deletions chrome/browser/chromeos/policy/dlp/dlp_copy_or_move_hook_delegate_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,47 @@ TEST_F(DlpCopyOrMoveHookDelegateTest, OnBeginProcessFileDeny) {
status_callback_run_loop.Run();
}

TEST_F(DlpCopyOrMoveHookDelegateTest, OnBeginProcessFileAllowHookDestruct) {
base::RunLoop hook_destruction_run_loop;
base::RunLoop continuation_run_loop;
base::RunLoop status_callback_run_loop;
base::MockCallback<base::OnceCallback<void()>> destructor_continuation;
EXPECT_CALL(destructor_continuation, Run)
.WillOnce([&continuation_run_loop]() { continuation_run_loop.Quit(); });
EXPECT_CALL(*manager_, GetDlpFilesController)
.WillOnce(testing::Return(controller_.get()));

EXPECT_CALL(*controller_, RequestCopyAccess(source, destination,
base::test::IsNotNullCallback()))
.WillOnce(
[&](const storage::FileSystemURL& source,
const storage::FileSystemURL& destination,
base::OnceCallback<void(
std::unique_ptr<file_access::ScopedFileAccess>)> callback) {
hook_.reset();
hook_destruction_run_loop.Quit();
std::move(callback).Run(
std::make_unique<file_access::ScopedFileAccessCopy>(
true, base::ScopedFD(), destructor_continuation.Get()));
});

auto task_runner = content::GetIOThreadTaskRunner({});
base::MockCallback<base::OnceCallback<void(base::File::Error)>> status;
EXPECT_CALL(status, Run)
.WillOnce([&status_callback_run_loop](base::File::Error status) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
EXPECT_EQ(base::File::FILE_OK, status);
status_callback_run_loop.Quit();
});
task_runner->PostTask(
FROM_HERE, base::BindOnce(&DlpCopyOrMoveHookDelegate::OnBeginProcessFile,
base::Unretained(hook_.get()), source,
destination, status.Get()));
hook_destruction_run_loop.Run();
status_callback_run_loop.Run();
continuation_run_loop.Run();
}

TEST_F(DlpCopyOrMoveHookDelegateTest, OnBeginProcessFileNoManager) {
policy::DlpRulesManagerFactory::GetInstance()->SetTestingFactory(
profile_.get(),
Expand Down

0 comments on commit 55afa35

Please sign in to comment.