Commit c4aa56c1 authored by Thomas Lukaszewicz's avatar Thomas Lukaszewicz Committed by Commit Bot

Revert "Reland: Plumb through an api to get display cell size from brltty"

This reverts commit cfba7924.

Reason for revert:
Tests failing on the Linux ChromiumOS MSan Tests bot:

- BrailleDisplayPrivateApiTest.WriteDots
- BrailleDisplayPrivateApiTest.KeyEvents

https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/19188

Original change's description:
> Reland: Plumb through an api to get display cell size from brltty
> 
> |cell_size| was not initialized during the test which expected it to be. The parameter is optional according to idl, but in this context, it probably should be initialized.
> 
> Original change
> https://chromium-review.googlesource.com/c/chromium/src/+/2206198
> 
> TBR=dtseng@chromium.org
> 
> Change-Id: I744c7b567cf2c4d27653f4368edc9c0f635d7d8d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207412
> Reviewed-by: David Tseng <dtseng@chromium.org>
> Commit-Queue: David Tseng <dtseng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#769736}

TBR=dtseng@chromium.org

Change-Id: I07143ad331dcb627ddd4a768525c1caf5330a70c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208103Reviewed-by: default avatarThomas Lukaszewicz <tluk@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769904}
parent 5cafdd90
......@@ -74,17 +74,23 @@ BrailleControllerImpl::~BrailleControllerImpl() {
void BrailleControllerImpl::TryLoadLibBrlApi() {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
if (skip_libbrlapi_so_load_ || libbrlapi_loader_.loaded())
if (libbrlapi_loader_.loaded())
return;
// This api version needs to match the one contained in
// third_party/libbrlapi/brlapi.h.
static const char* const kSupportedVersion = "libbrlapi.so.0.8";
if (!libbrlapi_loader_.Load(kSupportedVersion)) {
LOG(WARNING) << "Couldn't load libbrlapi(" << kSupportedVersion << ": "
<< strerror(errno);
// This list of api versions needs to contain the one used by the
// corresponding brltty used by Chrome OS for this version of Chrome. For
// example, in Chrome OS 84, we use brltty 6.1, which is using brlapi 0.8.
// The relevant header is checked into //third_party/libbrlapi/. Ensure to
// keep this list in descendning order. Note that we keep older versions so
// that tests will continue to work.
static const char* const kSupportedVersions[] = {
"libbrlapi.so.0.8", "libbrlapi.so.0.7", "libbrlapi.so.0.6",
"libbrlapi.so.0.5"};
for (size_t i = 0; i < base::size(kSupportedVersions); ++i) {
if (libbrlapi_loader_.Load(kSupportedVersions[i]))
return;
}
LOG(WARNING) << "Couldn't load libbrlapi: " << strerror(errno);
}
std::unique_ptr<DisplayState> BrailleControllerImpl::GetDisplayState() {
......@@ -101,10 +107,6 @@ std::unique_ptr<DisplayState> BrailleControllerImpl::GetDisplayState() {
display_state->available = true;
display_state->text_column_count.reset(new int(columns));
display_state->text_row_count.reset(new int(rows));
unsigned int cell_size = 0;
connection_->GetCellSize(&cell_size);
display_state->cell_size.reset(new int(cell_size));
}
}
return display_state;
......@@ -171,7 +173,7 @@ void BrailleControllerImpl::StartConnecting() {
return;
started_connecting_ = true;
TryLoadLibBrlApi();
if (!libbrlapi_loader_.loaded() && !skip_libbrlapi_so_load_) {
if (!libbrlapi_loader_.loaded()) {
return;
}
......@@ -233,7 +235,7 @@ void BrailleControllerImpl::OnSocketDirChangedOnIOThread() {
void BrailleControllerImpl::TryToConnect() {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK(skip_libbrlapi_so_load_ || libbrlapi_loader_.loaded());
DCHECK(libbrlapi_loader_.loaded());
connect_scheduled_ = false;
if (!connection_.get())
connection_ = create_brlapi_connection_function_.Run();
......@@ -291,7 +293,7 @@ void BrailleControllerImpl::Disconnect() {
std::unique_ptr<BrlapiConnection>
BrailleControllerImpl::CreateBrlapiConnection() {
DCHECK(skip_libbrlapi_so_load_ || libbrlapi_loader_.loaded());
DCHECK(libbrlapi_loader_.loaded());
return BrlapiConnection::Create(&libbrlapi_loader_);
}
......
......@@ -83,9 +83,6 @@ class BrailleControllerImpl : public BrailleController {
// Manipulated by the SequencedTaskRunner.
base::FilePathWatcher file_path_watcher_;
// Set by tests to skip libbrlapi.so loading.
bool skip_libbrlapi_so_load_ = false;
friend struct base::DefaultSingletonTraits<BrailleControllerImpl>;
DISALLOW_COPY_AND_ASSIGN(BrailleControllerImpl);
......
......@@ -50,7 +50,6 @@ struct MockBrlapiConnectionData {
bool connected;
size_t display_columns;
size_t display_rows;
size_t cell_size;
brlapi_error_t error;
std::vector<std::string> written_content;
// List of brlapi key codes. A negative number makes the connection mock
......@@ -126,11 +125,6 @@ class MockBrlapiConnection : public BrlapiConnection {
}
}
bool GetCellSize(unsigned int* cell_size) override {
*cell_size = data_->cell_size;
return true;
}
private:
void NotifyDataReady() {
on_data_ready_.Run();
......@@ -158,7 +152,6 @@ class BrailleDisplayPrivateApiTest : public ExtensionApiTest {
base::Bind(
&BrailleDisplayPrivateApiTest::CreateBrlapiConnection,
base::Unretained(this)));
BrailleControllerImpl::GetInstance()->skip_libbrlapi_so_load_ = true;
DisableAccessibilityManagerBraille();
}
......@@ -266,7 +259,6 @@ IN_PROC_BROWSER_TEST_F(BrailleDisplayPrivateApiTest, KeyEvents) {
IN_PROC_BROWSER_TEST_F(BrailleDisplayPrivateApiTest, DisplayStateChanges) {
connection_data_.display_columns = 11;
connection_data_.display_rows = 1;
connection_data_.cell_size = 6;
connection_data_.pending_keys.push_back(kErrorKeyCode);
connection_data_.reappear_on_disconnect = true;
ASSERT_TRUE(RunComponentExtensionTest(
......
......@@ -44,7 +44,6 @@ class BrlapiConnectionImpl : public BrlapiConnection {
bool GetDisplaySize(unsigned int* rows, unsigned int* columns) override;
bool WriteDots(const std::vector<unsigned char>& cells) override;
int ReadKey(brlapi_keyCode_t* keyCode) override;
bool GetCellSize(unsigned int* cell_size) override;
private:
bool CheckConnected();
......@@ -181,23 +180,6 @@ int BrlapiConnectionImpl::ReadKey(brlapi_keyCode_t* key_code) {
handle_.get(), 0 /*wait*/, key_code);
}
bool BrlapiConnectionImpl::GetCellSize(unsigned int* cell_size) {
if (!CheckConnected()) {
return false;
}
brlapi_param_deviceCellSize_t device_cell_size;
ssize_t result = libbrlapi_loader_->brlapi__getParameter(
handle_.get(), BRLAPI_PARAM_DEVICE_CELL_SIZE, 0, BRLAPI_PARAMF_GLOBAL,
&device_cell_size, sizeof(device_cell_size));
if (result == -1 || result != sizeof(device_cell_size))
return false;
*cell_size = device_cell_size;
return true;
}
bool BrlapiConnectionImpl::CheckConnected() {
if (!handle_) {
BrlapiError()->brlerrno = BRLAPI_ERROR_ILLEGAL_INSTRUCTION;
......
......@@ -67,9 +67,6 @@ class BrlapiConnection {
// value.
virtual int ReadKey(brlapi_keyCode_t* keyCode) = 0;
// Gets the number of dots in a braille cell.
virtual bool GetCellSize(unsigned int* cell_size) = 0;
protected:
BrlapiConnection();
DISALLOW_COPY_AND_ASSIGN(BrlapiConnection);
......
......@@ -56,8 +56,6 @@ namespace brailleDisplayPrivate {
long? textRowCount;
// Number of columns of braille cells on the currently connected display.
long? textColumnCount;
// The number of dots in a braille cell on the currently connected display.
long? cellSize;
};
callback DisplayStateCallback = void(DisplayState result);
......
......@@ -9,9 +9,9 @@ var pass = chrome.test.callbackPass;
var callbackCompleted;
var EXPECTED_EVENTS = [
{'available': true, 'textColumnCount': 11, 'textRowCount': 1, cellSize: 6},
{'available': false},
{'available': true, 'textColumnCount': 22, 'textRowCount': 1, cellSize: 6},
{ "available": true, "textColumnCount": 11, "textRowCount": 1 },
{ "available": false },
{ "available": true, "textColumnCount": 22, "textRowCount": 1 },
];
var eventNumber = 0;
......
......@@ -23,6 +23,5 @@ generate_library_loader("libbrlapi") {
"brlapi__leaveTtyMode",
"brlapi__writeDots",
"brlapi__readKey",
"brlapi__getParameter",
]
}
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