Commit 0e3c84e1 authored by Satoshi Niwa's avatar Satoshi Niwa Committed by Commit Bot

Make sure to close ARC-owned SelectFileDialogs when FileSystem connection is closed.

BUG=chromium:983047
TEST=Opt-out ARC and check if SelectFileDialog is closed

Change-Id: I3301d3c66b621f5cfbf1f0823bf4e9a17d6a57d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1705910
Commit-Queue: Satoshi Niwa <niwa@chromium.org>
Reviewed-by: default avatarRyo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678540}
parent 5bbaab09
...@@ -180,10 +180,12 @@ ArcFileSystemBridge::ArcFileSystemBridge(content::BrowserContext* context, ...@@ -180,10 +180,12 @@ ArcFileSystemBridge::ArcFileSystemBridge(content::BrowserContext* context,
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
bridge_service_->file_system()->SetHost(this); bridge_service_->file_system()->SetHost(this);
bridge_service_->file_system()->AddObserver(this);
} }
ArcFileSystemBridge::~ArcFileSystemBridge() { ArcFileSystemBridge::~ArcFileSystemBridge() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
bridge_service_->file_system()->RemoveObserver(this);
bridge_service_->file_system()->SetHost(nullptr); bridge_service_->file_system()->SetHost(nullptr);
} }
...@@ -334,11 +336,6 @@ void ArcFileSystemBridge::OnFileSelectorEvent( ...@@ -334,11 +336,6 @@ void ArcFileSystemBridge::OnFileSelectorEvent(
mojom::FileSelectorEventPtr event, mojom::FileSelectorEventPtr event,
ArcFileSystemBridge::OnFileSelectorEventCallback callback) { ArcFileSystemBridge::OnFileSelectorEventCallback callback) {
std::string track; std::string track;
if (!IsTestImageBuild()) {
LOG(ERROR) << "OnFileSelectorEvent is only allowed under test conditions";
std::move(callback).Run();
return;
}
select_files_handlers_manager_->OnFileSelectorEvent(std::move(event), select_files_handlers_manager_->OnFileSelectorEvent(std::move(event),
std::move(callback)); std::move(callback));
} }
...@@ -443,4 +440,10 @@ void ArcFileSystemBridge::OnReadRequestCompleted( ...@@ -443,4 +440,10 @@ void ArcFileSystemBridge::OnReadRequestCompleted(
file_stream_forwarders_.erase(it); file_stream_forwarders_.erase(it);
} }
void ArcFileSystemBridge::OnConnectionClosed() {
LOG(WARNING) << "FileSystem connection has been closed. "
<< "Closing SelectFileDialogs owned by ARC apps, if any.";
select_files_handlers_manager_->DeleteAllHandlers();
}
} // namespace arc } // namespace arc
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.h" #include "chrome/browser/chromeos/arc/fileapi/arc_select_files_handler.h"
#include "chrome/browser/chromeos/arc/fileapi/file_stream_forwarder.h" #include "chrome/browser/chromeos/arc/fileapi/file_stream_forwarder.h"
#include "components/arc/common/file_system.mojom.h" #include "components/arc/common/file_system.mojom.h"
#include "components/arc/session/connection_observer.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "storage/browser/fileapi/watcher_manager.h" #include "storage/browser/fileapi/watcher_manager.h"
...@@ -34,7 +35,10 @@ namespace arc { ...@@ -34,7 +35,10 @@ namespace arc {
class ArcBridgeService; class ArcBridgeService;
// This class handles file system related IPC from the ARC container. // This class handles file system related IPC from the ARC container.
class ArcFileSystemBridge : public KeyedService, public mojom::FileSystemHost { class ArcFileSystemBridge
: public KeyedService,
public ConnectionObserver<mojom::FileSystemInstance>,
public mojom::FileSystemHost {
public: public:
class Observer { class Observer {
public: public:
...@@ -95,6 +99,9 @@ class ArcFileSystemBridge : public KeyedService, public mojom::FileSystemHost { ...@@ -95,6 +99,9 @@ class ArcFileSystemBridge : public KeyedService, public mojom::FileSystemHost {
mojom::GetFileSelectorElementsRequestPtr request, mojom::GetFileSelectorElementsRequestPtr request,
GetFileSelectorElementsCallback callback) override; GetFileSelectorElementsCallback callback) override;
// ConnectionObserver<mojom::FileSystemInstance> overrides:
void OnConnectionClosed() override;
private: private:
// Used to implement OpenFileToRead(). // Used to implement OpenFileToRead().
void OpenFileToReadAfterGetFileSize(const GURL& url_decoded, void OpenFileToReadAfterGetFileSize(const GURL& url_decoded,
......
...@@ -225,7 +225,10 @@ ArcSelectFilesHandler::ArcSelectFilesHandler(content::BrowserContext* context) ...@@ -225,7 +225,10 @@ ArcSelectFilesHandler::ArcSelectFilesHandler(content::BrowserContext* context)
dialog_holder_ = std::make_unique<SelectFileDialogHolder>(this); dialog_holder_ = std::make_unique<SelectFileDialogHolder>(this);
} }
ArcSelectFilesHandler::~ArcSelectFilesHandler() = default; ArcSelectFilesHandler::~ArcSelectFilesHandler() {
// Make sure to close SelectFileDialog when the handler is destroyed.
dialog_holder_->ExecuteJavaScript(kScriptClickCancel, {});
}
void ArcSelectFilesHandler::SelectFiles( void ArcSelectFilesHandler::SelectFiles(
const mojom::SelectFilesRequestPtr& request, const mojom::SelectFilesRequestPtr& request,
......
...@@ -40,6 +40,9 @@ class ArcSelectFilesHandlersManager { ...@@ -40,6 +40,9 @@ class ArcSelectFilesHandlersManager {
explicit ArcSelectFilesHandlersManager(content::BrowserContext* context); explicit ArcSelectFilesHandlersManager(content::BrowserContext* context);
~ArcSelectFilesHandlersManager(); ~ArcSelectFilesHandlersManager();
// Delete all handlers and close all SelectFileDialogs.
void DeleteAllHandlers() { handlers_by_task_id_.clear(); }
// Handler for FileSystemHost.SelectFiles. // Handler for FileSystemHost.SelectFiles.
// Creates a new ArcSelectFilesHandler instance. // Creates a new ArcSelectFilesHandler instance.
void SelectFiles(const mojom::SelectFilesRequestPtr& request, void SelectFiles(const mojom::SelectFilesRequestPtr& request,
......
...@@ -289,6 +289,10 @@ TEST_F(ArcSelectFilesHandlerTest, OnFileSelectorEvent) { ...@@ -289,6 +289,10 @@ TEST_F(ArcSelectFilesHandlerTest, OnFileSelectorEvent) {
CallOnFileSelectorEventAndCheckScript( CallOnFileSelectorEventAndCheckScript(
mojom::FileSelectorEventType::CLICK_FILE, "Click\tTarget", mojom::FileSelectorEventType::CLICK_FILE, "Click\tTarget",
base::StringPrintf(kScriptClickFile, "\"Click\\tTarget\"")); base::StringPrintf(kScriptClickFile, "\"Click\\tTarget\""));
// The handler executes click-cancel script in its destructor.
EXPECT_CALL(*mock_dialog_holder_, ExecuteJavaScript(kScriptClickCancel, _))
.Times(1);
} }
TEST_F(ArcSelectFilesHandlerTest, GetFileSelectorElements) { TEST_F(ArcSelectFilesHandlerTest, GetFileSelectorElements) {
...@@ -316,6 +320,10 @@ TEST_F(ArcSelectFilesHandlerTest, GetFileSelectorElements) { ...@@ -316,6 +320,10 @@ TEST_F(ArcSelectFilesHandlerTest, GetFileSelectorElements) {
arc_select_files_handler_->GetFileSelectorElements( arc_select_files_handler_->GetFileSelectorElements(
arc::mojom::GetFileSelectorElementsRequest::New(), callback.Get()); arc::mojom::GetFileSelectorElementsRequest::New(), callback.Get());
// The handler executes click-cancel script in its destructor.
EXPECT_CALL(*mock_dialog_holder_, ExecuteJavaScript(kScriptClickCancel, _))
.Times(1);
} }
} // namespace arc } // namespace arc
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