Phone Hub: Delay connected view until phone model is initialized

Previously, the connected Phone Hub UI would display before the phone
model is initialized. This resulted in the UI flashing the
uninitialized status model before the actual phone model status. This
fix delays the connected view from displaying until the model is
initialized.

Fixed: 1155636
Change-Id: Iadbbefbc3e5cee70316cfe02ef9d466888a2e6f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2581221
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835826}

ash/system/phonehub/phone_hub_tray_unittest.cc

@@ -15,6 +15,7 @@
 #include "chromeos/components/phonehub/fake_connection_scheduler.h"
 #include "chromeos/components/phonehub/fake_notification_access_manager.h"
 #include "chromeos/components/phonehub/fake_phone_hub_manager.h"
+#include "chromeos/components/phonehub/phone_model_test_util.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/events/event.h"
@@ -52,6 +53,9 @@ class PhoneHubTrayTest : public AshTestBase {
     GetFeatureStatusProvider()->SetStatus(
         chromeos::phonehub::FeatureStatus::kEnabledAndConnected);
     phone_hub_tray_->SetPhoneHubManager(&phone_hub_manager_);
+
+    phone_hub_manager_.mutable_phone_model()->SetPhoneStatusModel(
+        chromeos::phonehub::CreateFakePhoneStatusModel());
   }
 
   chromeos::phonehub::FakeFeatureStatusProvider* GetFeatureStatusProvider() {

ash/system/phonehub/phone_hub_ui_controller.cc

@@ -37,6 +37,7 @@ void PhoneHubUiController::SetPhoneHubManager(
   if (phone_hub_manager_) {
     phone_hub_manager_->GetFeatureStatusProvider()->AddObserver(this);
     phone_hub_manager_->GetOnboardingUiTracker()->AddObserver(this);
+    phone_hub_manager_->GetPhoneModel()->AddObserver(this);
   }
 
   UpdateUiState(GetUiStateFromPhoneHubManager());
@@ -102,6 +103,10 @@ void PhoneHubUiController::OnShouldShowOnboardingUiChanged() {
   UpdateUiState(GetUiStateFromPhoneHubManager());
 }
 
+void PhoneHubUiController::OnModelChanged() {
+  UpdateUiState(GetUiStateFromPhoneHubManager());
+}
+
 void PhoneHubUiController::UpdateUiState(
     PhoneHubUiController::UiState new_state) {
   if (new_state == ui_state_)
@@ -121,6 +126,7 @@ PhoneHubUiController::GetUiStateFromPhoneHubManager() {
       phone_hub_manager_->GetFeatureStatusProvider()->GetStatus();
 
   auto* tracker = phone_hub_manager_->GetOnboardingUiTracker();
+  auto* phone_model = phone_hub_manager_->GetPhoneModel();
   bool should_show_onboarding_ui = tracker->ShouldShowOnboardingUi();
 
   switch (feature_status) {
@@ -141,7 +147,11 @@ PhoneHubUiController::GetUiStateFromPhoneHubManager() {
     case FeatureStatus::kEnabledAndConnecting:
       return UiState::kPhoneConnecting;
     case FeatureStatus::kEnabledAndConnected:
-      return UiState::kPhoneConnected;
+      // Delay displaying the connected view until the phone model is ready.
+      if (phone_model->phone_status_model().has_value())
+        return UiState::kPhoneConnected;
+      else
+        return UiState::kPhoneConnecting;
     case FeatureStatus::kLockOrSuspended:
       return UiState::kHidden;
   }
@@ -153,6 +163,7 @@ void PhoneHubUiController::CleanUpPhoneHubManager() {
 
   phone_hub_manager_->GetFeatureStatusProvider()->RemoveObserver(this);
   phone_hub_manager_->GetOnboardingUiTracker()->RemoveObserver(this);
+  phone_hub_manager_->GetPhoneModel()->RemoveObserver(this);
 }
 
 }  // namespace ash

ash/system/phonehub/phone_hub_ui_controller.h

@@ -13,6 +13,7 @@
 #include "base/observer_list_types.h"
 #include "chromeos/components/phonehub/feature_status_provider.h"
 #include "chromeos/components/phonehub/onboarding_ui_tracker.h"
+#include "chromeos/components/phonehub/phone_model.h"
 
 namespace chromeos {
 namespace phonehub {
@@ -30,7 +31,8 @@ namespace ash {
 // corresponding main content view to be displayed in the tray bubble.
 class ASH_EXPORT PhoneHubUiController
     : public chromeos::phonehub::FeatureStatusProvider::Observer,
-      public chromeos::phonehub::OnboardingUiTracker::Observer {
+      public chromeos::phonehub::OnboardingUiTracker::Observer,
+      public chromeos::phonehub::PhoneModel::Observer {
  public:
   class Observer : public base::CheckedObserver {
    public:
@@ -86,6 +88,9 @@ class ASH_EXPORT PhoneHubUiController
   // chromeos::phonehub::OnboardingUiTracker::Observer:
   void OnShouldShowOnboardingUiChanged() override;
 
+  // chromeos::phonehub::PhoneModel::Observer:
+  void OnModelChanged() override;
+
   // Updates the current UI state and notifies observers.
   void UpdateUiState(PhoneHubUiController::UiState new_state);
 

ash/system/phonehub/phone_hub_ui_controller_unittest.cc

@@ -6,7 +6,9 @@
 
 #include "ash/system/phonehub/phone_hub_view_ids.h"
 #include "ash/test/ash_test_base.h"
+#include "base/optional.h"
 #include "chromeos/components/phonehub/fake_phone_hub_manager.h"
+#include "chromeos/components/phonehub/phone_model_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/views/view.h"
 
@@ -43,6 +45,13 @@ class PhoneHubUiControllerTest : public AshTestBase,
     return phone_hub_manager_.fake_onboarding_ui_tracker();
   }
 
+  void SetPhoneStatusModel(
+      const base::Optional<chromeos::phonehub::PhoneStatusModel>&
+          phone_status_model) {
+    phone_hub_manager_.mutable_phone_model()->SetPhoneStatusModel(
+        phone_status_model);
+  }
+
  protected:
   // PhoneHubUiController::Observer:
   void OnPhoneHubUiStateChanged() override {
@@ -136,6 +145,7 @@ TEST_F(PhoneHubUiControllerTest, PhoneConnecting) {
 }
 
 TEST_F(PhoneHubUiControllerTest, PhoneConnected) {
+  SetPhoneStatusModel(chromeos::phonehub::CreateFakePhoneStatusModel());
   GetFeatureStatusProvider()->SetStatus(FeatureStatus::kEnabledAndConnected);
   EXPECT_EQ(PhoneHubUiController::UiState::kPhoneConnected,
             controller_.ui_state());
@@ -150,4 +160,22 @@ TEST_F(PhoneHubUiControllerTest, UnavailableScreenLocked) {
   EXPECT_FALSE(controller_.CreateContentView(/*bubble_view=*/nullptr).get());
 }
 
+TEST_F(PhoneHubUiControllerTest, ConnectedViewDelayed) {
+  // Since there is no phone model, expect that we stay at the connecting screen
+  // even though the feature status is kEnabledAndConnected.
+  SetPhoneStatusModel(base::nullopt);
+  GetFeatureStatusProvider()->SetStatus(FeatureStatus::kEnabledAndConnected);
+  EXPECT_EQ(PhoneHubUiController::UiState::kPhoneConnecting,
+            controller_.ui_state());
+  auto content_view = controller_.CreateContentView(/*delegate=*/nullptr);
+  EXPECT_EQ(kPhoneConnectingView, content_view->GetID());
+
+  // Update the phone status model and expect the connected view to show up.
+  SetPhoneStatusModel(chromeos::phonehub::CreateFakePhoneStatusModel());
+  EXPECT_EQ(PhoneHubUiController::UiState::kPhoneConnected,
+            controller_.ui_state());
+  auto content_view2 = controller_.CreateContentView(/*delegate=*/nullptr);
+  EXPECT_EQ(kPhoneConnectedView, content_view2->GetID());
+}
+
 }  // namespace ash