Revert "[floss] Integrate version check into manager client initializ…

…ation flow"

This reverts commit 82a5d8d.

Reason for revert: Causes DeviceEventLogErrorBrowserTest.LoginUser
to fail on various builders, e.g.
https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-dbg/34415/overview

Original change's description:
> [floss] Integrate version check into manager client initialization flow
>
> This patch integrates Floss API version query into the Floss D-Bus
> manager client registration flow, checks if the API version is supported
> before initializing a Floss adapter.
> It also implements a callback for the manager client initialization to
> ensure that initialization of the manager client happens before other
> D-Bus clients.
>
> BUG=b:300216447
> TEST=device_unittests
> TEST=manual test on brya
>
> Change-Id: I35201337c2dfab4eeb14d0f66cc4e99208341982
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4876516
> Auto-Submit: Ying Hsu <yinghsu@chromium.org>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@google.com>
> Commit-Queue: Ying Hsu <yinghsu@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1209229}

BUG=b:300216447
No-Presubmit: true
No-Tree-Checks: true
No-Try: true

Change-Id: Ica443a94e83721155273ecdbc6c8df6dfbfe69e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4939621
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Owners-Override: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209720}

device/BUILD.gn

@@ -312,7 +312,6 @@ test("device_unittests") {
       "bluetooth/floss/floss_lescan_client_unittest.cc",
       "bluetooth/floss/floss_manager_client_unittest.cc",
       "bluetooth/floss/floss_socket_manager_unittest.cc",
-      "bluetooth/floss/floss_version_unittest.cc",
       "bluetooth/test/bluetooth_test_bluez.cc",
       "bluetooth/test/bluetooth_test_bluez.h",
     ]

device/bluetooth/BUILD.gn

