Reland "[CrOS PhoneHub] Mark tether ineligible if SIM has no reception."

This is a reland of a42b2a30.

phone_model_test_util's CreateFakePhoneStatusModel() returned
unexpected mobile status, resulting in failing tests. Created
TetherControllerImplTest's own CreateFakePhoneStatusModel() to fix.

Original change's description:
> [CrOS PhoneHub] Mark tether ineligible if SIM has no reception.
>
> TetherControllerImpl will now mark the tether feature ineligible if
> it is notified by the PhoneModel that the SIM state has changed to
> anything other than kSimWithReception.
>
> Fixed: 1140812
> Bug: 1106937
> Change-Id: Ia18c5e06c57805541b03b29afea1762e85b8304f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500182
> Commit-Queue: Regan Hsu <hsuregan@chromium.org>
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#821493}

Fixed: 1140812
Fixed: 1143173
Bug: 1106937
Change-Id: I310daa1d1feea6a74485ddd143608b216414f3bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2507632
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822892}

chromeos/components/phonehub/phone_hub_manager_impl.cc

@@ -83,7 +83,8 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
           multidevice_setup_client,
           phone_model_.get())),
       tether_controller_(
-          std::make_unique<TetherControllerImpl>(multidevice_setup_client)),
+          std::make_unique<TetherControllerImpl>(phone_model_.get(),
+                                                 multidevice_setup_client)),
       browser_tabs_model_provider_(std::move(browser_tabs_model_provider)),
       browser_tabs_model_controller_(
           std::make_unique<BrowserTabsModelController>(

chromeos/components/phonehub/tether_controller_impl.cc

@@ -5,6 +5,7 @@
 #include "chromeos/components/phonehub/tether_controller_impl.h"
 
 #include "chromeos/components/multidevice/logging/logging.h"
+#include "chromeos/components/phonehub/phone_status_model.h"
 #include "chromeos/services/network_config/in_process_instance.h"
 
 namespace chromeos {
@@ -44,15 +45,19 @@ void TetherControllerImpl::TetherNetworkConnector::StartDisconnect(
 }
 
 TetherControllerImpl::TetherControllerImpl(
+    PhoneModel* phone_model,
     MultiDeviceSetupClient* multidevice_setup_client)
     : TetherControllerImpl(
+          phone_model,
           multidevice_setup_client,
           std::make_unique<TetherControllerImpl::TetherNetworkConnector>()) {}
 
 TetherControllerImpl::TetherControllerImpl(
+    PhoneModel* phone_model,
     MultiDeviceSetupClient* multidevice_setup_client,
     std::unique_ptr<TetherControllerImpl::TetherNetworkConnector> connector)
-    : multidevice_setup_client_(multidevice_setup_client),
+    : phone_model_(phone_model),
+      multidevice_setup_client_(multidevice_setup_client),
       connector_(std::move(connector)) {
   // Receive updates when devices (e.g., Tether, Ethernet, Wi-Fi) go on/offline
   // This class only cares about Tether devices.
@@ -60,6 +65,7 @@ TetherControllerImpl::TetherControllerImpl(
       cros_network_config_.BindNewPipeAndPassReceiver());
   cros_network_config_->AddObserver(receiver_.BindNewPipeAndPassRemote());
 
+  phone_model_->AddObserver(this);
   multidevice_setup_client_->AddObserver(this);
 
   // Compute current status.
@@ -70,6 +76,7 @@ TetherControllerImpl::TetherControllerImpl(
 }
 
 TetherControllerImpl::~TetherControllerImpl() {
+  phone_model_->RemoveObserver(this);
   multidevice_setup_client_->RemoveObserver(this);
 }
 
@@ -219,6 +226,10 @@ void TetherControllerImpl::Disconnect() {
                      weak_ptr_factory_.GetWeakPtr()));
 }
 
+void TetherControllerImpl::OnModelChanged() {
+  UpdateStatus();
+}
+
 void TetherControllerImpl::OnDisconnectCompleted(bool success) {
   if (connect_disconnect_status_ != ConnectDisconnectStatus::kDisconnecting)
     return;
@@ -353,6 +364,14 @@ void TetherControllerImpl::UpdateStatus() {
 }
 
 TetherController::Status TetherControllerImpl::ComputeStatus() const {
+  bool does_sim_exist_with_reception =
+      phone_model_->phone_status_model().has_value() &&
+      phone_model_->phone_status_model()->mobile_status() ==
+          PhoneStatusModel::MobileStatus::kSimWithReception;
+
+  if (!does_sim_exist_with_reception)
+    return Status::kIneligibleForFeature;
+
   FeatureState feature_state =
       multidevice_setup_client_->GetFeatureState(Feature::kInstantTethering);
 

chromeos/components/phonehub/tether_controller_impl.h

@@ -6,6 +6,7 @@
 #define CHROMEOS_COMPONENTS_PHONEHUB_TETHER_CONTROLLER_IMPL_H_
 
 #include "base/memory/weak_ptr.h"
+#include "chromeos/components/phonehub/phone_model.h"
 #include "chromeos/components/phonehub/tether_controller.h"
 #include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
 #include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
@@ -31,10 +32,12 @@ using multidevice_setup::MultiDeviceSetupClient;
 // if one exists.
 class TetherControllerImpl
     : public TetherController,
+      public PhoneModel::Observer,
       public multidevice_setup::MultiDeviceSetupClient::Observer,
       public chromeos::network_config::mojom::CrosNetworkConfigObserver {
  public:
   explicit TetherControllerImpl(
+      PhoneModel* phone_model,
       multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client);
   ~TetherControllerImpl() override;
 
@@ -104,9 +107,13 @@ class TetherControllerImpl
   // Two parameter constructor made available for testing purposes. The one
   // parameter constructor calls this constructor.
   TetherControllerImpl(
+      PhoneModel* phone_model,
       multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
       std::unique_ptr<TetherControllerImpl::TetherNetworkConnector> connector);
 
+  // PhoneModel::Observer:
+  void OnModelChanged() override;
+
   // multidevice_setup::MultiDeviceSetupClient::Observer:
   void OnFeatureStatesChanged(const MultiDeviceSetupClient::FeatureStatesMap&
                                   feature_states_map) override;
@@ -140,6 +147,7 @@ class TetherControllerImpl
   void UpdateStatus();
   TetherController::Status ComputeStatus() const;
 
+  PhoneModel* phone_model_;
   multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_;
   ConnectDisconnectStatus connect_disconnect_status_ =
       ConnectDisconnectStatus::kIdle;

chromeos/components/phonehub/tether_controller_impl_unittest.cc

@@ -7,7 +7,10 @@
 #include <memory>
 
 #include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/test/task_environment.h"
+#include "chromeos/components/phonehub/mutable_phone_model.h"
+#include "chromeos/components/phonehub/phone_status_model.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_state_test_helper.h"
 #include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
@@ -49,6 +52,21 @@ class FakeObserver : public TetherController::Observer {
   size_t num_scan_failed_ = 0;
 };
 
+PhoneStatusModel::MobileConnectionMetadata
+CreateFakeMobileConnectionMetadata() {
+  return {PhoneStatusModel::SignalStrength::kFourBars,
+          base::UTF8ToUTF16(kTetherNetworkCarrier)};
+}
+
+PhoneStatusModel CreateFakePhoneStatusModel(
+    PhoneStatusModel::MobileStatus mobile_status =
+        PhoneStatusModel::MobileStatus::kSimWithReception) {
+  return PhoneStatusModel(mobile_status, CreateFakeMobileConnectionMetadata(),
+                          PhoneStatusModel::ChargingState::kNotCharging,
+                          PhoneStatusModel::BatterySaverState::kOff,
+                          /*battery_percentage=*/100u);
+}
+
 }  // namespace
 
 class TetherControllerImplTest : public testing::Test {
@@ -112,9 +130,11 @@ class TetherControllerImplTest : public testing::Test {
             "Connectable": true})",
                 kWifiGuid));
 
+    fake_phone_model_.SetPhoneStatusModel(CreateFakePhoneStatusModel());
+
     controller_ =
         base::WrapUnique<TetherControllerImpl>(new TetherControllerImpl(
-            &fake_multidevice_setup_client_,
+            &fake_phone_model_, &fake_multidevice_setup_client_,
             std::make_unique<FakeTetherNetworkConnector>()));
     controller_->AddObserver(&fake_observer_);
   }
@@ -213,11 +233,14 @@ class TetherControllerImplTest : public testing::Test {
     return fake_observer_.num_scan_failed();
   }
 
+  MutablePhoneModel* phone_model() { return &fake_phone_model_; }
+
  private:
   base::test::TaskEnvironment task_environment_;
   network_config::CrosNetworkConfigTestHelper cros_network_config_helper_;
   std::string service_path_;
   multidevice_setup::FakeMultiDeviceSetupClient fake_multidevice_setup_client_;
+  MutablePhoneModel fake_phone_model_;
   FakeObserver fake_observer_;
   std::unique_ptr<TetherControllerImpl> controller_;
 };
