Automated g4 rollback of changelist 451248048.

*** Reason for rollback *** Although this fixes Bluetooth Classic, it appears to break Ble, will resubmit once Ble is fixed. *** Original change description *** BEGIN_PUBLIC Bug fixes for 233012255 and 232995024 Refactor bluetooth_adapter setname and getname to remove logic errors, array overruns, and leaking memory. Refactor bluetooth discovery callbacks to prevent getting updates before we have cached the device found. Refactor the dart code interfaces to more closely align between the dll and the dart UI, and move from using the device name (which can be easily duplicated) as the primary key to using endpoint_id as the primary key for all calls into... *** PiperOrigin-RevId: 452177878
2026-09-16 15:36:12 -04:00 · 2022-05-31 17:31:03 -07:00
parent b4b74ca5b4
commit b5a1359d1f
12 changed files with 95 additions and 140 deletions
@@ -29,7 +29,7 @@ namespace api {
 class InputFile : public InputStream {
 public:
  ~InputFile() override = default;
-  virtual std::string_view GetFilePath() const = 0;
+  virtual std::string GetFilePath() const = 0;
  virtual std::int64_t GetTotalSize() const = 0;
 };

@@ -35,7 +35,9 @@ class IOFile final : public api::InputFile, public api::OutputFile {
  static std::unique_ptr<IOFile> CreateOutputFile(const absl::string_view path);

  ExceptionOr<ByteArray> Read(std::int64_t size) override;
-  std::string_view GetFilePath() const override { return path_; }
+  std::string GetFilePath() const override {
+    return std::string(path_.data(), path_.size());
+  }
  std::int64_t GetTotalSize() const override { return total_size_; }
  Exception Close() override;

@@ -52,7 +52,8 @@ BluetoothAdapter::BluetoothAdapter() {
      winrt::Windows::Devices::Bluetooth::BluetoothAdapter::GetDefaultAsync()
          .get();
  if (windows_bluetooth_adapter_ == nullptr) {
-    NEARBY_LOGS(ERROR) << __func__ << ": No Bluetooth adapter on this device.";
+    NEARBY_LOGS(ERROR)
+        << __func__ << ": No Bluetooth adapter on this device.";
  } else {
    // Gets the radio represented by this Bluetooth adapter.
    // https://docs.microsoft.com/en-us/uwp/api/windows.devices.bluetooth.bluetoothadapter.getradioasync?view=winrt-20348
@@ -64,7 +65,8 @@ BluetoothAdapter::BluetoothAdapter() {
 // returns true if the operation was a success.
 bool BluetoothAdapter::SetStatus(Status status) {
  if (windows_bluetooth_radio_ == nullptr) {
-    NEARBY_LOGS(ERROR) << __func__ << ": No Bluetooth radio on this device.";
+    NEARBY_LOGS(ERROR)
+        << __func__ << ": No Bluetooth radio on this device.";
    return false;
  }
  if (status == Status::kDisabled) {
@@ -82,7 +84,8 @@ bool BluetoothAdapter::SetStatus(Status status) {
 // Status::Value::kEnabled.
 bool BluetoothAdapter::IsEnabled() const {
  if (windows_bluetooth_radio_ == nullptr) {
-    NEARBY_LOGS(ERROR) << __func__ << ": No Bluetooth radio on this device.";
+    NEARBY_LOGS(ERROR)
+        << __func__ << ": No Bluetooth radio on this device.";
    return false;
  }
  // Gets the current state of the radio represented by this object.
@@ -151,7 +154,7 @@ std::string BluetoothAdapter::GetName() const {
               // key

  if (status == ERROR_SUCCESS) {
-    DWORD local_name_size = 0;
+    DWORD local_name_size;
    DWORD value_type;

    // Retrieves the size of the data for the specified value name associated
@@ -170,42 +173,42 @@ std::string BluetoothAdapter::GetName() const {
        &local_name_size);  // A pointer to a variable that specifies the
                            // size of the buffer pointed to by the lpData
                            // parameter, in bytes.
-    if (status == ERROR_SUCCESS) {
-      unsigned char *local_name = new unsigned char[local_name_size];
-      memset(local_name, '\0', local_name_size);
-
-      status = RegQueryValueExA(
-          hKey,  // A handle to an open registry key.
-          BLUETOOTH_RADIO_REGISTRY_NAME_KEY,  // The name of the registry
-                                              // value.
-          nullptr,            // This parameter is reserved and must be NULL.
-          &value_type,        // A pointer to a variable that receives a code
-                              // indicating the type of data stored in the
-                              // specified value.
-          local_name,         // A pointer to a buffer that
-                              // receives the value's data.
-          &local_name_size);  // A pointer to a variable that specifies the
-                              // size of the buffer pointed to by the lpData
-                              // parameter, in bytes.
-
-      // Closes a handle to the specified registry key.
-      // https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regclosekey
-      RegCloseKey(hKey);
-
-      if (status == ERROR_SUCCESS) {
-        std::string local_name_return = std::string(
-            local_name, local_name + local_name_size / sizeof local_name[0]);
-
-        delete[] local_name;
-
-        return local_name_return;
-      }
-      delete[] local_name;
-    } else {
+    if (status != ERROR_SUCCESS) {
      NEARBY_LOGS(ERROR)
          << __func__
          << ": Failed to get the required size of the local name buffer";
+      return {};
    }
+    unsigned char *local_name = new unsigned char[local_name_size];
+    memset(local_name, '\0', local_name_size);
+
+    status = RegQueryValueExA(
+        hKey,                               // A handle to an open registry key.
+        BLUETOOTH_RADIO_REGISTRY_NAME_KEY,  // The name of the registry
+                                            // value.
+        nullptr,            // This parameter is reserved and must be NULL.
+        &value_type,        // A pointer to a variable that receives a code
+                            // indicating the type of data stored in the
+                            // specified value.
+        local_name,         // A pointer to a buffer that
+                            // receives the value's data.
+        &local_name_size);  // A pointer to a variable that specifies the
+                            // size of the buffer pointed to by the lpData
+                            // parameter, in bytes.
+
+    // Closes a handle to the specified registry key.
+    // https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regclosekey
+    RegCloseKey(hKey);
+
+    if (status == ERROR_SUCCESS) {
+      std::string local_name_return = std::string(
+          local_name, local_name + local_name_size / sizeof local_name[0]);
+
+      delete[] local_name;
+
+      return local_name_return;
+    }
+    delete[] local_name;
  }

  // The local name is not in the registry, return the machine name
@@ -219,7 +222,7 @@ std::string BluetoothAdapter::GetName() const {
    return {};
  }

-  local_name.resize(name_size);
+  local_name.reserve(name_size);

  // Retrieves the NetBIOS name of the local computer.
  // https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getcomputernamea
@@ -395,8 +398,9 @@ bool BluetoothAdapter::SetName(absl::string_view name) {
        0,           // This parameter is reserved and must be zero.
        REG_BINARY,  // The type of data pointed to by the lpData parameter.
        (LPBYTE)std::string(name).c_str(),  // The data to be stored.
-        std::string(name).size());  // The size of the information pointed
-                                    // to by the lpData parameter, in bytes.
+        strlen(std::string(name)
+                   .c_str()));  // The size of the information pointed
+                                // to by the lpData parameter, in bytes.
  } else {
    // If we are told to set the key to "", we treat this as a reset
    // If we delete the key value the OS will default to the system
@@ -547,7 +551,8 @@ char *BluetoothAdapter::GetGenericBluetoothAdapterInstanceID(void) const {
 // Returns BT MAC address assigned to this adapter.
 std::string BluetoothAdapter::GetMacAddress() const {
  if (windows_bluetooth_adapter_ == nullptr) {
-    NEARBY_LOGS(ERROR) << __func__ << ": No Bluetooth adapter on this device.";
+    NEARBY_LOGS(ERROR)
+        << __func__ << ": No Bluetooth adapter on this device.";
    return "";
  }
  return uint64_to_mac_address_string(
@@ -68,7 +68,6 @@ void BluetoothClassicMedium::OnScanModeChanged(
 bool BluetoothClassicMedium::StartDiscovery(
    BluetoothClassicMedium::DiscoveryCallback discovery_callback) {
  EnterCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "StartDisovery entered critical section.";

  bool result = false;
  discovery_callback_ = discovery_callback;
@@ -78,14 +77,13 @@ bool BluetoothClassicMedium::StartDiscovery(
  }

  LeaveCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "StartDisovery left critical section.";

  return result;
 }

 bool BluetoothClassicMedium::StopDiscovery() {
  EnterCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "StopDiscovery entered critical section.";
+
  bool result = false;

  if (IsWatcherStarted()) {
@@ -93,7 +91,6 @@ bool BluetoothClassicMedium::StopDiscovery() {
  }

  LeaveCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "StopDiscovery left critical section.";

  return result;
 }
@@ -185,7 +182,6 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
  }

  EnterCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "ConnectToService entered critical section.";

  std::unique_ptr<BluetoothSocket> rfcommSocket =
      std::make_unique<BluetoothSocket>();
@@ -204,13 +200,11 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
                       << exception.what();

    LeaveCriticalSection(&critical_section_);
-    NEARBY_LOGS(INFO) << "ConnectToService left critical section.";

    return nullptr;
  }

  LeaveCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "ConnectToService left critical section.";

  return rfcommSocket;
 }
@@ -372,9 +366,6 @@ bool BluetoothClassicMedium::StopScanning() {

 winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Added(
    DeviceWatcher sender, DeviceInformation deviceInfo) {
-  EnterCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "DeviceWatcher_Added entered critical section.";
-  NEARBY_LOGS(INFO) << "Device added " << winrt::to_string(deviceInfo.Id());
  if (IsWatcherStarted()) {
    // Represents a Bluetooth device.
    // https://docs.microsoft.com/en-us/uwp/api/windows.devices.bluetooth.bluetoothdevice?view=winrt-20348
@@ -387,52 +378,45 @@ winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Added(

    // Add to our internal list if necessary
    if (it != discovered_devices_by_id_.end()) {
-      // We're already tracking this one  NEARBY_LOGS(INFO) <<
-      // "DeviceWatcher_Added entered critical section.";
-
-      LeaveCriticalSection(&critical_section_);
-      NEARBY_LOGS(INFO) << "DeviceWatcher_Added left critical section.";
-
+      // We're already tracking this one
      return winrt::fire_and_forget();
    }

    // Create a bluetooth device out of this id
-    auto bluetoothDevice =
-        winrt::Windows::Devices::Bluetooth::BluetoothDevice::FromIdAsync(
-            deviceInfo.Id())
-            .get();
+    winrt::Windows::Devices::Bluetooth::BluetoothDevice::FromIdAsync(
+        deviceInfo.Id())
+        .Completed([this, deviceInfo](
+                       winrt::Windows::Foundation::IAsyncOperation<
+                           winrt::Windows::Devices::Bluetooth::BluetoothDevice>
+                           bluetoothDevice,
+                       winrt::Windows::Foundation::AsyncStatus status) {
+          EnterCriticalSection(&critical_section_);

-    auto bluetoothDeviceP =
-        absl::WrapUnique(new BluetoothDevice(bluetoothDevice));
+          auto bluetoothDeviceP =
+              absl::WrapUnique(new BluetoothDevice(bluetoothDevice.get()));

-    discovered_devices_by_id_[deviceInfo.Id()] = std::move(bluetoothDeviceP);
+          discovered_devices_by_id_[deviceInfo.Id()] =
+              std::move(bluetoothDeviceP);

-    if (discovery_callback_.device_discovered_cb != nullptr) {
-      discovery_callback_.device_discovered_cb(
-          *discovered_devices_by_id_[deviceInfo.Id()]);
-    }
+          if (discovery_callback_.device_discovered_cb != nullptr) {
+            discovery_callback_.device_discovered_cb(
+                *discovered_devices_by_id_[deviceInfo.Id()]);
+          }
+
+          LeaveCriticalSection(&critical_section_);
+        });
  }

-  LeaveCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "DeviceWatcher_Added left critical section.";
-
  return winrt::fire_and_forget();
 }

 winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Updated(
    DeviceWatcher sender, DeviceInformationUpdate deviceInfoUpdate) {
  EnterCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "DeviceWatcher_Updated entered critical section.";
-
-  NEARBY_LOGS(INFO)
-      << "Device updated "
-      << discovered_devices_by_id_[deviceInfoUpdate.Id()]->GetName() << " ("
-      << winrt::to_string(deviceInfoUpdate.Id()) << ")";

  if (!IsWatcherStarted()) {
    // Spurious call, watcher has stopped or wasn't started
    LeaveCriticalSection(&critical_section_);
-    NEARBY_LOGS(INFO) << "DeviceWatcher_Updated left critical section.";
    return winrt::fire_and_forget();
  }

@@ -440,7 +424,6 @@ winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Updated(

  if (it == discovered_devices_by_id_.end()) {
    LeaveCriticalSection(&critical_section_);
-    NEARBY_LOGS(INFO) << "DeviceWatcher_Updated left critical section.";
    // Not tracking this device
    return winrt::fire_and_forget();
  }
@@ -452,7 +435,6 @@ winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Updated(
  }

  LeaveCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "DeviceWatcher_Updated left critical section.";

  return winrt::fire_and_forget();
 }
@@ -460,15 +442,8 @@ winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Updated(
 winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Removed(
    DeviceWatcher sender, DeviceInformationUpdate deviceInfo) {
  EnterCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "DeviceWatcher_Removed entered critical section.";
-  NEARBY_LOGS(INFO) << "Device removed "
-                    << discovered_devices_by_id_[deviceInfo.Id()]->GetName()
-                    << " (" << winrt::to_string(deviceInfo.Id()) << ")";

  if (!IsWatcherStarted()) {
-    LeaveCriticalSection(&critical_section_);
-    NEARBY_LOGS(INFO) << "DeviceWatcher_Removed left critical section.";
-
    return winrt::fire_and_forget();
  }

@@ -480,7 +455,6 @@ winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Removed(
  discovered_devices_by_id_.erase(deviceInfo.Id());

  LeaveCriticalSection(&critical_section_);
-  NEARBY_LOGS(INFO) << "DeviceWatcher_Removed left critical section.";

  return winrt::fire_and_forget();
 }
@@ -96,9 +96,7 @@ class BluetoothServerSocket : public api::BluetoothServerSocket {
  void SetScanMode(bool radioDiscoverable) {
    StopAdvertising();
    radio_discoverable_ = radioDiscoverable;
-    if (radio_discoverable_) {
-      StartAdvertising();
-    }
+    StartAdvertising();
  }

 private:
@@ -29,7 +29,7 @@ class InputFile : public api::InputFile {
  // TODO(b/184975123): replace with real implementation.
  ~InputFile() override = default;
  // TODO(b/184975123): replace with real implementation.
-  std::string_view GetFilePath() const override { return "Un-implemented"; }
+  std::string GetFilePath() const override { return "Un-implemented"; }
  // TODO(b/184975123): replace with real implementation.
  std::int64_t GetTotalSize() const override { return 0; }