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.

Tested locally on an msan build.

Original change
https://chromium-review.googlesource.com/c/chromium/src/+/2206198

TBR=dtseng@chromium.org

Change-Id: Ie623a55ad298c9e6522102338fe7357dd28c521e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208024Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769988}

chrome/browser/extensions/api/braille_display_private/braille_controller_brlapi.cc

@@ -74,23 +74,17 @@ BrailleControllerImpl::~BrailleControllerImpl() {
 
 void BrailleControllerImpl::TryLoadLibBrlApi() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (libbrlapi_loader_.loaded())
+  if (skip_libbrlapi_so_load_ || libbrlapi_loader_.loaded())
     return;
 
-  // 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;
+  // 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);
   }
-  LOG(WARNING) << "Couldn't load libbrlapi: " << strerror(errno);
 }
 
 std::unique_ptr<DisplayState> BrailleControllerImpl::GetDisplayState() {
@@ -107,6 +101,10 @@ 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;
@@ -173,7 +171,7 @@ void BrailleControllerImpl::StartConnecting() {
     return;
   started_connecting_ = true;
   TryLoadLibBrlApi();
-  if (!libbrlapi_loader_.loaded()) {
+  if (!libbrlapi_loader_.loaded() && !skip_libbrlapi_so_load_) {
     return;
   }
 
@@ -235,7 +233,7 @@ void BrailleControllerImpl::OnSocketDirChangedOnIOThread() {
 
 void BrailleControllerImpl::TryToConnect() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK(libbrlapi_loader_.loaded());
+  DCHECK(skip_libbrlapi_so_load_ || libbrlapi_loader_.loaded());
   connect_scheduled_ = false;
   if (!connection_.get())
     connection_ = create_brlapi_connection_function_.Run();
@@ -293,7 +291,7 @@ void BrailleControllerImpl::Disconnect() {
 
 std::unique_ptr<BrlapiConnection>
 BrailleControllerImpl::CreateBrlapiConnection() {
-  DCHECK(libbrlapi_loader_.loaded());
+  DCHECK(skip_libbrlapi_so_load_ || libbrlapi_loader_.loaded());
   return BrlapiConnection::Create(&libbrlapi_loader_);
 }
 

chrome/browser/extensions/api/braille_display_private/braille_controller_brlapi.h

@@ -83,6 +83,9 @@ 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);

chrome/browser/extensions/api/braille_display_private/braille_display_private_apitest.cc

@@ -50,6 +50,7 @@ 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
@@ -125,6 +126,11 @@ 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();
@@ -152,6 +158,7 @@ class BrailleDisplayPrivateApiTest : public ExtensionApiTest {
         base::Bind(
             &BrailleDisplayPrivateApiTest::CreateBrlapiConnection,
             base::Unretained(this)));
+    BrailleControllerImpl::GetInstance()->skip_libbrlapi_so_load_ = true;
     DisableAccessibilityManagerBraille();
   }
 
@@ -178,6 +185,7 @@ class BrailleDisplayPrivateApiTest : public ExtensionApiTest {
 IN_PROC_BROWSER_TEST_F(BrailleDisplayPrivateApiTest, WriteDots) {
   connection_data_.display_columns = 11;
   connection_data_.display_rows = 1;
+  connection_data_.cell_size = 6;
   ASSERT_TRUE(RunComponentExtensionTest("braille_display_private/write_dots"))
       << message_;
   ASSERT_EQ(3U, connection_data_.written_content.size());
@@ -195,6 +203,7 @@ IN_PROC_BROWSER_TEST_F(BrailleDisplayPrivateApiTest, WriteDots) {
 IN_PROC_BROWSER_TEST_F(BrailleDisplayPrivateApiTest, KeyEvents) {
   connection_data_.display_columns = 11;
   connection_data_.display_rows = 1;
+  connection_data_.cell_size = 6;
 
   // Braille navigation commands.
   connection_data_.pending_keys.push_back(BRLAPI_KEY_TYPE_CMD |
@@ -259,6 +268,7 @@ 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(

chrome/browser/extensions/api/braille_display_private/brlapi_connection.cc

@@ -44,6 +44,7 @@ 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();
@@ -180,6 +181,23 @@ 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;

chrome/browser/extensions/api/braille_display_private/brlapi_connection.h

@@ -67,6 +67,9 @@ 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);

chrome/common/extensions/api/braille_display_private.idl

@@ -56,6 +56,8 @@ 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);

chrome/test/data/extensions/api_test/braille_display_private/display_state_changes/test.js

@@ -9,9 +9,9 @@ var pass = chrome.test.callbackPass;
 
 var callbackCompleted;
 var EXPECTED_EVENTS = [
-  { "available": true, "textColumnCount": 11, "textRowCount": 1 },
-  { "available": false },
-  { "available": true, "textColumnCount": 22, "textRowCount": 1 },
+  {'available': true, 'textColumnCount': 11, 'textRowCount': 1, cellSize: 6},
+  {'available': false},
+  {'available': true, 'textColumnCount': 22, 'textRowCount': 1, cellSize: 6},
 ];
 
 var eventNumber = 0;

third_party/libbrlapi/BUILD.gn

@@ -23,5 +23,6 @@ generate_library_loader("libbrlapi") {
     "brlapi__leaveTtyMode",
     "brlapi__writeDots",
     "brlapi__readKey",
+    "brlapi__getParameter",
   ]
 }