@@ -254,6 +277,23 @@ TEST_F(TetherControllerImplTest, ExternalTetherChangesReflectToStatus) {
   RemoveVisibleTetherNetwork();
   EXPECT_EQ(GetStatus(), TetherController::Status::kConnectionUnavailable);
   EXPECT_EQ(GetNumObserverStatusChanged(), 6U);
+
+  // Phone status changed to no reception.
+  phone_model()->SetPhoneStatusModel(CreateFakePhoneStatusModel(
+      PhoneStatusModel::MobileStatus::kSimButNoReception));
+  EXPECT_EQ(GetStatus(), TetherController::Status::kIneligibleForFeature);
+  EXPECT_EQ(GetNumObserverStatusChanged(), 7U);
+
+  // Phone status changed to having reception. Connection is still unavailable.
+  phone_model()->SetPhoneStatusModel(CreateFakePhoneStatusModel(
+      PhoneStatusModel::MobileStatus::kSimWithReception));
+  EXPECT_EQ(GetStatus(), TetherController::Status::kConnectionUnavailable);
+  EXPECT_EQ(GetNumObserverStatusChanged(), 8U);
+
+  // Phone Model is lost.
+  phone_model()->SetPhoneStatusModel(base::nullopt);
+  EXPECT_EQ(GetStatus(), TetherController::Status::kIneligibleForFeature);
+  EXPECT_EQ(GetNumObserverStatusChanged(), 9U);
 }
 
 TEST_F(TetherControllerImplTest, AttemptConnectDisconnect) {