[CrOS Tether] Work around GATT services unavailable error.

This CL introduces the GattServicesWorkaround class with an
implementation and test double. The class is initialized as part of the
asynchronous shutdown flow and is invoked by BleConnectionManager when
GATT services are unavailable.

Bug: 784968, 672263
Change-Id: If81085bf9f8c7ac7f6715f93f9c3c07a8dbc13e2
Reviewed-on: https://chromium-review.googlesource.com/777983Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519516}

chromeos/components/tether/BUILD.gn

@@ -10,6 +10,10 @@ static_library("tether") {
     "active_host.h",
     "active_host_network_state_updater.cc",
     "active_host_network_state_updater.h",
+    "ad_hoc_ble_advertiser.cc",
+    "ad_hoc_ble_advertiser.h",
+    "ad_hoc_ble_advertiser_impl.cc",
+    "ad_hoc_ble_advertiser_impl.h",
     "asynchronous_shutdown_object_container.h",
     "asynchronous_shutdown_object_container_impl.cc",
     "asynchronous_shutdown_object_container_impl.h",
@@ -147,6 +151,8 @@ static_library("test_support") {
   sources = [
     "fake_active_host.cc",
     "fake_active_host.h",
+    "fake_ad_hoc_ble_advertiser.cc",
+    "fake_ad_hoc_ble_advertiser.h",
     "fake_asynchronous_shutdown_object_container.cc",
     "fake_asynchronous_shutdown_object_container.h",
     "fake_ble_advertiser.cc",
@@ -219,6 +225,7 @@ source_set("unit_tests") {
   sources = [
     "active_host_network_state_updater_unittest.cc",
     "active_host_unittest.cc",
+    "ad_hoc_ble_advertiser_impl_unittest.cc",
     "asynchronous_shutdown_object_container_impl_unittest.cc",
     "ble_advertisement_device_queue_unittest.cc",
     "ble_advertiser_impl_unittest.cc",

chromeos/components/tether/ad_hoc_ble_advertiser.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/tether/ad_hoc_ble_advertiser.h"
+
+namespace chromeos {
+
+namespace tether {
+
+AdHocBleAdvertiser::AdHocBleAdvertiser() {}
+
+AdHocBleAdvertiser::~AdHocBleAdvertiser() {}
+
+void AdHocBleAdvertiser::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void AdHocBleAdvertiser::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void AdHocBleAdvertiser::NotifyAsynchronousShutdownComplete() {
+  for (auto& observer : observer_list_)
+    observer.OnAsynchronousShutdownComplete();
+}
+
+}  // namespace tether
+
+}  // namespace chromeos

chromeos/components/tether/ad_hoc_ble_advertiser.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_TETHER_AD_HOC_BLE_ADVERTISER_H_
+#define CHROMEOS_COMPONENTS_TETHER_AD_HOC_BLE_ADVERTISER_H_
+
+#include "base/macros.h"
+#include "base/observer_list.h"
+#include "components/cryptauth/remote_device.h"
+
+namespace chromeos {
+
+namespace tether {
+
+// Works around crbug.com/784968. This bug causes the tether host's GATT server
+// to shut down incorrectly, leaving behind a "stale" advertisement which does
+// not have any registered GATT services. This prevents a BLE connection from
+// being created. When this situation occurs, AdHocBleAdvertiser can work
+// around the issue by advertising to the device again.
+//
+// This class is *different* from BleAdvertiser because it advertises for an
+// extended period of time, regardless of whether the device to which it is
+// advertising is part of the BleAdvertisementDeviceQueue. The normal flow
+// (i.e., BleConnectionManager and BleAdvertiser) stops advertising to a device
+// once an advertisement is received from that same device; instead, this class
+// keeps advertising regardless. This ensures that the remote device receives
+// the advertisement.
+class AdHocBleAdvertiser {
+ public:
+  class Observer {
+   public:
+    virtual void OnAsynchronousShutdownComplete() = 0;
+    virtual ~Observer() {}
+  };
+
+  AdHocBleAdvertiser();
+  virtual ~AdHocBleAdvertiser();
+
+  // Requests that |remote_device| add GATT services. This should only be called
+  // when the device has a stale advertisement with no GATT services. See
+  // crbug.com/784968.
+  virtual void RequestGattServicesForDevice(
+      const cryptauth::RemoteDevice& remote_device) = 0;
+
+  // Returns whether there are any pending requests for GATT services.
+  virtual bool HasPendingRequests() = 0;
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+ protected:
+  void NotifyAsynchronousShutdownComplete();
+
+ private:
+  base::ObserverList<Observer> observer_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(AdHocBleAdvertiser);
+};
+
+}  // namespace tether
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_TETHER_AD_HOC_BLE_ADVERTISER_H_

chromeos/components/tether/ad_hoc_ble_advertiser_impl.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/tether/ad_hoc_ble_advertiser_impl.h"
+
+#include "base/bind.h"
+#include "chromeos/components/tether/error_tolerant_ble_advertisement_impl.h"
+#include "chromeos/components/tether/timer_factory.h"
+#include "components/cryptauth/ble/ble_advertisement_generator.h"
+#include "components/proximity_auth/logging/logging.h"
+
+namespace chromeos {
+
+namespace tether {
+
+namespace {
+
+// Empirically, phones generally pick up scan results in 1-6 seconds. 12 seconds
+// adds an extra buffer to that time to ensure that the advertisement is
+// discovered by the Tether host.
+constexpr const int64_t kNumSecondsToAdvertise = 12;
+
+}  // namespace
+
+AdHocBleAdvertiserImpl::AdvertisementWithTimer::AdvertisementWithTimer(
+    std::unique_ptr<ErrorTolerantBleAdvertisement> advertisement,
+    std::unique_ptr<base::Timer> timer)
+    : advertisement(std::move(advertisement)), timer(std::move(timer)) {}
+
+AdHocBleAdvertiserImpl::AdvertisementWithTimer::~AdvertisementWithTimer() {}
+
+AdHocBleAdvertiserImpl::AdHocBleAdvertiserImpl(
+    cryptauth::LocalDeviceDataProvider* local_device_data_provider,
+    cryptauth::RemoteBeaconSeedFetcher* remote_beacon_seed_fetcher,
+    BleSynchronizerBase* ble_synchronizer)
+    : local_device_data_provider_(local_device_data_provider),
+      remote_beacon_seed_fetcher_(remote_beacon_seed_fetcher),
+      ble_synchronizer_(ble_synchronizer),
+      timer_factory_(base::MakeUnique<TimerFactory>()),
+      weak_ptr_factory_(this) {}
+
+AdHocBleAdvertiserImpl::~AdHocBleAdvertiserImpl() {}
+
+void AdHocBleAdvertiserImpl::RequestGattServicesForDevice(
+    const cryptauth::RemoteDevice& remote_device) {
+  const std::string device_id = remote_device.GetDeviceId();
+
+  // If an advertisement to that device is already in progress, there is nothing
+  // to do.
+  if (device_id_to_advertisement_with_timer_map_.find(device_id) !=
+      device_id_to_advertisement_with_timer_map_.end()) {
+    PA_LOG(INFO) << "Advertisement already in progress to device with ID \""
+                 << remote_device.GetTruncatedDeviceIdForLogs() << "\".";
+    return;
+  }
+
+  // Generate a new advertisement to |remote_device|.
+  std::unique_ptr<cryptauth::DataWithTimestamp> service_data =
+      cryptauth::BleAdvertisementGenerator::GenerateBleAdvertisement(
+          remote_device, local_device_data_provider_,
+          remote_beacon_seed_fetcher_);
+  if (!service_data) {
+    PA_LOG(WARNING) << "Cannot generate advertisement for device with ID \""
+                    << remote_device.GetTruncatedDeviceIdForLogs() << "\"; "
+                    << "GATT services cannot be requested.";
+    return;
+  }
+
+  std::unique_ptr<ErrorTolerantBleAdvertisement> advertisement =
+      ErrorTolerantBleAdvertisementImpl::Factory::NewInstance(
+          device_id, std::move(service_data), ble_synchronizer_);
+
+  PA_LOG(INFO) << "Requesting GATT services for device with ID \""
+               << remote_device.GetTruncatedDeviceIdForLogs() << "\" by "
+               << "creating a new advertisement to that device.";
+
+  std::unique_ptr<base::Timer> timer = timer_factory_->CreateOneShotTimer();
+  timer->Start(FROM_HERE, base::TimeDelta::FromSeconds(kNumSecondsToAdvertise),
+               base::Bind(&AdHocBleAdvertiserImpl::OnTimerFired,
+                          weak_ptr_factory_.GetWeakPtr(), device_id));
+
+  device_id_to_advertisement_with_timer_map_.emplace(
+      std::piecewise_construct, std::forward_as_tuple(device_id),
+      std::forward_as_tuple(std::move(advertisement), std::move(timer)));
+}
+
+bool AdHocBleAdvertiserImpl::HasPendingRequests() {
+  return !device_id_to_advertisement_with_timer_map_.empty();
+}
+
+void AdHocBleAdvertiserImpl::OnTimerFired(const std::string& device_id) {
+  auto it = device_id_to_advertisement_with_timer_map_.find(device_id);
+  if (it == device_id_to_advertisement_with_timer_map_.end()) {
+    PA_LOG(ERROR) << "Timer fired for device with ID \""
+                  << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id)
+                  << "\", but that device is not present in the map.";
+    return;
+  }
+
+  PA_LOG(INFO) << "Stopping workaround advertisement for device with ID \""
+               << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id)
+               << "\".";
+
+  it->second.advertisement->Stop(
+      base::Bind(&AdHocBleAdvertiserImpl::OnAdvertisementStopped,
+                 weak_ptr_factory_.GetWeakPtr(), device_id));
+}
+
+void AdHocBleAdvertiserImpl::OnAdvertisementStopped(
+    const std::string& device_id) {
+  auto it = device_id_to_advertisement_with_timer_map_.find(device_id);
+  if (it == device_id_to_advertisement_with_timer_map_.end()) {
+    PA_LOG(ERROR) << "Advertisement stopped for device ID \""
+                  << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id)
+                  << "\", but that device is not present in the map.";
+    return;
+  }
+
+  device_id_to_advertisement_with_timer_map_.erase(it);
+
+  if (device_id_to_advertisement_with_timer_map_.empty())
+    NotifyAsynchronousShutdownComplete();
+}
+
+void AdHocBleAdvertiserImpl::SetTimerFactoryForTesting(
+    std::unique_ptr<TimerFactory> test_timer_factory) {
+  timer_factory_ = std::move(test_timer_factory);
+}
+
+}  // namespace tether
+
+}  // namespace chromeos

chromeos/components/tether/ad_hoc_ble_advertiser_impl.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_TETHER_AD_HOC_BLE_ADVERTISER_IMPL_H_
+#define CHROMEOS_COMPONENTS_TETHER_AD_HOC_BLE_ADVERTISER_IMPL_H_
+
+#include <memory>
+#include <string>
+#include <unordered_map>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "chromeos/components/tether/ad_hoc_ble_advertiser.h"
+
+namespace base {
+class Timer;
+}  // namespace base
+
+namespace cryptauth {
+class LocalDeviceDataProvider;
+class RemoteBeaconSeedFetcher;
+}  // namespace cryptauth
+
+namespace chromeos {
+
+namespace tether {
+
+class BleSynchronizerBase;
+class ErrorTolerantBleAdvertisement;
+class TimerFactory;
+
+// Concrete AdHocBleAdvertiser implementation. To work around the GATT
+// services bug, this class advertises to the device whose GATT services are
+// unavailable. When the remote device receives this advertisement, it is
+// expected to re-add these GATT services. See crbug.com/784968.
+class AdHocBleAdvertiserImpl : public AdHocBleAdvertiser {
+ public:
+  AdHocBleAdvertiserImpl(
+      cryptauth::LocalDeviceDataProvider* local_device_data_provider,
+      cryptauth::RemoteBeaconSeedFetcher* remote_beacon_seed_fetcher,
+      BleSynchronizerBase* ble_synchronizer);
+  ~AdHocBleAdvertiserImpl() override;
+
+  // AdHocBleAdvertiser:
+  void RequestGattServicesForDevice(
+      const cryptauth::RemoteDevice& remote_device) override;
+  bool HasPendingRequests() override;
+
+ private:
+  friend class AdHocBleAdvertiserImplTest;
+
+  struct AdvertisementWithTimer {
+    AdvertisementWithTimer(
+        std::unique_ptr<ErrorTolerantBleAdvertisement> advertisement,
+        std::unique_ptr<base::Timer> timer);
+    ~AdvertisementWithTimer();
+
+    std::unique_ptr<ErrorTolerantBleAdvertisement> advertisement;
+    std::unique_ptr<base::Timer> timer;
+  };
+
+  void OnTimerFired(const std::string& device_id);
+  void OnAdvertisementStopped(const std::string& device_id);
+
+  void SetTimerFactoryForTesting(
+      std::unique_ptr<TimerFactory> test_timer_factory);
+
+  cryptauth::LocalDeviceDataProvider* local_device_data_provider_;
+  cryptauth::RemoteBeaconSeedFetcher* remote_beacon_seed_fetcher_;
+  BleSynchronizerBase* ble_synchronizer_;
+
+  std::unique_ptr<TimerFactory> timer_factory_;
+  std::unordered_map<std::string, AdvertisementWithTimer>
+      device_id_to_advertisement_with_timer_map_;
+
+  base::WeakPtrFactory<AdHocBleAdvertiserImpl> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(AdHocBleAdvertiserImpl);
+};
+
+}  // namespace tether
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_TETHER_AD_HOC_BLE_ADVERTISER_IMPL_H_

chromeos/components/tether/asynchronous_shutdown_object_container_impl.cc

@@ -4,6 +4,7 @@
 
 #include "chromeos/components/tether/asynchronous_shutdown_object_container_impl.h"
 
+#include "chromeos/components/tether/ad_hoc_ble_advertiser_impl.h"
 #include "chromeos/components/tether/ble_advertisement_device_queue.h"
 #include "chromeos/components/tether/ble_advertiser_impl.h"
 #include "chromeos/components/tether/ble_connection_manager.h"
@@ -95,12 +96,17 @@ AsynchronousShutdownObjectContainerImpl::
           base::MakeUnique<BleScannerImpl>(adapter,
                                            local_device_data_provider_.get(),
                                            ble_synchronizer_.get())),
+      ad_hoc_ble_advertiser_(base::MakeUnique<AdHocBleAdvertiserImpl>(
+          local_device_data_provider_.get(),
+          remote_beacon_seed_fetcher_.get(),
+          ble_synchronizer_.get())),
       ble_connection_manager_(base::MakeUnique<BleConnectionManager>(
           cryptauth_service,
           adapter,
           ble_advertisement_device_queue_.get(),
           ble_advertiser_.get(),
-          ble_scanner_.get())),
+          ble_scanner_.get(),
+          ad_hoc_ble_advertiser_.get())),
       disconnect_tethering_request_sender_(
           base::MakeUnique<DisconnectTetheringRequestSenderImpl>(
               ble_connection_manager_.get(),
@@ -120,6 +126,7 @@ AsynchronousShutdownObjectContainerImpl::
   ble_advertiser_->RemoveObserver(this);
   ble_scanner_->RemoveObserver(this);
   disconnect_tethering_request_sender_->RemoveObserver(this);
+  ad_hoc_ble_advertiser_->RemoveObserver(this);
 }
 
 void AsynchronousShutdownObjectContainerImpl::Shutdown(
@@ -133,6 +140,7 @@ void AsynchronousShutdownObjectContainerImpl::Shutdown(
   ble_advertiser_->AddObserver(this);
   ble_scanner_->AddObserver(this);
   disconnect_tethering_request_sender_->AddObserver(this);
+  ad_hoc_ble_advertiser_->AddObserver(this);
 
   ShutdownIfPossible();
 }
@@ -172,6 +180,10 @@ void AsynchronousShutdownObjectContainerImpl::
   ShutdownIfPossible();
 }
 
+void AsynchronousShutdownObjectContainerImpl::OnAsynchronousShutdownComplete() {
+  ShutdownIfPossible();
+}
+
 void AsynchronousShutdownObjectContainerImpl::OnDiscoverySessionStateChanged(
     bool discovery_session_active) {
   ShutdownIfPossible();
@@ -186,6 +198,7 @@ void AsynchronousShutdownObjectContainerImpl::ShutdownIfPossible() {
   ble_advertiser_->RemoveObserver(this);
   ble_scanner_->RemoveObserver(this);
   disconnect_tethering_request_sender_->RemoveObserver(this);
+  ad_hoc_ble_advertiser_->RemoveObserver(this);
 
   shutdown_complete_callback_.Run();
 }
@@ -213,6 +226,10 @@ bool AsynchronousShutdownObjectContainerImpl::
   if (ble_advertiser_->AreAdvertisementsRegistered())
     return true;
 
+  // Likewise, the ad hoc BLE advertiser must be shut down.
+  if (ad_hoc_ble_advertiser_->HasPendingRequests())
+    return true;
+
   return false;
 }
 
@@ -220,11 +237,13 @@ void AsynchronousShutdownObjectContainerImpl::SetTestDoubles(
     std::unique_ptr<BleAdvertiser> ble_advertiser,
     std::unique_ptr<BleScanner> ble_scanner,
     std::unique_ptr<DisconnectTetheringRequestSender>
-        disconnect_tethering_request_sender) {
+        disconnect_tethering_request_sender,
+    std::unique_ptr<AdHocBleAdvertiser> ad_hoc_ble_advertiser) {
   ble_advertiser_ = std::move(ble_advertiser);
   ble_scanner_ = std::move(ble_scanner);
   disconnect_tethering_request_sender_ =
       std::move(disconnect_tethering_request_sender);
+  ad_hoc_ble_advertiser_ = std::move(ad_hoc_ble_advertiser);
 }
 
 }  // namespace tether

chromeos/components/tether/asynchronous_shutdown_object_container_impl.h

@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "chromeos/components/tether/ad_hoc_ble_advertiser.h"
 #include "chromeos/components/tether/asynchronous_shutdown_object_container.h"
 #include "chromeos/components/tether/ble_advertiser.h"
 #include "chromeos/components/tether/ble_scanner.h"
@@ -47,7 +48,8 @@ class AsynchronousShutdownObjectContainerImpl
     : public AsynchronousShutdownObjectContainer,
       public BleAdvertiser::Observer,
       public BleScanner::Observer,
-      public DisconnectTetheringRequestSender::Observer {
+      public DisconnectTetheringRequestSender::Observer,
+      public AdHocBleAdvertiser::Observer {
  public:
   class Factory {
    public:
@@ -104,16 +106,21 @@ class AsynchronousShutdownObjectContainerImpl
   // DisconnectTetheringRequestSender::Observer:
   void OnPendingDisconnectRequestsComplete() override;
 
+  // AdHocBleAdvertiser::Observer:
+  void OnAsynchronousShutdownComplete() override;
+
  private:
   friend class AsynchronousShutdownObjectContainerImplTest;
 
   void ShutdownIfPossible();
   bool AreAsynchronousOperationsActive();
 
-  void SetTestDoubles(std::unique_ptr<BleAdvertiser> ble_advertiser,
-                      std::unique_ptr<BleScanner> ble_scanner,
-                      std::unique_ptr<DisconnectTetheringRequestSender>
-                          disconnect_tethering_request_sender);
+  void SetTestDoubles(
+      std::unique_ptr<BleAdvertiser> ble_advertiser,
+      std::unique_ptr<BleScanner> ble_scanner,
+      std::unique_ptr<DisconnectTetheringRequestSender>
+          disconnect_tethering_request_sender,
+      std::unique_ptr<AdHocBleAdvertiser> ad_hoc_ble_advertiser);
 
   scoped_refptr<device::BluetoothAdapter> adapter_;
 
@@ -126,6 +133,7 @@ class AsynchronousShutdownObjectContainerImpl
   std::unique_ptr<BleSynchronizer> ble_synchronizer_;
   std::unique_ptr<BleAdvertiser> ble_advertiser_;
   std::unique_ptr<BleScanner> ble_scanner_;
+  std::unique_ptr<AdHocBleAdvertiser> ad_hoc_ble_advertiser_;
   std::unique_ptr<BleConnectionManager> ble_connection_manager_;
   std::unique_ptr<DisconnectTetheringRequestSender>
       disconnect_tethering_request_sender_;

chromeos/components/tether/asynchronous_shutdown_object_container_impl_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/test/scoped_task_environment.h"
+#include "chromeos/components/tether/fake_ad_hoc_ble_advertiser.h"
 #include "chromeos/components/tether/fake_ble_advertiser.h"
 #include "chromeos/components/tether/fake_ble_scanner.h"
 #include "chromeos/components/tether/fake_disconnect_tethering_request_sender.h"
@@ -65,11 +66,13 @@ class AsynchronousShutdownObjectContainerImplTest : public testing::Test {
         new FakeBleScanner(false /* automatically_update_discovery_session */);
     fake_disconnect_tethering_request_sender_ =
         new FakeDisconnectTetheringRequestSender();
+    fake_ad_hoc_ble_advertisement_ = new FakeAdHocBleAdvertiser();
 
     container_->SetTestDoubles(
         base::WrapUnique(fake_ble_advertiser_),
         base::WrapUnique(fake_ble_scanner_),
-        base::WrapUnique(fake_disconnect_tethering_request_sender_));
+        base::WrapUnique(fake_disconnect_tethering_request_sender_),
+        base::WrapUnique(fake_ad_hoc_ble_advertisement_));
   }
 
   bool MockIsAdapterPowered() { return is_adapter_powered_; }
@@ -93,6 +96,7 @@ class AsynchronousShutdownObjectContainerImplTest : public testing::Test {
   FakeBleScanner* fake_ble_scanner_;
   FakeDisconnectTetheringRequestSender*
       fake_disconnect_tethering_request_sender_;
+  FakeAdHocBleAdvertiser* fake_ad_hoc_ble_advertisement_;
 
   bool was_shutdown_callback_invoked_;
   bool is_adapter_powered_;
@@ -163,6 +167,23 @@ TEST_F(AsynchronousShutdownObjectContainerImplTest,
   EXPECT_TRUE(was_shutdown_callback_invoked_);
 }
 
+TEST_F(AsynchronousShutdownObjectContainerImplTest,
+       TestShutdown_AsyncAdHocBleAdvertiserShutdown) {
+  fake_ad_hoc_ble_advertisement_->set_has_pending_requests(true);
+  EXPECT_TRUE(fake_ad_hoc_ble_advertisement_->HasPendingRequests());
+
+  // Start the shutdown; it should not yet succeed since there are pending
+  // requests.
+  CallShutdown();
+  EXPECT_FALSE(was_shutdown_callback_invoked_);
+
+  // Now, finish the pending requests; this should cause the shutdown to
+  // complete.
+  fake_ad_hoc_ble_advertisement_->set_has_pending_requests(false);
+  fake_ad_hoc_ble_advertisement_->NotifyAsynchronousShutdownComplete();
+  EXPECT_TRUE(was_shutdown_callback_invoked_);
+}
+
 TEST_F(AsynchronousShutdownObjectContainerImplTest,
        TestShutdown_MultipleSimultaneousAsyncShutdowns) {
   fake_ble_advertiser_->set_are_advertisements_registered(true);
@@ -175,6 +196,9 @@ TEST_F(AsynchronousShutdownObjectContainerImplTest,
   fake_disconnect_tethering_request_sender_->set_has_pending_requests(true);
   EXPECT_TRUE(fake_disconnect_tethering_request_sender_->HasPendingRequests());
 
+  fake_ad_hoc_ble_advertisement_->set_has_pending_requests(true);
+  EXPECT_TRUE(fake_ad_hoc_ble_advertisement_->HasPendingRequests());
+
   // Start the shutdown; it should not yet succeed since there are pending
   // requests.
   CallShutdown();
@@ -187,6 +211,12 @@ TEST_F(AsynchronousShutdownObjectContainerImplTest,
   fake_ble_advertiser_->NotifyAllAdvertisementsUnregistered();
   EXPECT_FALSE(was_shutdown_callback_invoked_);
 
+  // Now, finish GATT services workaround shutdown; this should not cause the
+  // shutdown to complete since there are still pending requests
+  fake_ad_hoc_ble_advertisement_->set_has_pending_requests(false);
+  fake_ad_hoc_ble_advertisement_->NotifyAsynchronousShutdownComplete();
+  EXPECT_FALSE(was_shutdown_callback_invoked_);
+
   // Now, remove the discovery session; this should not cause the shutdown to
   // complete since there are still pending requests.
   fake_ble_scanner_->set_is_discovery_session_active(false);

chromeos/components/tether/ble_connection_manager.cc

@@ -6,6 +6,7 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/time/default_clock.h"
+#include "chromeos/components/tether/ad_hoc_ble_advertiser.h"
 #include "chromeos/components/tether/ble_constants.h"
 #include "chromeos/components/tether/timer_factory.h"
 #include "components/cryptauth/ble/bluetooth_low_energy_weave_client_connection.h"
@@ -190,7 +191,7 @@ void BleConnectionManager::ConnectionMetadata::OnMessageSent(
 
 void BleConnectionManager::ConnectionMetadata::
     OnGattCharacteristicsNotAvailable() {
-  // TODO(khorimoto): Work around GATT bug. See crbug.com/784968.
+  manager_->OnGattCharacteristicsNotAvailable(remote_device_);
 }
 
 BleConnectionManager::BleConnectionManager(
@@ -198,12 +199,14 @@ BleConnectionManager::BleConnectionManager(
     scoped_refptr<device::BluetoothAdapter> adapter,
     BleAdvertisementDeviceQueue* ble_advertisement_device_queue,
     BleAdvertiser* ble_advertiser,
-    BleScanner* ble_scanner)
+    BleScanner* ble_scanner,
+    AdHocBleAdvertiser* ad_hoc_ble_advertisement)
     : cryptauth_service_(cryptauth_service),
       adapter_(adapter),
       ble_advertisement_device_queue_(ble_advertisement_device_queue),
       ble_advertiser_(ble_advertiser),
       ble_scanner_(ble_scanner),
+      ad_hoc_ble_advertisement_(ad_hoc_ble_advertisement),
       timer_factory_(base::MakeUnique<TimerFactory>()),
       clock_(base::MakeUnique<base::DefaultClock>()),
       has_registered_observer_(false),
@@ -518,6 +521,14 @@ void BleConnectionManager::OnSecureChannelStatusChanged(
   UpdateConnectionAttempts();
 }
 
+void BleConnectionManager::OnGattCharacteristicsNotAvailable(
+    const cryptauth::RemoteDevice& remote_device) {
+  PA_LOG(WARNING) << "Previous connection attempt failed due to unavailable "
+                  << "GATT services for device ID \""
+                  << remote_device.GetTruncatedDeviceIdForLogs() << "\".";
+  ad_hoc_ble_advertisement_->RequestGattServicesForDevice(remote_device);
+}
+
 void BleConnectionManager::SendMessageReceivedEvent(
     cryptauth::RemoteDevice remote_device,
     std::string payload) {

chromeos/components/tether/ble_connection_manager.h

@@ -36,6 +36,7 @@ namespace chromeos {
 
 namespace tether {
 
+class AdHocBleAdvertiser;
 class TimerFactory;
 
 // Manages connections to remote devices. When a device is registered,
@@ -84,7 +85,8 @@ class BleConnectionManager : public BleScanner::Observer {
       scoped_refptr<device::BluetoothAdapter> adapter,
       BleAdvertisementDeviceQueue* ble_advertisement_device_queue,
       BleAdvertiser* ble_advertiser,
-      BleScanner* ble_scanner);
+      BleScanner* ble_scanner,
+      AdHocBleAdvertiser* ad_hoc_ble_advertisement);
   virtual ~BleConnectionManager();
 
   // Registers |remote_device| for |connection_reason|. Once registered, this
@@ -210,6 +212,8 @@ class BleConnectionManager : public BleScanner::Observer {
       const cryptauth::RemoteDevice& remote_device,
       const cryptauth::SecureChannel::Status& old_status,
       const cryptauth::SecureChannel::Status& new_status);
+  void OnGattCharacteristicsNotAvailable(
+      const cryptauth::RemoteDevice& remote_device);
 
   void SetTestDoubles(std::unique_ptr<base::Clock> test_clock,
                       std::unique_ptr<TimerFactory> test_timer_factory);
@@ -225,6 +229,7 @@ class BleConnectionManager : public BleScanner::Observer {
   BleAdvertisementDeviceQueue* ble_advertisement_device_queue_;
   BleAdvertiser* ble_advertiser_;
   BleScanner* ble_scanner_;
+  AdHocBleAdvertiser* ad_hoc_ble_advertisement_;
 
   std::unique_ptr<TimerFactory> timer_factory_;
   std::unique_ptr<base::Clock> clock_;

chromeos/components/tether/ble_connection_manager_unittest.cc

@@ -8,6 +8,7 @@
 #include "base/test/simple_test_clock.h"
 #include "base/timer/mock_timer.h"
 #include "chromeos/components/tether/ble_constants.h"
+#include "chromeos/components/tether/fake_ad_hoc_ble_advertiser.h"
 #include "chromeos/components/tether/fake_ble_advertiser.h"
 #include "chromeos/components/tether/fake_ble_scanner.h"
 #include "chromeos/components/tether/proto/tether.pb.h"
@@ -270,6 +271,8 @@ class BleConnectionManagerTest : public testing::Test {
     fake_ble_scanner_ = base::MakeUnique<FakeBleScanner>(
         true /* automatically_update_discovery_session */);
 
+    fake_ad_hoc_ble_advertiser_ = base::MakeUnique<FakeAdHocBleAdvertiser>();
+
     fake_connection_factory_ = base::WrapUnique(new FakeConnectionFactory(
         mock_adapter_, device::BluetoothUUID(kGattServerUuid)));
     cryptauth::weave::BluetoothLowEnergyWeaveClientConnection::Factory::
@@ -282,7 +285,8 @@ class BleConnectionManagerTest : public testing::Test {
 
     manager_ = base::WrapUnique(new BleConnectionManager(
         fake_cryptauth_service_.get(), mock_adapter_, device_queue_.get(),
-        fake_ble_advertiser_.get(), fake_ble_scanner_.get()));
+        fake_ble_advertiser_.get(), fake_ble_scanner_.get(),
+        fake_ad_hoc_ble_advertiser_.get()));
     test_observer_ = base::WrapUnique(new TestObserver());
     manager_->AddObserver(test_observer_.get());
 
@@ -517,6 +521,7 @@ class BleConnectionManagerTest : public testing::Test {
   scoped_refptr<NiceMock<device::MockBluetoothAdapter>> mock_adapter_;
   std::unique_ptr<FakeBleAdvertiser> fake_ble_advertiser_;
   std::unique_ptr<FakeBleScanner> fake_ble_scanner_;
+  std::unique_ptr<FakeAdHocBleAdvertiser> fake_ad_hoc_ble_advertiser_;
   std::unique_ptr<BleAdvertisementDeviceQueue> device_queue_;
   MockTimerFactory* mock_timer_factory_;
   base::SimpleTestClock* test_clock_;
@@ -594,6 +599,30 @@ TEST_F(BleConnectionManagerTest, TestRegistersAndUnregister_NoConnection) {
   VerifyConnectionToAuthenticationDurationMetricNotRecorded();
 }
 
+TEST_F(BleConnectionManagerTest, TestAdHocBleAdvertiser) {
+  manager_->RegisterRemoteDevice(test_devices_[0],
+                                 MessageType::TETHER_AVAILABILITY_REQUEST);
+  VerifyAdvertisingTimeoutSet(test_devices_[0]);
+  VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange>{
+      {test_devices_[0], cryptauth::SecureChannel::Status::DISCONNECTED,
+       cryptauth::SecureChannel::Status::CONNECTING}});
+
+  // Simulate the channel failing to find GATT services and disconnecting.
+  FakeSecureChannel* channel =
+      ConnectChannel(test_devices_[0], kBluetoothAddress1);
+  channel->NotifyGattCharacteristicsNotAvailable();
+  channel->Disconnect();
+  VerifyConnectionStateChanges(std::vector<SecureChannelStatusChange>{
+      {test_devices_[0], cryptauth::SecureChannel::Status::CONNECTING,
+       cryptauth::SecureChannel::Status::DISCONNECTED},
+      {test_devices_[0], cryptauth::SecureChannel::Status::DISCONNECTED,
+       cryptauth::SecureChannel::Status::CONNECTING}});
+
+  // A GATT services workaround should have been requested for that device.
+  EXPECT_EQ(std::vector<std::string>{test_devices_[0].GetDeviceId()},
+            fake_ad_hoc_ble_advertiser_->requested_device_ids());
+}
+
 TEST_F(BleConnectionManagerTest, TestRegisterWithNoConnection_TimeoutOccurs) {
   manager_->RegisterRemoteDevice(test_devices_[0],
                                  MessageType::TETHER_AVAILABILITY_REQUEST);

chromeos/components/tether/fake_ad_hoc_ble_advertiser.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/tether/fake_ad_hoc_ble_advertiser.h"
+
+namespace chromeos {
+
+namespace tether {
+
+FakeAdHocBleAdvertiser::FakeAdHocBleAdvertiser() {}
+
+FakeAdHocBleAdvertiser::~FakeAdHocBleAdvertiser() {}
+
+void FakeAdHocBleAdvertiser::NotifyAsynchronousShutdownComplete() {
+  AdHocBleAdvertiser::NotifyAsynchronousShutdownComplete();
+}
+
+void FakeAdHocBleAdvertiser::RequestGattServicesForDevice(
+    const cryptauth::RemoteDevice& remote_device) {
+  requested_device_ids_.push_back(remote_device.GetDeviceId());
+}
+
+bool FakeAdHocBleAdvertiser::HasPendingRequests() {
+  return has_pending_requests_;
+}
+
+}  // namespace tether
+
+}  // namespace chromeos

chromeos/components/tether/fake_ad_hoc_ble_advertiser.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_TETHER_FAKE_AD_HOC_BLE_ADVERTISER_H_
+#define CHROMEOS_COMPONENTS_TETHER_FAKE_AD_HOC_BLE_ADVERTISER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "chromeos/components/tether/ad_hoc_ble_advertiser.h"
+
+namespace chromeos {
+
+namespace tether {
+
+// Test doublue for AdHocBleAdvertiser.
+class FakeAdHocBleAdvertiser : public AdHocBleAdvertiser {
+ public:
+  FakeAdHocBleAdvertiser();
+  ~FakeAdHocBleAdvertiser() override;
+
+  void set_has_pending_requests(bool has_pending_requests) {
+    has_pending_requests_ = has_pending_requests;
+  }
+
+  const std::vector<std::string>& requested_device_ids() {
+    return requested_device_ids_;
+  }
+
+  void NotifyAsynchronousShutdownComplete();
+
+  // AdHocBleAdvertiser:
+  void RequestGattServicesForDevice(
+      const cryptauth::RemoteDevice& remote_device) override;
+  bool HasPendingRequests() override;
+
+ private:
+  bool has_pending_requests_ = false;
+  std::vector<std::string> requested_device_ids_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeAdHocBleAdvertiser);
+};
+
+}  // namespace tether
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_TETHER_FAKE_AD_HOC_BLE_ADVERTISER_H_

chromeos/components/tether/fake_ble_connection_manager.cc

@@ -29,6 +29,7 @@ FakeBleConnectionManager::FakeBleConnectionManager()
                            nullptr,
                            nullptr,
                            nullptr,
+                           nullptr,
                            nullptr) {}
 
 FakeBleConnectionManager::~FakeBleConnectionManager() {}

components/cryptauth/ble/ble_advertisement_generator.h

@@ -14,6 +14,7 @@
 namespace chromeos {
 namespace tether {
 class BleAdvertiserImplTest;
+class AdHocBleAdvertiserImplTest;
 }  // namespace tether
 }  // namespace chromeos
 
@@ -46,6 +47,7 @@ class BleAdvertisementGenerator {
  private:
   friend class CryptAuthBleAdvertisementGeneratorTest;
   friend class chromeos::tether::BleAdvertiserImplTest;
+  friend class chromeos::tether::AdHocBleAdvertiserImplTest;
 
   static BleAdvertisementGenerator* instance_;
 

components/cryptauth/fake_secure_channel.cc

@@ -33,25 +33,28 @@ void FakeSecureChannel::ReceiveMessage(const std::string& feature,
                                        const std::string& payload) {
   // Copy to prevent channel from being removed during handler.
   std::vector<Observer*> observers_copy = observers_;
-  for (auto* observer : observers_copy) {
+  for (auto* observer : observers_copy)
     observer->OnMessageReceived(this, feature, payload);
-  }
 }
 
 void FakeSecureChannel::CompleteSendingMessage(int sequence_number) {
   DCHECK(next_sequence_number_ > sequence_number);
   // Copy to prevent channel from being removed during handler.
   std::vector<Observer*> observers_copy = observers_;
-  for (auto* observer : observers_copy) {
+  for (auto* observer : observers_copy)
     observer->OnMessageSent(this, sequence_number);
-  }
 }
 
 void FakeSecureChannel::NotifyGattCharacteristicsNotAvailable() {
-  SecureChannel::NotifyGattCharacteristicsNotAvailable();
+  // Copy to prevent channel from being removed during handler.
+  std::vector<Observer*> observers_copy = observers_;
+  for (auto* observer : observers_copy)
+    observer->OnGattCharacteristicsNotAvailable();
 }
 
-void FakeSecureChannel::Initialize() {}
+void FakeSecureChannel::Initialize() {
+  ChangeStatus(Status::CONNECTING);
+}
 
 int FakeSecureChannel::SendMessage(const std::string& feature,
                                    const std::string& payload) {