@@ -565,8 +565,6 @@ component("bluetooth") {
         "floss/floss_sdp_types.h",
         "floss/floss_socket_manager.cc",
         "floss/floss_socket_manager.h",
-        "floss/floss_version.cc",
-        "floss/floss_version.h",
         "floss/test_helpers.h",
       ]
       if (is_chromeos) {

device/bluetooth/floss/fake_floss_manager_client.cc

@@ -10,7 +10,6 @@
 namespace floss {
 
 FakeFlossManagerClient::FakeFlossManagerClient() {
-  version_ = floss::version::GetMaximalSupportedVersion();
   adapter_to_enabled_.emplace(GetDefaultAdapter(), true);
 }
 
@@ -20,7 +19,6 @@ void FakeFlossManagerClient::Init(dbus::Bus* bus,
                                   const std::string& service_name,
                                   const int adapter_index,
                                   base::OnceClosure on_ready) {
-  init_ = true;
   std::move(on_ready).Run();
 }
 

device/bluetooth/floss/floss_dbus_manager.cc

@@ -66,7 +66,6 @@ FlossDBusManager::FlossDBusManager(dbus::Bus* bus, bool use_stubs) : bus_(bus) {
     active_adapter_ = 0;
     object_manager_supported_ = true;
     object_manager_support_known_ = true;
-    mgmt_client_present_ = true;
     InitializeAdapterClients(active_adapter_, base::DoNothing());
     return;
   }
@@ -275,7 +274,6 @@ void FlossDBusManager::InitializeFake() {
 }
 
 void FlossDBusManager::SetAllClientsPresentForTesting() {
-  mgmt_client_present_ = true;
   adapter_interface_present_ = true;
   adapter_logging_interface_present_ = true;
 #if BUILDFLAG(IS_CHROMEOS)
@@ -350,10 +348,8 @@ void FlossDBusManager::OnObjectManagerSupported(dbus::Response* response) {
 
   // Initialize the manager client (which doesn't depend on any specific
   // adapter being present)
-  client_bundle_->manager_client()->Init(
-      GetSystemBus(), kManagerInterface, kInvalidAdapter,
-      base::BindOnce(&FlossDBusManager::OnManagerClientInitComplete,
-                     weak_ptr_factory_.GetWeakPtr()));
+  client_bundle_->manager_client()->Init(GetSystemBus(), kManagerInterface,
+                                         kInvalidAdapter, base::DoNothing());
 
   // Register object manager for Manager.
   object_manager_ =
@@ -367,6 +363,11 @@ void FlossDBusManager::OnObjectManagerSupported(dbus::Response* response) {
   object_manager_->RegisterInterface(kBluetoothTelephonyInterface, this);
   object_manager_->RegisterInterface(kGattInterface, this);
   object_manager_->RegisterInterface(kSocketManagerInterface, this);
+
+  object_manager_support_known_ = true;
+  if (object_manager_support_known_callback_) {
+    std::move(object_manager_support_known_callback_).Run();
+  }
 }
 
 void FlossDBusManager::OnObjectManagerNotSupported(
@@ -382,16 +383,6 @@ void FlossDBusManager::OnObjectManagerNotSupported(
   }
 }
 
-void FlossDBusManager::OnManagerClientInitComplete() {
-  mgmt_client_present_ = client_bundle_->manager_client()->IsInitialized();
-  DVLOG(1) << "Floss manager client initialized: " << mgmt_client_present_;
-
-  object_manager_support_known_ = true;
-  if (object_manager_support_known_callback_) {
-    std::move(object_manager_support_known_callback_).Run();
-  }
-}
-
 void FlossDBusManager::SwitchAdapter(int adapter, base::OnceClosure on_ready) {
   if (!object_manager_supported_) {
     DVLOG(1) << "Floss can't switch to adapter without object manager";

device/bluetooth/floss/floss_dbus_manager.h

@@ -169,9 +169,7 @@ class DEVICE_BLUETOOTH_EXPORT FlossDBusManager
   }
 
   // Returns true if Object Manager is supported.
-  bool IsObjectManagerSupported() const {
-    return object_manager_supported_ && mgmt_client_present_;
-  }
+  bool IsObjectManagerSupported() const { return object_manager_supported_; }
 
   // Shuts down the existing adapter clients and initializes a new set for the
   // given adapter. When the new adapter clients are ready, calls the |on_ready|
@@ -232,7 +230,6 @@ class DEVICE_BLUETOOTH_EXPORT FlossDBusManager
 
   void OnObjectManagerSupported(dbus::Response* response);
   void OnObjectManagerNotSupported(dbus::ErrorResponse* response);
-  void OnManagerClientInitComplete();
 
   // Initializes the manager client
   void InitializeManagerClient();
@@ -269,9 +266,6 @@ class DEVICE_BLUETOOTH_EXPORT FlossDBusManager
   bool object_manager_support_known_ = false;
   bool object_manager_supported_ = false;
 
-  // Whether the manager client has been initialized successfully.
-  bool mgmt_client_present_ = false;
-
   bool adapter_interface_present_ = false;
   bool adapter_logging_interface_present_ = false;
 #if BUILDFLAG(IS_CHROMEOS)

device/bluetooth/floss/floss_manager_client.cc

@@ -17,7 +17,6 @@
 #include "base/logging.h"
 #include "base/observer_list.h"
 #include "base/task/single_thread_task_runner.h"
-#include "components/device_event_log/device_event_log.h"
 #include "dbus/bus.h"
 #include "dbus/exported_object.h"
 #include "dbus/message.h"
@@ -164,7 +163,7 @@ void FlossManagerClient::SetAdapterEnabled(int adapter,
       command, adapter);
 }
 
-base::Version FlossManagerClient::GetFlossApiVersion() const {
+uint32_t FlossManagerClient::GetFlossApiVersion() const {
   return version_;
 }
 
@@ -175,11 +174,6 @@ void FlossManagerClient::DoGetFlossApiVersion() {
       manager::kGetFlossApiVersion);
 }
 
-bool FlossManagerClient::IsCompatibleFlossApi() {
-  return version_ >= floss::version::GetMinimalSupportedVersion() &&
-         version_ <= floss::version::GetMaximalSupportedVersion();
-}
-
 void FlossManagerClient::OnSetAdapterEnabled(DBusResult<Void> response) {
   // Only handle error cases since non-error called in OnHciEnabledChange
   if (adapter_enabled_callback_ && !response.has_value()) {
@@ -205,9 +199,6 @@ void FlossManagerClient::SetDevCoredump(ResponseCallback<Void> callback,
 void FlossManagerClient::RegisterWithManager() {
   DCHECK(!manager_available_);
 
-  // Get Floss API version of the daemon.
-  DoGetFlossApiVersion();
-
   // Get the default adapter.
   CallManagerMethod<int>(
       base::BindOnce(&FlossManagerClient::HandleGetDefaultAdapter,
@@ -257,16 +248,13 @@ void FlossManagerClient::Init(dbus::Bus* bus,
                               const std::string& service_name,
                               const int adapter_index,
                               base::OnceClosure on_ready) {
-  init_ = false;
   bus_ = bus;
   service_name_ = service_name;
-  on_ready_ = std::move(on_ready);
 
   // We should always have object proxy since the client initialization is
   // gated on ObjectManager marking the manager interface as available.
   if (!bus_->GetObjectProxy(service_name_, dbus::ObjectPath(kManagerObject))) {
     LOG(ERROR) << "FlossManagerClient couldn't init. Object proxy was null.";
-    std::move(on_ready_).Run();
     return;
   }
 
@@ -288,7 +276,6 @@ void FlossManagerClient::Init(dbus::Bus* bus,
           base::BindOnce(&FlossManagerClient::RegisterWithManager,
                          weak_ptr_factory_.GetWeakPtr()))) {
     LOG(ERROR) << "Unable to successfully export FlossManagerClientCallbacks.";
-    std::move(on_ready_).Run();
     return;
   }
 
@@ -322,6 +309,8 @@ void FlossManagerClient::Init(dbus::Bus* bus,
         }
       }),
       base::FeatureList::IsEnabled(bluez::features::kLinkLayerPrivacy));
+
+  on_ready_ = std::move(on_ready);
 }
 
 void FlossManagerClient::HandleGetDefaultAdapter(DBusResult<int32_t> response) {
@@ -375,9 +364,7 @@ void FlossManagerClient::HandleRegisterCallback(DBusResult<Void> result) {
   if (!result.has_value()) {
     LOG(ERROR) << "Floss manager RegisterCallback returned error: "
                << result.error();
-    init_ = false;
-  } else {
-    init_ = IsCompatibleFlossApi();
+    return;
   }
 
   if (on_ready_) {
@@ -500,23 +487,13 @@ void FlossManagerClient::CompleteSetFlossEnabled(DBusResult<bool> ret) {
 void FlossManagerClient::HandleGetFlossApiVersion(
     DBusResult<uint32_t> response) {
   if (!response.has_value()) {
-    BLUETOOTH_LOG(EVENT) << "Floss API version is not available! Error="
-                         << response.error();
-    version_ = base::Version("0.0");
+    LOG(WARNING) << "Floss API version is not available. Default version is 0."
+                    " Error="
+                 << response.error();
     return;
   }
 
-  uint32_t val = response.value();
-  version_ = floss::version::IntoVersion(val);
-
-  BLUETOOTH_LOG(EVENT) << "Floss API version " << version_;
-  if (!IsCompatibleFlossApi()) {
-    BLUETOOTH_LOG(ERROR) << "Unsupported Floss API version " << version_
-                         << ". Valid range: "
-                         << floss::version::GetMinimalSupportedVersion()
-                         << " to "
-                         << floss::version::GetMinimalSupportedVersion();
-  }
+  version_ = response.value();
 }
 
 dbus::PropertySet* FlossManagerClient::CreateProperties(

device/bluetooth/floss/floss_manager_client.h

@@ -19,7 +19,6 @@
 #include "device/bluetooth/bluetooth_export.h"
 #include "device/bluetooth/floss/exported_callback_manager.h"
 #include "device/bluetooth/floss/floss_dbus_client.h"
-#include "device/bluetooth/floss/floss_version.h"
 
 namespace dbus {
 class PropertySet;
@@ -124,7 +123,7 @@ class DEVICE_BLUETOOTH_EXPORT FlossManagerClient
                                  ResponseCallback<Void> callback);
 
   // Gets Floss API version.
-  virtual base::Version GetFlossApiVersion() const;
+  virtual uint32_t GetFlossApiVersion() const;
 
   // Invoke D-Bus API to enable or disable LL privacy.
   virtual void SetLLPrivacy(ResponseCallback<bool> callback, const bool enable);
@@ -139,9 +138,6 @@ class DEVICE_BLUETOOTH_EXPORT FlossManagerClient
             const int adapter_index,
             base::OnceClosure on_ready) override;
 
-  // Whether the manager client has been initialized successfully.
-  bool IsInitialized() const { return init_; }
-
  protected:
   friend class FlossManagerClientTest;
 
@@ -164,9 +160,6 @@ class DEVICE_BLUETOOTH_EXPORT FlossManagerClient
   // Make actual D-Bus call to retrieve Floss API version from daemon.
   void DoGetFlossApiVersion();
 
-  // Checks if it is safe to use the API exported by the Floss daemon.
-  bool IsCompatibleFlossApi();
-
   // Handle response to |GetDefaultAdapter| DBus method call.
   void HandleGetDefaultAdapter(DBusResult<int32_t> response);
 
@@ -241,10 +234,7 @@ class DEVICE_BLUETOOTH_EXPORT FlossManagerClient
   base::ObserverList<Observer> observers_;
 
   // Floss API version.
-  base::Version version_;
-
-  // Whether the manager client has been initialized successfully.
-  bool init_ = false;
+  uint32_t version_ = 0;
 
  private:
   // Handle response to SetAdapterEnabled

device/bluetooth/floss/floss_manager_client_unittest.cc

@@ -320,8 +320,6 @@ class FlossManagerClientTest : public testing::Test {
 
   void DoGetFlossApiVersion() { client_->DoGetFlossApiVersion(); }
 
-  bool IsCompatibleFlossApi() { return client_->IsCompatibleFlossApi(); }
-
   void EndRunLoopCallback(base::RepeatingClosure quit, DBusResult<bool> ret) {
     std::move(quit).Run();
   }
@@ -527,29 +525,13 @@ TEST_F(FlossManagerClientTest, SetFlossEnabledRetries) {
 }
 
 TEST_F(FlossManagerClientTest, GetFlossApiVersion) {
-  base::Version version = floss::version::IntoVersion(floss_api_version_);
-
   TestManagerObserver observer(client_.get());
   client_->Init(bus_.get(), kManagerInterface, /*adapter_index=*/-1,
                 base::DoNothing());
 
-  method_called_.clear();
   DoGetFlossApiVersion();
 
   EXPECT_EQ(method_called_[manager::kGetFlossApiVersion], 1);
-  EXPECT_EQ(client_->GetFlossApiVersion(), version);
-}
-
-TEST_F(FlossManagerClientTest, NewFlossDaemonIsNotCompatible) {
-  // Given Floss daemon's Floss API version is a newer one.
-  floss_api_version_ = 0xffffffff;
-
-  // When FlossManagerClient gets the Floss API version at initialized.
-  TestManagerObserver observer(client_.get());
-  client_->Init(bus_.get(), kManagerInterface, /*adapter_index=*/-1,
-                base::DoNothing());
-
-  // Then, the Floss API exported by Floss daemon is not compatible.
-  EXPECT_FALSE(IsCompatibleFlossApi());
+  EXPECT_EQ(client_->GetFlossApiVersion(), floss_api_version_);
 }
 }  // namespace floss