Commit ed9a49bc authored by Marijn Kruisselbrink's avatar Marijn Kruisselbrink Committed by Commit Bot

[FS] Refactor how directory read access is granted.

This gets rid of the redundant ConfirmDirectoryReadAccess method,
and instead just requests read access on the corresponding permission
grant instead. In the future this will make it easier to support a
single prompt for getting access to a writable directory, instead
of the current two separate prompts.

To do this, also adds an extra flag to the
NFSPermissionGrant::RequestPermission method to indicate if this
particular permission requests requires user activation.

Bug: 1115632
Change-Id: Ie3980867d162e16723569a0fd36f27323d2700ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2341214Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarVictor Costan <pwnall@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797406}
parent bb830832
...@@ -61,13 +61,6 @@ class TestNativeFileSystemPermissionContext ...@@ -61,13 +61,6 @@ class TestNativeFileSystemPermissionContext
NOTREACHED(); NOTREACHED();
return nullptr; return nullptr;
} }
void ConfirmDirectoryReadAccess(
const url::Origin& origin,
const base::FilePath& path,
content::GlobalFrameRoutingId frame_id,
base::OnceCallback<void(PermissionStatus)> callback) override {
NOTREACHED();
}
// ChromeNativeFileSystemPermissionContext: // ChromeNativeFileSystemPermissionContext:
Grants GetPermissionGrants(const url::Origin& origin) override { Grants GetPermissionGrants(const url::Origin& origin) override {
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#endif #endif
namespace { namespace {
using blink::mojom::PermissionStatus;
using permissions::PermissionAction; using permissions::PermissionAction;
enum class GrantType { kRead, kWrite }; enum class GrantType { kRead, kWrite };
...@@ -52,15 +53,8 @@ class OriginScopedNativeFileSystemPermissionContext::PermissionGrantImpl ...@@ -52,15 +53,8 @@ class OriginScopedNativeFileSystemPermissionContext::PermissionGrantImpl
PermissionStatus GetStatus() override { return status_; } PermissionStatus GetStatus() override { return status_; }
void RequestPermission( void RequestPermission(
content::GlobalFrameRoutingId frame_id, content::GlobalFrameRoutingId frame_id,
UserActivationState user_activation_state,
base::OnceCallback<void(PermissionRequestOutcome)> callback) override { base::OnceCallback<void(PermissionRequestOutcome)> callback) override {
RequestPermissionImpl(frame_id,
/*require_user_gesture=*/true, std::move(callback));
}
void RequestPermissionImpl(
content::GlobalFrameRoutingId frame_id,
bool require_user_gesture,
base::OnceCallback<void(PermissionRequestOutcome)> callback) {
// Check if a permission request has already been processed previously. This // Check if a permission request has already been processed previously. This
// check is done first because we don't want to reset the status of a // check is done first because we don't want to reset the status of a
// permission if it has already been granted. // permission if it has already been granted.
...@@ -102,7 +96,8 @@ class OriginScopedNativeFileSystemPermissionContext::PermissionGrantImpl ...@@ -102,7 +96,8 @@ class OriginScopedNativeFileSystemPermissionContext::PermissionGrantImpl
return; return;
} }
if (require_user_gesture && !rfh->HasTransientUserActivation()) { if (user_activation_state == UserActivationState::kRequired &&
!rfh->HasTransientUserActivation()) {
// No permission prompts without user activation. // No permission prompts without user activation.
RunCallbackAndRecordPermissionRequestOutcome( RunCallbackAndRecordPermissionRequestOutcome(
std::move(callback), PermissionRequestOutcome::kNoUserActivation); std::move(callback), PermissionRequestOutcome::kNoUserActivation);
...@@ -416,30 +411,6 @@ OriginScopedNativeFileSystemPermissionContext::GetWritePermissionGrant( ...@@ -416,30 +411,6 @@ OriginScopedNativeFileSystemPermissionContext::GetWritePermissionGrant(
return existing_grant; return existing_grant;
} }
void OriginScopedNativeFileSystemPermissionContext::ConfirmDirectoryReadAccess(
const url::Origin& origin,
const base::FilePath& path,
content::GlobalFrameRoutingId frame_id,
base::OnceCallback<void(PermissionStatus)> callback) {
// TODO(mek): Once tab-scoped permission model is no longer used we can
// refactor the calling code of this method to just do what this
// implementation does directly.
scoped_refptr<content::NativeFileSystemPermissionGrant> grant =
GetReadPermissionGrant(origin, path, HandleType::kDirectory,
UserAction::kOpen);
static_cast<PermissionGrantImpl*>(grant.get())
->RequestPermissionImpl(
frame_id, /*require_user_gesture=*/false,
base::BindOnce(
[](base::OnceCallback<void(PermissionStatus)> callback,
scoped_refptr<content::NativeFileSystemPermissionGrant> grant,
content::NativeFileSystemPermissionGrant::
PermissionRequestOutcome outcome) {
std::move(callback).Run(grant->GetStatus());
},
std::move(callback), grant));
}
ChromeNativeFileSystemPermissionContext::Grants ChromeNativeFileSystemPermissionContext::Grants
OriginScopedNativeFileSystemPermissionContext::GetPermissionGrants( OriginScopedNativeFileSystemPermissionContext::GetPermissionGrants(
const url::Origin& origin) { const url::Origin& origin) {
......
...@@ -35,11 +35,6 @@ class OriginScopedNativeFileSystemPermissionContext ...@@ -35,11 +35,6 @@ class OriginScopedNativeFileSystemPermissionContext
const base::FilePath& path, const base::FilePath& path,
HandleType handle_type, HandleType handle_type,
UserAction user_action) override; UserAction user_action) override;
void ConfirmDirectoryReadAccess(
const url::Origin& origin,
const base::FilePath& path,
content::GlobalFrameRoutingId frame_id,
base::OnceCallback<void(PermissionStatus)> callback) override;
// ChromeNativeFileSystemPermissionContext: // ChromeNativeFileSystemPermissionContext:
Grants GetPermissionGrants(const url::Origin& origin) override; Grants GetPermissionGrants(const url::Origin& origin) override;
......
...@@ -42,6 +42,8 @@ using PermissionRequestOutcome = ...@@ -42,6 +42,8 @@ using PermissionRequestOutcome =
using SensitiveDirectoryResult = using SensitiveDirectoryResult =
ChromeNativeFileSystemPermissionContext::SensitiveDirectoryResult; ChromeNativeFileSystemPermissionContext::SensitiveDirectoryResult;
using HandleType = content::NativeFileSystemPermissionContext::HandleType; using HandleType = content::NativeFileSystemPermissionContext::HandleType;
using UserActivationState =
content::NativeFileSystemPermissionGrant::UserActivationState;
class OriginScopedNativeFileSystemPermissionContextTest : public testing::Test { class OriginScopedNativeFileSystemPermissionContextTest : public testing::Test {
public: public:
...@@ -318,7 +320,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -318,7 +320,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop; base::RunLoop loop;
grant->RequestPermission( grant->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kUserDismissed, outcome); EXPECT_EQ(PermissionRequestOutcome::kUserDismissed, outcome);
loop.Quit(); loop.Quit();
...@@ -340,7 +342,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -340,7 +342,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop; base::RunLoop loop;
grant->RequestPermission( grant->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kUserGranted, outcome); EXPECT_EQ(PermissionRequestOutcome::kUserGranted, outcome);
loop.Quit(); loop.Quit();
...@@ -361,7 +363,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -361,7 +363,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop; base::RunLoop loop;
grant->RequestPermission( grant->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kUserDenied, outcome); EXPECT_EQ(PermissionRequestOutcome::kUserDenied, outcome);
loop.Quit(); loop.Quit();
...@@ -380,7 +382,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -380,7 +382,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop; base::RunLoop loop;
grant->RequestPermission( grant->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kNoUserActivation, outcome); EXPECT_EQ(PermissionRequestOutcome::kNoUserActivation, outcome);
loop.Quit(); loop.Quit();
...@@ -390,6 +392,26 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -390,6 +392,26 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
EXPECT_EQ(PermissionStatus::ASK, grant->GetStatus()); EXPECT_EQ(PermissionStatus::ASK, grant->GetStatus());
} }
TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
RequestPermission_NoUserActivation_UserActivationNotRequired) {
NativeFileSystemPermissionRequestManager::FromWebContents(web_contents_.get())
->set_auto_response_for_test(PermissionAction::GRANTED);
auto grant = permission_context()->GetWritePermissionGrant(
kTestOrigin, kTestPath, HandleType::kFile, UserAction::kOpen);
base::RunLoop loop;
grant->RequestPermission(
frame_id(), UserActivationState::kNotRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kUserGranted, outcome);
loop.Quit();
}));
loop.Run();
// No user activation, so status should not change.
EXPECT_EQ(PermissionStatus::GRANTED, grant->GetStatus());
}
TEST_F(OriginScopedNativeFileSystemPermissionContextTest, TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
RequestPermission_AlreadyGranted) { RequestPermission_AlreadyGranted) {
// If the permission has already been granted, a call to RequestPermission() // If the permission has already been granted, a call to RequestPermission()
...@@ -400,7 +422,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -400,7 +422,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop; base::RunLoop loop;
grant->RequestPermission( grant->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kRequestAborted, outcome); EXPECT_EQ(PermissionRequestOutcome::kRequestAborted, outcome);
loop.Quit(); loop.Quit();
...@@ -422,7 +444,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -422,7 +444,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop; base::RunLoop loop;
grant->RequestPermission( grant->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kRequestAborted, outcome); EXPECT_EQ(PermissionRequestOutcome::kRequestAborted, outcome);
loop.Quit(); loop.Quit();
...@@ -435,7 +457,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -435,7 +457,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop2; base::RunLoop loop2;
grant2->RequestPermission( grant2->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kRequestAborted, outcome); EXPECT_EQ(PermissionRequestOutcome::kRequestAborted, outcome);
loop2.Quit(); loop2.Quit();
...@@ -453,7 +475,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -453,7 +475,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop3; base::RunLoop loop3;
grant2->RequestPermission( grant2->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kNoUserActivation, outcome); EXPECT_EQ(PermissionRequestOutcome::kNoUserActivation, outcome);
loop3.Quit(); loop3.Quit();
...@@ -477,7 +499,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -477,7 +499,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop; base::RunLoop loop;
grant->RequestPermission( grant->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kBlockedByContentSetting, outcome); EXPECT_EQ(PermissionRequestOutcome::kBlockedByContentSetting, outcome);
loop.Quit(); loop.Quit();
...@@ -487,7 +509,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -487,7 +509,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop2; base::RunLoop loop2;
grant2->RequestPermission( grant2->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kBlockedByContentSetting, outcome); EXPECT_EQ(PermissionRequestOutcome::kBlockedByContentSetting, outcome);
loop2.Quit(); loop2.Quit();
...@@ -508,7 +530,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -508,7 +530,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop3; base::RunLoop loop3;
grant->RequestPermission( grant->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kNoUserActivation, outcome); EXPECT_EQ(PermissionRequestOutcome::kNoUserActivation, outcome);
loop3.Quit(); loop3.Quit();
...@@ -518,7 +540,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest, ...@@ -518,7 +540,7 @@ TEST_F(OriginScopedNativeFileSystemPermissionContextTest,
base::RunLoop loop4; base::RunLoop loop4;
grant2->RequestPermission( grant2->RequestPermission(
frame_id(), frame_id(), UserActivationState::kRequired,
base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) { base::BindLambdaForTesting([&](PermissionRequestOutcome outcome) {
EXPECT_EQ(PermissionRequestOutcome::kRequestAborted, outcome); EXPECT_EQ(PermissionRequestOutcome::kRequestAborted, outcome);
loop4.Quit(); loop4.Quit();
......
...@@ -9,7 +9,9 @@ ...@@ -9,7 +9,9 @@
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "content/browser/native_file_system/file_system_chooser_test_helpers.h" #include "content/browser/native_file_system/file_system_chooser_test_helpers.h"
#include "content/browser/native_file_system/fixed_native_file_system_permission_grant.h"
#include "content/browser/native_file_system/mock_native_file_system_permission_context.h" #include "content/browser/native_file_system/mock_native_file_system_permission_context.h"
#include "content/browser/native_file_system/mock_native_file_system_permission_grant.h"
#include "content/browser/native_file_system/native_file_system_manager_impl.h" #include "content/browser/native_file_system/native_file_system_manager_impl.h"
#include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
...@@ -367,21 +369,43 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, OpenDirectory_DenyAccess) { ...@@ -367,21 +369,43 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, OpenDirectory_DenyAccess) {
->GetNativeFileSystemEntryFactory()) ->GetNativeFileSystemEntryFactory())
->SetPermissionContextForTesting(&permission_context); ->SetPermissionContextForTesting(&permission_context);
auto read_grant = base::MakeRefCounted<
testing::StrictMock<MockNativeFileSystemPermissionGrant>>();
auto write_grant = base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
PermissionStatus::ASK);
EXPECT_CALL(permission_context, EXPECT_CALL(permission_context,
ConfirmSensitiveDirectoryAccess_( ConfirmSensitiveDirectoryAccess_(
testing::_, testing::_, testing::_, testing::_, testing::_)) testing::_, testing::_, testing::_, testing::_, testing::_))
.WillOnce(RunOnceCallback<4>(SensitiveDirectoryResult::kAllowed)); .WillOnce(RunOnceCallback<4>(SensitiveDirectoryResult::kAllowed));
auto origin =
url::Origin::Create(embedded_test_server()->GetURL("/title1.html"));
EXPECT_CALL(permission_context,
GetReadPermissionGrant(
origin, test_dir,
NativeFileSystemPermissionContext::HandleType::kDirectory,
NativeFileSystemPermissionContext::UserAction::kOpen))
.WillOnce(testing::Return(read_grant));
EXPECT_CALL(permission_context,
GetWritePermissionGrant(
origin, test_dir,
NativeFileSystemPermissionContext::HandleType::kDirectory,
NativeFileSystemPermissionContext::UserAction::kOpen))
.WillOnce(testing::Return(write_grant));
EXPECT_CALL( EXPECT_CALL(
permission_context, *read_grant,
ConfirmDirectoryReadAccess_( RequestPermission_(
url::Origin::Create(embedded_test_server()->GetURL("/title1.html")),
test_dir,
GlobalFrameRoutingId( GlobalFrameRoutingId(
shell()->web_contents()->GetMainFrame()->GetProcess()->GetID(), shell()->web_contents()->GetMainFrame()->GetProcess()->GetID(),
shell()->web_contents()->GetMainFrame()->GetRoutingID()), shell()->web_contents()->GetMainFrame()->GetRoutingID()),
NativeFileSystemPermissionGrant::UserActivationState::kNotRequired,
testing::_)) testing::_))
.WillOnce(RunOnceCallback<3>(PermissionStatus::DENIED)); .WillOnce(RunOnceCallback<2>(NativeFileSystemPermissionGrant::
PermissionRequestOutcome::kUserDenied));
EXPECT_CALL(*read_grant, GetStatus())
.WillRepeatedly(testing::Return(PermissionStatus::ASK));
ASSERT_TRUE( ASSERT_TRUE(
NavigateToURL(shell(), embedded_test_server()->GetURL("/title1.html"))); NavigateToURL(shell(), embedded_test_server()->GetURL("/title1.html")));
......
...@@ -20,6 +20,7 @@ FixedNativeFileSystemPermissionGrant::GetStatus() { ...@@ -20,6 +20,7 @@ FixedNativeFileSystemPermissionGrant::GetStatus() {
void FixedNativeFileSystemPermissionGrant::RequestPermission( void FixedNativeFileSystemPermissionGrant::RequestPermission(
GlobalFrameRoutingId frame_id, GlobalFrameRoutingId frame_id,
UserActivationState user_activation_state,
base::OnceCallback<void(PermissionRequestOutcome)> callback) { base::OnceCallback<void(PermissionRequestOutcome)> callback) {
std::move(callback).Run(PermissionRequestOutcome::kRequestAborted); std::move(callback).Run(PermissionRequestOutcome::kRequestAborted);
} }
......
...@@ -25,6 +25,7 @@ class CONTENT_EXPORT FixedNativeFileSystemPermissionGrant ...@@ -25,6 +25,7 @@ class CONTENT_EXPORT FixedNativeFileSystemPermissionGrant
PermissionStatus GetStatus() override; PermissionStatus GetStatus() override;
void RequestPermission( void RequestPermission(
GlobalFrameRoutingId frame_id, GlobalFrameRoutingId frame_id,
UserActivationState user_activation_state,
base::OnceCallback<void(PermissionRequestOutcome)> callback) override; base::OnceCallback<void(PermissionRequestOutcome)> callback) override;
protected: protected:
......
...@@ -11,14 +11,6 @@ MockNativeFileSystemPermissionContext::MockNativeFileSystemPermissionContext() = ...@@ -11,14 +11,6 @@ MockNativeFileSystemPermissionContext::MockNativeFileSystemPermissionContext() =
MockNativeFileSystemPermissionContext:: MockNativeFileSystemPermissionContext::
~MockNativeFileSystemPermissionContext() = default; ~MockNativeFileSystemPermissionContext() = default;
void MockNativeFileSystemPermissionContext::ConfirmDirectoryReadAccess(
const url::Origin& origin,
const base::FilePath& path,
GlobalFrameRoutingId frame_id,
base::OnceCallback<void(PermissionStatus)> callback) {
ConfirmDirectoryReadAccess_(origin, path, frame_id, callback);
}
void MockNativeFileSystemPermissionContext::ConfirmSensitiveDirectoryAccess( void MockNativeFileSystemPermissionContext::ConfirmSensitiveDirectoryAccess(
const url::Origin& origin, const url::Origin& origin,
const std::vector<base::FilePath>& paths, const std::vector<base::FilePath>& paths,
......
...@@ -21,7 +21,6 @@ class MockNativeFileSystemPermissionContext ...@@ -21,7 +21,6 @@ class MockNativeFileSystemPermissionContext
const url::Origin& origin, const url::Origin& origin,
const base::FilePath& path, const base::FilePath& path,
HandleType handle_type, HandleType handle_type,
NativeFileSystemPermissionContext::UserAction user_action)); NativeFileSystemPermissionContext::UserAction user_action));
MOCK_METHOD4(GetWritePermissionGrant, MOCK_METHOD4(GetWritePermissionGrant,
...@@ -29,20 +28,8 @@ class MockNativeFileSystemPermissionContext ...@@ -29,20 +28,8 @@ class MockNativeFileSystemPermissionContext
const url::Origin& origin, const url::Origin& origin,
const base::FilePath& path, const base::FilePath& path,
HandleType handle_type, HandleType handle_type,
NativeFileSystemPermissionContext::UserAction user_action)); NativeFileSystemPermissionContext::UserAction user_action));
void ConfirmDirectoryReadAccess(
const url::Origin& origin,
const base::FilePath& path,
GlobalFrameRoutingId frame_id,
base::OnceCallback<void(PermissionStatus)> callback) override;
MOCK_METHOD4(ConfirmDirectoryReadAccess_,
void(const url::Origin& origin,
const base::FilePath& path,
GlobalFrameRoutingId frame_id,
base::OnceCallback<void(PermissionStatus)>& callback));
void ConfirmSensitiveDirectoryAccess( void ConfirmSensitiveDirectoryAccess(
const url::Origin& origin, const url::Origin& origin,
const std::vector<base::FilePath>& paths, const std::vector<base::FilePath>& paths,
......
...@@ -13,8 +13,9 @@ MockNativeFileSystemPermissionGrant::~MockNativeFileSystemPermissionGrant() = ...@@ -13,8 +13,9 @@ MockNativeFileSystemPermissionGrant::~MockNativeFileSystemPermissionGrant() =
void MockNativeFileSystemPermissionGrant::RequestPermission( void MockNativeFileSystemPermissionGrant::RequestPermission(
GlobalFrameRoutingId frame_id, GlobalFrameRoutingId frame_id,
UserActivationState user_activation_state,
base::OnceCallback<void(PermissionRequestOutcome)> callback) { base::OnceCallback<void(PermissionRequestOutcome)> callback) {
RequestPermission_(frame_id, callback); RequestPermission_(frame_id, user_activation_state, callback);
} }
} // namespace content } // namespace content
...@@ -19,9 +19,11 @@ class MockNativeFileSystemPermissionGrant ...@@ -19,9 +19,11 @@ class MockNativeFileSystemPermissionGrant
MOCK_METHOD0(GetStatus, PermissionStatus()); MOCK_METHOD0(GetStatus, PermissionStatus());
void RequestPermission( void RequestPermission(
GlobalFrameRoutingId frame_id, GlobalFrameRoutingId frame_id,
UserActivationState user_activation_state,
base::OnceCallback<void(PermissionRequestOutcome)> callback) override; base::OnceCallback<void(PermissionRequestOutcome)> callback) override;
MOCK_METHOD2(RequestPermission_, MOCK_METHOD3(RequestPermission_,
void(GlobalFrameRoutingId frame_id, void(GlobalFrameRoutingId frame_id,
UserActivationState user_activation_state,
base::OnceCallback<void(PermissionRequestOutcome)>&)); base::OnceCallback<void(PermissionRequestOutcome)>&));
using NativeFileSystemPermissionGrant::NotifyPermissionStatusChanged; using NativeFileSystemPermissionGrant::NotifyPermissionStatusChanged;
......
...@@ -113,6 +113,7 @@ void NativeFileSystemHandleBase::DoRequestPermission( ...@@ -113,6 +113,7 @@ void NativeFileSystemHandleBase::DoRequestPermission(
if (!writable) { if (!writable) {
handle_state_.read_grant->RequestPermission( handle_state_.read_grant->RequestPermission(
context().frame_id, context().frame_id,
NativeFileSystemPermissionGrant::UserActivationState::kRequired,
base::BindOnce(&NativeFileSystemHandleBase::DidRequestPermission, base::BindOnce(&NativeFileSystemHandleBase::DidRequestPermission,
AsWeakPtr(), writable, std::move(callback))); AsWeakPtr(), writable, std::move(callback)));
return; return;
...@@ -125,12 +126,15 @@ void NativeFileSystemHandleBase::DoRequestPermission( ...@@ -125,12 +126,15 @@ void NativeFileSystemHandleBase::DoRequestPermission(
// the write permission request probably fails the same way. And we check // the write permission request probably fails the same way. And we check
// the final permission status after the permission request completes // the final permission status after the permission request completes
// anyway. // anyway.
handle_state_.read_grant->RequestPermission(context().frame_id, handle_state_.read_grant->RequestPermission(
base::DoNothing()); context().frame_id,
NativeFileSystemPermissionGrant::UserActivationState::kRequired,
base::DoNothing());
} }
handle_state_.write_grant->RequestPermission( handle_state_.write_grant->RequestPermission(
context().frame_id, context().frame_id,
NativeFileSystemPermissionGrant::UserActivationState::kRequired,
base::BindOnce(&NativeFileSystemHandleBase::DidRequestPermission, base::BindOnce(&NativeFileSystemHandleBase::DidRequestPermission,
AsWeakPtr(), writable, std::move(callback))); AsWeakPtr(), writable, std::move(callback)));
} }
......
...@@ -21,6 +21,8 @@ namespace content { ...@@ -21,6 +21,8 @@ namespace content {
using base::test::RunOnceCallback; using base::test::RunOnceCallback;
using blink::mojom::PermissionStatus; using blink::mojom::PermissionStatus;
using storage::FileSystemURL; using storage::FileSystemURL;
using UserActivationState =
NativeFileSystemPermissionGrant::UserActivationState;
class TestNativeFileSystemHandle : public NativeFileSystemHandleBase { class TestNativeFileSystemHandle : public NativeFileSystemHandleBase {
public: public:
...@@ -188,9 +190,11 @@ TEST_F(NativeFileSystemHandleBaseTest, RequestWritePermission) { ...@@ -188,9 +190,11 @@ TEST_F(NativeFileSystemHandleBaseTest, RequestWritePermission) {
testing::InSequence sequence; testing::InSequence sequence;
EXPECT_CALL(*write_grant_, GetStatus()) EXPECT_CALL(*write_grant_, GetStatus())
.WillOnce(testing::Return(PermissionStatus::ASK)); .WillOnce(testing::Return(PermissionStatus::ASK));
EXPECT_CALL(*write_grant_, RequestPermission_(kFrameId, testing::_)) EXPECT_CALL(*write_grant_,
RequestPermission_(kFrameId, UserActivationState::kRequired,
testing::_))
.WillOnce( .WillOnce(
RunOnceCallback<1>(NativeFileSystemPermissionGrant:: RunOnceCallback<2>(NativeFileSystemPermissionGrant::
PermissionRequestOutcome::kUserGranted)); PermissionRequestOutcome::kUserGranted));
EXPECT_CALL(*write_grant_, GetStatus()) EXPECT_CALL(*write_grant_, GetStatus())
.WillOnce(testing::Return(PermissionStatus::GRANTED)); .WillOnce(testing::Return(PermissionStatus::GRANTED));
......
...@@ -823,16 +823,15 @@ void NativeFileSystemManagerImpl::DidVerifySensitiveDirectoryAccess( ...@@ -823,16 +823,15 @@ void NativeFileSystemManagerImpl::DidVerifySensitiveDirectoryAccess(
if (options.type() == if (options.type() ==
blink::mojom::ChooseFileSystemEntryType::kOpenDirectory) { blink::mojom::ChooseFileSystemEntryType::kOpenDirectory) {
DCHECK_EQ(entries.size(), 1u); DCHECK_EQ(entries.size(), 1u);
if (permission_context_) { SharedHandleState shared_handle_state = GetSharedHandleStateForPath(
permission_context_->ConfirmDirectoryReadAccess( entries.front(), binding_context.origin, {}, HandleType::kDirectory,
binding_context.origin, entries.front(), binding_context.frame_id, NativeFileSystemPermissionContext::UserAction::kOpen);
base::BindOnce(&NativeFileSystemManagerImpl::DidChooseDirectory, this, shared_handle_state.read_grant->RequestPermission(
binding_context, entries.front(), binding_context.frame_id,
std::move(callback))); NativeFileSystemPermissionGrant::UserActivationState::kNotRequired,
} else { base::BindOnce(&NativeFileSystemManagerImpl::DidChooseDirectory, this,
DidChooseDirectory(binding_context, entries.front(), std::move(callback), binding_context, entries.front(), std::move(callback),
PermissionStatus::GRANTED); shared_handle_state));
}
return; return;
} }
...@@ -881,20 +880,31 @@ void NativeFileSystemManagerImpl::DidChooseDirectory( ...@@ -881,20 +880,31 @@ void NativeFileSystemManagerImpl::DidChooseDirectory(
const BindingContext& binding_context, const BindingContext& binding_context,
const base::FilePath& path, const base::FilePath& path,
ChooseEntriesCallback callback, ChooseEntriesCallback callback,
NativeFileSystemPermissionContext::PermissionStatus permission) { const SharedHandleState& shared_handle_state,
NativeFileSystemPermissionGrant::PermissionRequestOutcome outcome) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
base::UmaHistogramEnumeration( base::UmaHistogramEnumeration(
"NativeFileSystemAPI.ConfirmReadDirectoryResult", permission); "NativeFileSystemAPI.ConfirmReadDirectoryResult",
shared_handle_state.read_grant->GetStatus());
std::vector<blink::mojom::NativeFileSystemEntryPtr> result_entries; std::vector<blink::mojom::NativeFileSystemEntryPtr> result_entries;
if (permission != PermissionStatus::GRANTED) { if (shared_handle_state.read_grant->GetStatus() !=
PermissionStatus::GRANTED) {
std::move(callback).Run(native_file_system_error::FromStatus( std::move(callback).Run(native_file_system_error::FromStatus(
NativeFileSystemStatus::kOperationAborted), NativeFileSystemStatus::kOperationAborted),
std::move(result_entries)); std::move(result_entries));
return; return;
} }
result_entries.push_back(CreateDirectoryEntryFromPath(binding_context, path)); auto url = CreateFileSystemURLFromPath(binding_context.origin, path);
result_entries.push_back(blink::mojom::NativeFileSystemEntry::New(
blink::mojom::NativeFileSystemHandle::NewDirectory(CreateDirectoryHandle(
binding_context, url.url,
SharedHandleState(shared_handle_state.read_grant,
shared_handle_state.write_grant,
std::move(url.file_system)))),
url.base_name));
std::move(callback).Run(native_file_system_error::Ok(), std::move(callback).Run(native_file_system_error::Ok(),
std::move(result_entries)); std::move(result_entries));
} }
......
...@@ -261,7 +261,8 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl ...@@ -261,7 +261,8 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
const BindingContext& binding_context, const BindingContext& binding_context,
const base::FilePath& path, const base::FilePath& path,
ChooseEntriesCallback callback, ChooseEntriesCallback callback,
NativeFileSystemPermissionContext::PermissionStatus permission); const SharedHandleState& shared_handle_state,
NativeFileSystemPermissionGrant::PermissionRequestOutcome outcome);
void CreateTransferTokenImpl( void CreateTransferTokenImpl(
const storage::FileSystemURL& url, const storage::FileSystemURL& url,
......
...@@ -59,15 +59,6 @@ class NativeFileSystemPermissionContext { ...@@ -59,15 +59,6 @@ class NativeFileSystemPermissionContext {
HandleType handle_type, HandleType handle_type,
UserAction user_action) = 0; UserAction user_action) = 0;
// Displays a dialog to confirm that the user intended to give read access to
// a specific directory.
using PermissionStatus = blink::mojom::PermissionStatus;
virtual void ConfirmDirectoryReadAccess(
const url::Origin& origin,
const base::FilePath& path,
GlobalFrameRoutingId frame_id,
base::OnceCallback<void(PermissionStatus)> callback) = 0;
// These values are persisted to logs. Entries should not be renumbered and // These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused. // numeric values should never be reused.
enum class SensitiveDirectoryResult { enum class SensitiveDirectoryResult {
......
...@@ -49,11 +49,16 @@ class CONTENT_EXPORT NativeFileSystemPermissionGrant ...@@ -49,11 +49,16 @@ class CONTENT_EXPORT NativeFileSystemPermissionGrant
kMaxValue = kGrantedByContentSetting kMaxValue = kGrantedByContentSetting
}; };
// Passed to |RequestPermission| to indicate if for this particular permission
// request user activation is required or not.
enum class UserActivationState { kRequired, kNotRequired };
// Call this method to request permission for this grant. The |callback| // Call this method to request permission for this grant. The |callback|
// should be called after the status of this grant has been updated with // should be called after the status of this grant has been updated with
// the outcome of the request. // the outcome of the request.
virtual void RequestPermission( virtual void RequestPermission(
GlobalFrameRoutingId frame_id, GlobalFrameRoutingId frame_id,
UserActivationState user_activation_state,
base::OnceCallback<void(PermissionRequestOutcome)> callback) = 0; base::OnceCallback<void(PermissionRequestOutcome)> callback) = 0;
// This observer can be used to be notified of changes to the permission // This observer can be used to be notified of changes to the permission
......
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