Commit 80043932 authored by Maksim Moskvitin's avatar Maksim Moskvitin Committed by Chromium LUCI CQ

[CodeHealth] Convert c/b/e/api/braille_display_private to use modern callback

Note: |connection_| in BrailleControllerImpl never reset and
CreateBrlapiConnectionFunction never returns nullptr, thus OnceCallback
can be used.

Bug: 1152279
Change-Id: I3a7044c7b4143698fb363aa5a4a694156754a0df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640537Reviewed-by: default avatarFinnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#845678}
parent bcfc2024
...@@ -55,9 +55,8 @@ BrailleControllerImpl* BrailleControllerImpl::GetInstance() { ...@@ -55,9 +55,8 @@ BrailleControllerImpl* BrailleControllerImpl::GetInstance() {
} }
BrailleControllerImpl::BrailleControllerImpl() { BrailleControllerImpl::BrailleControllerImpl() {
create_brlapi_connection_function_ = base::Bind( create_brlapi_connection_function_ = base::BindOnce(
&BrailleControllerImpl::CreateBrlapiConnection, &BrailleControllerImpl::CreateBrlapiConnection, base::Unretained(this));
base::Unretained(this));
} }
BrailleControllerImpl::~BrailleControllerImpl() = default; BrailleControllerImpl::~BrailleControllerImpl() = default;
...@@ -141,13 +140,12 @@ void BrailleControllerImpl::RemoveObserver(BrailleObserver* observer) { ...@@ -141,13 +140,12 @@ void BrailleControllerImpl::RemoveObserver(BrailleObserver* observer) {
} }
void BrailleControllerImpl::SetCreateBrlapiConnectionForTesting( void BrailleControllerImpl::SetCreateBrlapiConnectionForTesting(
const CreateBrlapiConnectionFunction& function) { CreateBrlapiConnectionFunction function) {
if (function.is_null()) { if (function.is_null()) {
create_brlapi_connection_function_ = base::Bind( create_brlapi_connection_function_ = base::BindOnce(
&BrailleControllerImpl::CreateBrlapiConnection, &BrailleControllerImpl::CreateBrlapiConnection, base::Unretained(this));
base::Unretained(this));
} else { } else {
create_brlapi_connection_function_ = function; create_brlapi_connection_function_ = std::move(function);
} }
} }
...@@ -190,8 +188,9 @@ void BrailleControllerImpl::StartWatchingSocketDirOnTaskThread() { ...@@ -190,8 +188,9 @@ void BrailleControllerImpl::StartWatchingSocketDirOnTaskThread() {
base::FilePath brlapi_dir(BRLAPI_SOCKETPATH); base::FilePath brlapi_dir(BRLAPI_SOCKETPATH);
if (!file_path_watcher_.Watch( if (!file_path_watcher_.Watch(
brlapi_dir, base::FilePathWatcher::Type::kNonRecursive, brlapi_dir, base::FilePathWatcher::Type::kNonRecursive,
base::Bind(&BrailleControllerImpl::OnSocketDirChangedOnTaskThread, base::BindRepeating(
base::Unretained(this)))) { &BrailleControllerImpl::OnSocketDirChangedOnTaskThread,
base::Unretained(this)))) {
LOG(WARNING) << "Couldn't watch brlapi directory " << BRLAPI_SOCKETPATH; LOG(WARNING) << "Couldn't watch brlapi directory " << BRLAPI_SOCKETPATH;
} }
} }
...@@ -225,13 +224,17 @@ void BrailleControllerImpl::TryToConnect() { ...@@ -225,13 +224,17 @@ void BrailleControllerImpl::TryToConnect() {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK(skip_libbrlapi_so_load_ || libbrlapi_loader_.loaded()); DCHECK(skip_libbrlapi_so_load_ || libbrlapi_loader_.loaded());
connect_scheduled_ = false; connect_scheduled_ = false;
if (!connection_.get()) if (!connection_.get()) {
connection_ = create_brlapi_connection_function_.Run(); DCHECK(!create_brlapi_connection_function_.is_null());
if (connection_.get() && !connection_->Connected()) { connection_ = std::move(create_brlapi_connection_function_).Run();
}
DCHECK(connection_);
if (!connection_->Connected()) {
VLOG(1) << "Trying to connect to brlapi"; VLOG(1) << "Trying to connect to brlapi";
BrlapiConnection::ConnectResult result = connection_->Connect(base::Bind( BrlapiConnection::ConnectResult result =
&BrailleControllerImpl::DispatchKeys, connection_->Connect(base::BindRepeating(
base::Unretained(this))); &BrailleControllerImpl::DispatchKeys, base::Unretained(this)));
switch (result) { switch (result) {
case BrlapiConnection::CONNECT_SUCCESS: case BrlapiConnection::CONNECT_SUCCESS:
DispatchOnDisplayStateChanged(GetDisplayState()); DispatchOnDisplayStateChanged(GetDisplayState());
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <memory> #include <memory>
#include <vector> #include <vector>
#include "base/callback_forward.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/files/file_path_watcher.h" #include "base/files/file_path_watcher.h"
#include "base/memory/singleton.h" #include "base/memory/singleton.h"
...@@ -42,13 +43,13 @@ class BrailleControllerImpl : public BrailleController { ...@@ -42,13 +43,13 @@ class BrailleControllerImpl : public BrailleController {
~BrailleControllerImpl() override; ~BrailleControllerImpl() override;
void TryLoadLibBrlApi(); void TryLoadLibBrlApi();
typedef base::Callback<std::unique_ptr<BrlapiConnection>()> using CreateBrlapiConnectionFunction =
CreateBrlapiConnectionFunction; base::OnceCallback<std::unique_ptr<BrlapiConnection>()>;
// For dependency injection in tests. Sets the function used to create // For dependency injection in tests. Sets the function used to create
// brlapi connections. // brlapi connections.
void SetCreateBrlapiConnectionForTesting( void SetCreateBrlapiConnectionForTesting(
const CreateBrlapiConnectionFunction& callback); CreateBrlapiConnectionFunction callback);
// Makes the controller try to reconnect (if disconnected) as if the brlapi // Makes the controller try to reconnect (if disconnected) as if the brlapi
// socket directory had changed. // socket directory had changed.
......
...@@ -65,9 +65,10 @@ class MockBrlapiConnection : public BrlapiConnection { ...@@ -65,9 +65,10 @@ class MockBrlapiConnection : public BrlapiConnection {
public: public:
explicit MockBrlapiConnection(MockBrlapiConnectionData* data) explicit MockBrlapiConnection(MockBrlapiConnectionData* data)
: data_(data) {} : data_(data) {}
ConnectResult Connect(const OnDataReadyCallback& on_data_ready) override {
ConnectResult Connect(OnDataReadyCallback on_data_ready) override {
data_->connected = true; data_->connected = true;
on_data_ready_ = on_data_ready; on_data_ready_ = std::move(on_data_ready);
if (!data_->pending_keys.empty()) { if (!data_->pending_keys.empty()) {
content::GetIOThreadTaskRunner({})->PostTask( content::GetIOThreadTaskRunner({})->PostTask(
FROM_HERE, base::BindOnce(&MockBrlapiConnection::NotifyDataReady, FROM_HERE, base::BindOnce(&MockBrlapiConnection::NotifyDataReady,
...@@ -154,9 +155,8 @@ class BrailleDisplayPrivateApiTest : public ExtensionApiTest { ...@@ -154,9 +155,8 @@ class BrailleDisplayPrivateApiTest : public ExtensionApiTest {
connection_data_.error.brlerrno = BRLAPI_ERROR_SUCCESS; connection_data_.error.brlerrno = BRLAPI_ERROR_SUCCESS;
connection_data_.reappear_on_disconnect = false; connection_data_.reappear_on_disconnect = false;
BrailleControllerImpl::GetInstance()->SetCreateBrlapiConnectionForTesting( BrailleControllerImpl::GetInstance()->SetCreateBrlapiConnectionForTesting(
base::Bind( base::BindOnce(&BrailleDisplayPrivateApiTest::CreateBrlapiConnection,
&BrailleDisplayPrivateApiTest::CreateBrlapiConnection, base::Unretained(this)));
base::Unretained(this)));
BrailleControllerImpl::GetInstance()->skip_libbrlapi_so_load_ = true; BrailleControllerImpl::GetInstance()->skip_libbrlapi_so_load_ = true;
DisableAccessibilityManagerBraille(); DisableAccessibilityManagerBraille();
} }
......
...@@ -41,7 +41,7 @@ class BrlapiConnectionImpl : public BrlapiConnection { ...@@ -41,7 +41,7 @@ class BrlapiConnectionImpl : public BrlapiConnection {
BrlapiConnectionImpl& operator=(const BrlapiConnectionImpl&) = delete; BrlapiConnectionImpl& operator=(const BrlapiConnectionImpl&) = delete;
~BrlapiConnectionImpl() override { Disconnect(); } ~BrlapiConnectionImpl() override { Disconnect(); }
ConnectResult Connect(const OnDataReadyCallback& on_data_ready) override; ConnectResult Connect(OnDataReadyCallback on_data_ready) override;
void Disconnect() override; void Disconnect() override;
bool Connected() override { return handle_ != nullptr; } bool Connected() override { return handle_ != nullptr; }
brlapi_error_t* BrlapiError() override; brlapi_error_t* BrlapiError() override;
...@@ -67,7 +67,7 @@ std::unique_ptr<BrlapiConnection> BrlapiConnection::Create( ...@@ -67,7 +67,7 @@ std::unique_ptr<BrlapiConnection> BrlapiConnection::Create(
} }
BrlapiConnection::ConnectResult BrlapiConnectionImpl::Connect( BrlapiConnection::ConnectResult BrlapiConnectionImpl::Connect(
const OnDataReadyCallback& on_data_ready) { OnDataReadyCallback on_data_ready) {
DCHECK(!handle_); DCHECK(!handle_);
handle_.reset(reinterpret_cast<brlapi_handle_t*>( handle_.reset(reinterpret_cast<brlapi_handle_t*>(
malloc(libbrlapi_loader_->brlapi_getHandleSize()))); malloc(libbrlapi_loader_->brlapi_getHandleSize())));
...@@ -125,7 +125,7 @@ BrlapiConnection::ConnectResult BrlapiConnectionImpl::Connect( ...@@ -125,7 +125,7 @@ BrlapiConnection::ConnectResult BrlapiConnectionImpl::Connect(
} }
fd_controller_ = fd_controller_ =
base::FileDescriptorWatcher::WatchReadable(fd, on_data_ready); base::FileDescriptorWatcher::WatchReadable(fd, std::move(on_data_ready));
return CONNECT_SUCCESS; return CONNECT_SUCCESS;
} }
......
...@@ -21,7 +21,7 @@ namespace braille_display_private { ...@@ -21,7 +21,7 @@ namespace braille_display_private {
// about the semantics of the methods in this class. // about the semantics of the methods in this class.
class BrlapiConnection { class BrlapiConnection {
public: public:
typedef base::Closure OnDataReadyCallback; typedef base::RepeatingClosure OnDataReadyCallback;
enum ConnectResult { enum ConnectResult {
CONNECT_ERROR_RETRY, CONNECT_ERROR_RETRY,
...@@ -35,7 +35,7 @@ class BrlapiConnection { ...@@ -35,7 +35,7 @@ class BrlapiConnection {
BrlapiConnection& operator=(const BrlapiConnection&) = delete; BrlapiConnection& operator=(const BrlapiConnection&) = delete;
virtual ~BrlapiConnection() = default; virtual ~BrlapiConnection() = default;
virtual ConnectResult Connect(const OnDataReadyCallback& onDataReady) = 0; virtual ConnectResult Connect(OnDataReadyCallback onDataReady) = 0;
virtual void Disconnect() = 0; virtual void Disconnect() = 0;
......
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