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

This reverts commit a42b2a30.

Reason for revert: Causing failures on linux-chromeos-dbg and linux-chromeos-chrome - see https://crbug.com/1143173

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}

TBR=khorimoto@chromium.org,hsuregan@chromium.org

Change-Id: Id0ae4601f07b125f8e1e11b99b2de81ab0344bfd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1106937, 1143173
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2504857Reviewed-by: Austin Tankiang <austinct@chromium.org>
Commit-Queue: Austin Tankiang <austinct@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821641}

chromeos/components/phonehub/mutable_phone_model_unittest.cc

@@ -82,14 +82,13 @@ TEST_F(MutablePhoneModelTest, PhoneStatusModel) {
   EXPECT_EQ(0u, GetNumObserverCalls());
 
   // Set the PhoneStatusModel; observers should be notified.
-  auto fake_phone_status_model = CreateFakePhoneStatusModel();
-  model_.SetPhoneStatusModel(fake_phone_status_model);
-  EXPECT_EQ(fake_phone_status_model, model_.phone_status_model());
+  model_.SetPhoneStatusModel(CreateFakePhoneStatusModel());
+  EXPECT_EQ(CreateFakePhoneStatusModel(), model_.phone_status_model());
   EXPECT_EQ(1u, GetNumObserverCalls());
 
   // Set the same PhoneStatusModel; observers should not be notified.
-  model_.SetPhoneStatusModel(fake_phone_status_model);
-  EXPECT_EQ(fake_phone_status_model, model_.phone_status_model());
+  model_.SetPhoneStatusModel(CreateFakePhoneStatusModel());
+  EXPECT_EQ(CreateFakePhoneStatusModel(), model_.phone_status_model());
   EXPECT_EQ(1u, GetNumObserverCalls());
 
   // Set the PhoneStatusModel back to null; observers should be notified.

chromeos/components/phonehub/phone_hub_manager_impl.cc

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

chromeos/components/phonehub/phone_model_test_util.cc

@@ -21,10 +21,10 @@ CreateFakeMobileConnectionMetadata() {
   return *fake_mobile_connection_metadata;
 }
 
-const PhoneStatusModel& CreateFakePhoneStatusModel(
-    PhoneStatusModel::MobileStatus mobile_status) {
-  const base::NoDestructor<PhoneStatusModel> fake_phone_status_model{
-      mobile_status, CreateFakeMobileConnectionMetadata(),
+const PhoneStatusModel& CreateFakePhoneStatusModel() {
+  static const base::NoDestructor<PhoneStatusModel> fake_phone_status_model{
+      PhoneStatusModel::MobileStatus::kSimWithReception,
+      CreateFakeMobileConnectionMetadata(),
       PhoneStatusModel::ChargingState::kNotCharging,
       PhoneStatusModel::BatterySaverState::kOff,
       /*battery_percentage=*/100u};

chromeos/components/phonehub/phone_model_test_util.h

@@ -20,9 +20,7 @@ extern const char kFakeMobileProviderName[];
 // Creates fake phone status data for use in tests.
 const PhoneStatusModel::MobileConnectionMetadata&
 CreateFakeMobileConnectionMetadata();
-const PhoneStatusModel& CreateFakePhoneStatusModel(
-    PhoneStatusModel::MobileStatus mobile_status =
-        PhoneStatusModel::MobileStatus::kSimWithReception);
+const PhoneStatusModel& CreateFakePhoneStatusModel();
 
 // Fake data for browser tabs.
 extern const char kFakeBrowserTabUrl1[];

chromeos/components/phonehub/tether_controller_impl.cc

@@ -5,7 +5,6 @@
 #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 {
@@ -45,19 +44,15 @@ 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)
-    : phone_model_(phone_model),
-      multidevice_setup_client_(multidevice_setup_client),
+    : 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.
@@ -65,7 +60,6 @@ TetherControllerImpl::TetherControllerImpl(
       cros_network_config_.BindNewPipeAndPassReceiver());
   cros_network_config_->AddObserver(receiver_.BindNewPipeAndPassRemote());
 
-  phone_model_->AddObserver(this);
   multidevice_setup_client_->AddObserver(this);
 
   // Compute current status.
@@ -76,7 +70,6 @@ TetherControllerImpl::TetherControllerImpl(
 }
 
 TetherControllerImpl::~TetherControllerImpl() {
-  phone_model_->RemoveObserver(this);
   multidevice_setup_client_->RemoveObserver(this);
 }
 
@@ -226,10 +219,6 @@ void TetherControllerImpl::Disconnect() {
                      weak_ptr_factory_.GetWeakPtr()));
 }
 
-void TetherControllerImpl::OnModelChanged() {
-  UpdateStatus();
-}
-
 void TetherControllerImpl::OnDisconnectCompleted(bool success) {
   if (connect_disconnect_status_ != ConnectDisconnectStatus::kDisconnecting)
     return;
@@ -364,14 +353,6 @@ 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,7 +6,6 @@
 #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"
@@ -32,12 +31,10 @@ 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;
 
@@ -107,13 +104,9 @@ 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;
@@ -147,7 +140,6 @@ 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

@@ -8,8 +8,6 @@
 
 #include "base/strings/stringprintf.h"
 #include "base/test/task_environment.h"
-#include "chromeos/components/phonehub/mutable_phone_model.h"
-#include "chromeos/components/phonehub/phone_model_test_util.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"
@@ -114,11 +112,9 @@ class TetherControllerImplTest : public testing::Test {
             "Connectable": true})",
                 kWifiGuid));
 
-    fake_phone_model_.SetPhoneStatusModel(CreateFakePhoneStatusModel());
-
     controller_ =
         base::WrapUnique<TetherControllerImpl>(new TetherControllerImpl(
-            &fake_phone_model_, &fake_multidevice_setup_client_,
+            &fake_multidevice_setup_client_,
             std::make_unique<FakeTetherNetworkConnector>()));
     controller_->AddObserver(&fake_observer_);
   }
@@ -217,14 +213,11 @@ 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_;
 };
@@ -261,23 +254,6 @@ 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) {