[CrOS PhoneHub] Add BrowserTabsModel controller and provider and edits.

The controller sets the MutablePhoneModel using info observed from the
the provider class. The provider get browser tab info via SyncedSession
(available via SessionService) with a |session_name| that matches the
|pii_free_name| of the phone provided by a MultiDeviceSetupClient.
If sync is enabled, the class uses a BrowserTabsMetadataFetcher to
actually fetch the browser tab metadata once it finds the correct
SyncedSession.

This CL also replaces FaviconService with HistoryUiFaviconRequestHandler
because FaviconService often fails at fetching the favicon.

HistoryUiFaviconRequestHandler, from manual testing, has always
succeeded. This is because HistoryUiFaviconRequestHandler fetches
favicons from google servers instead of the endpoint directly. If
it can't find one on google server, it fetches the favicon from the url
and caches it on the server.

Bug: 1106937
Change-Id: Id2f4949b344d7685a814b7635d2f7986117d10ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427297
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813463}

chrome/browser/chromeos/BUILD.gn

@@ -1940,6 +1940,8 @@ source_set("chromeos") {
     "ownership/owner_settings_service_chromeos_factory.h",
     "phonehub/browser_tabs_metadata_fetcher_impl.cc",
     "phonehub/browser_tabs_metadata_fetcher_impl.h",
+    "phonehub/browser_tabs_model_provider_impl.cc",
+    "phonehub/browser_tabs_model_provider_impl.h",
     "phonehub/phone_hub_manager_factory.cc",
     "phonehub/phone_hub_manager_factory.h",
     "platform_keys/extension_platform_keys_service.cc",
@@ -3441,6 +3443,7 @@ source_set("unit_tests") {
     "note_taking_helper_unittest.cc",
     "ownership/owner_settings_service_chromeos_unittest.cc",
     "phonehub/browser_tabs_metadata_fetcher_impl_unittest.cc",
+    "phonehub/browser_tabs_model_provider_impl_unittest.cc",
     "platform_keys/key_permissions/key_permissions_service_impl_unittest.cc",
     "plugin_vm/mock_plugin_vm_manager.cc",
     "plugin_vm/mock_plugin_vm_manager.h",

chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_impl.cc

@@ -5,7 +5,8 @@
 #include "chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_impl.h"
 
 #include "base/barrier_closure.h"
-#include "components/favicon/core/favicon_service.h"
+#include "components/favicon/core/history_ui_favicon_request_handler.h"
+#include "components/favicon_base/favicon_types.h"
 #include "components/sync_sessions/synced_session.h"
 
 namespace chromeos {
@@ -55,8 +56,8 @@ GetSortedMetadataWithoutFavicons(const sync_sessions::SyncedSession* session) {
 }  // namespace
 
 BrowserTabsMetadataFetcherImpl::BrowserTabsMetadataFetcherImpl(
-    favicon::FaviconService* favicon_service)
-    : favicon_service_(favicon_service) {}
+    favicon::HistoryUiFaviconRequestHandler* favicon_request_handler)
+    : favicon_request_handler_(favicon_request_handler) {}
 
 BrowserTabsMetadataFetcherImpl::~BrowserTabsMetadataFetcherImpl() = default;
 
@@ -66,7 +67,6 @@ void BrowserTabsMetadataFetcherImpl::Fetch(
   // A new fetch was made, return a base::nullopt to the previous |callback_|.
   if (!callback_.is_null()) {
     weak_ptr_factory_.InvalidateWeakPtrs();
-    favicon_tracker_.TryCancelAll();
     std::move(callback_).Run(base::nullopt);
   }
 
@@ -81,11 +81,11 @@ void BrowserTabsMetadataFetcherImpl::Fetch(
                      weak_ptr_factory_.GetWeakPtr()));
 
   for (size_t i = 0; i < results_.size(); ++i) {
-    favicon_service_->GetFaviconImageForPageURL(
+    favicon_request_handler_->GetFaviconImageForPageURL(
         results_[i].url,
         base::BindOnce(&BrowserTabsMetadataFetcherImpl::OnFaviconReady,
                        weak_ptr_factory_.GetWeakPtr(), i, barrier),
-        &favicon_tracker_);
+        favicon::HistoryUiFaviconRequestOrigin::kRecentTabs);
   }
 }
 

chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_impl.h

@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_CHROMEOS_PHONEHUB_BROWSER_TABS_METADATA_FETCHER_IMPL_H_
 
 #include "base/memory/weak_ptr.h"
-#include "base/task/cancelable_task_tracker.h"
 #include "chromeos/components/phonehub/browser_tabs_metadata_fetcher.h"
 
 namespace favicon_base {
@@ -14,7 +13,7 @@ struct FaviconImageResult;
 }  // namespace favicon_base
 
 namespace favicon {
-class FaviconService;
+class HistoryUiFaviconRequestHandler;
 }  // namespace favicon
 
 namespace chromeos {
@@ -23,15 +22,15 @@ namespace phonehub {
 // BrowserTabsMetadataFetcher implementation. First, a vector containing
 // metadata of the most recently visited tab to least recently visited is
 // created. The metadata is stored from data provided by a SyncedSession. After
-// the ordered vector is created, the FaviconService is used to asynchronously
-// fetch favicon images for the most recently visited tabs. Once all the
-// favicons for the most recently visited tabs (up to
+// the ordered vector is created, the HistoryUiFaviconRequestHandler is used to
+// asynchronously fetch favicon images for the most recently visited tabs. Once
+// all the favicons for the most recently visited tabs (up to
 // BrowserTabsModel::kMaxMostRecentTabs) have been fetched, |results_| is
 // invoked with the callback passed to the class.
 class BrowserTabsMetadataFetcherImpl : public BrowserTabsMetadataFetcher {
  public:
   explicit BrowserTabsMetadataFetcherImpl(
-      favicon::FaviconService* favicon_service);
+      favicon::HistoryUiFaviconRequestHandler* favicon_request_handler);
   ~BrowserTabsMetadataFetcherImpl() override;
 
   // BrowserTabsMetadataFetcher:
@@ -46,13 +45,10 @@ class BrowserTabsMetadataFetcherImpl : public BrowserTabsMetadataFetcher {
       base::OnceClosure done_closure,
       const favicon_base::FaviconImageResult& favicon_image_result);
 
-  favicon::FaviconService* const favicon_service_;
+  favicon::HistoryUiFaviconRequestHandler* const favicon_request_handler_;
   std::vector<BrowserTabsModel::BrowserTabMetadata> results_;
   base::OnceCallback<void(BrowserTabsMetadataResponse)> callback_;
 
-  // Used to track a requested favicon.
-  base::CancelableTaskTracker favicon_tracker_;
-
   base::WeakPtrFactory<BrowserTabsMetadataFetcherImpl> weak_ptr_factory_{this};
 };
 

chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_impl_unittest.cc

@@ -5,7 +5,8 @@
 #include "chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_impl.h"
 
 #include "base/strings/utf_string_conversions.h"
-#include "components/favicon/core/test/mock_favicon_service.h"
+#include "components/favicon/core/history_ui_favicon_request_handler.h"
+#include "components/favicon_base/favicon_types.h"
 #include "components/sessions/core/serialized_navigation_entry_test_helper.h"
 #include "components/sync_sessions/synced_session.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -26,6 +27,26 @@ const base::Time kTimeC = base::Time::FromDoubleT(3);
 const base::Time kTimeD = base::Time::FromDoubleT(4);
 const base::Time kTimeE = base::Time::FromDoubleT(5);
 
+class MockHistoryUiFaviconRequestHandler
+    : public favicon::HistoryUiFaviconRequestHandler {
+ public:
+  MockHistoryUiFaviconRequestHandler() = default;
+  ~MockHistoryUiFaviconRequestHandler() override = default;
+
+  MOCK_METHOD4(
+      GetRawFaviconForPageURL,
+      void(const GURL& page_url,
+           int desired_size_in_pixel,
+           favicon_base::FaviconRawBitmapCallback callback,
+           favicon::HistoryUiFaviconRequestOrigin request_origin_for_uma));
+
+  MOCK_METHOD3(
+      GetFaviconImageForPageURL,
+      void(const GURL& page_url,
+           favicon_base::FaviconImageCallback callback,
+           favicon::HistoryUiFaviconRequestOrigin request_origin_for_uma));
+};
+
 gfx::Image GetDummyImage() {
   SkBitmap bitmap;
   bitmap.allocN32Pixels(gfx::kFaviconSize, gfx::kFaviconSize);
@@ -44,7 +65,7 @@ favicon_base::FaviconImageResult GetDummyFaviconResult() {
 class BrowserTabsMetadataFetcherImplTest : public testing::Test {
  public:
   BrowserTabsMetadataFetcherImplTest()
-      : browser_tabs_metadata_job_(&favicon_service_),
+      : browser_tabs_metadata_job_(&favicon_request_handler_),
         synced_session_(std::make_unique<sync_sessions::SyncedSession>()) {}
 
   BrowserTabsMetadataFetcherImplTest(
@@ -92,16 +113,18 @@ class BrowserTabsMetadataFetcherImplTest : public testing::Test {
   }
 
   void ExpectFaviconUrlFetchAttempt(const GURL& url) {
-    EXPECT_CALL(favicon_service_, GetFaviconImageForPageURL(url, /*callback=*/_,
-                                                            /*tracker=*/_))
+    EXPECT_CALL(favicon_request_handler_,
+                GetFaviconImageForPageURL(url, /*callback=*/_,
+                                          /*request_origin_for_uma=*/_))
         .WillRepeatedly(
             [&](auto, favicon_base::FaviconImageCallback callback, auto) {
               // Randomize the order in which callbacks may return.
               if (std::rand() % 2)
-                favicon_service_responses_.emplace_front(std::move(callback));
+                favicon_request_handler_responses_.emplace_front(
+                    std::move(callback));
               else
-                favicon_service_responses_.emplace_back(std::move(callback));
-              return base::CancelableTaskTracker::kBadTaskId;
+                favicon_request_handler_responses_.emplace_back(
+                    std::move(callback));
             });
   }
 
@@ -115,9 +138,9 @@ class BrowserTabsMetadataFetcherImplTest : public testing::Test {
 
   void InvokeNextFaviconCallbacks(size_t num_successful_fetches) {
     for (size_t i = 0; i < num_successful_fetches; i++) {
-      std::move(favicon_service_responses_.front())
+      std::move(favicon_request_handler_responses_.front())
           .Run(GetDummyFaviconResult());
-      favicon_service_responses_.pop_front();
+      favicon_request_handler_responses_.pop_front();
     }
   }
 
@@ -139,7 +162,8 @@ class BrowserTabsMetadataFetcherImplTest : public testing::Test {
   }
 
  private:
-  testing::NiceMock<favicon::MockFaviconService> favicon_service_;
+  testing::NiceMock<MockHistoryUiFaviconRequestHandler>
+      favicon_request_handler_;
   BrowserTabsMetadataFetcherImpl browser_tabs_metadata_job_;
   base::Optional<std::vector<BrowserTabsModel::BrowserTabMetadata>>
       actual_browser_tabs_metadata_;
@@ -147,7 +171,8 @@ class BrowserTabsMetadataFetcherImplTest : public testing::Test {
   std::map<SessionID, std::unique_ptr<sync_sessions::SyncedSessionWindow>>
       windows;
   std::unique_ptr<sync_sessions::SyncedSession> synced_session_;
-  std::deque<favicon_base::FaviconImageCallback> favicon_service_responses_;
+  std::deque<favicon_base::FaviconImageCallback>
+      favicon_request_handler_responses_;
 };
 
 TEST_F(BrowserTabsMetadataFetcherImplTest, NewFetchDuringOldFetchInProgress) {

chrome/browser/chromeos/phonehub/browser_tabs_model_provider_impl.cc

+// Copyright 2020 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 "chrome/browser/chromeos/phonehub/browser_tabs_model_provider_impl.h"
+
+#include "chromeos/components/multidevice/remote_device_ref.h"
+#include "chromeos/components/phonehub/browser_tabs_metadata_fetcher.h"
+#include "chromeos/components/phonehub/browser_tabs_model.h"
+#include "components/sync_sessions/open_tabs_ui_delegate.h"
+#include "components/sync_sessions/session_sync_service.h"
+
+namespace chromeos {
+namespace phonehub {
+
+BrowserTabsModelProviderImpl::BrowserTabsModelProviderImpl(
+    multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
+    sync_sessions::SessionSyncService* session_sync_service,
+    std::unique_ptr<BrowserTabsMetadataFetcher> browser_tabs_metadata_fetcher)
+    : multidevice_setup_client_(multidevice_setup_client),
+      session_sync_service_(session_sync_service),
+      browser_tabs_metadata_fetcher_(std::move(browser_tabs_metadata_fetcher)) {
+  multidevice_setup_client_->AddObserver(this);
+  session_updated_subscription_ =
+      session_sync_service->SubscribeToForeignSessionsChanged(
+          base::BindRepeating(
+              &BrowserTabsModelProviderImpl::AttemptBrowserTabsModelUpdate,
+              base::Unretained(this)));
+  AttemptBrowserTabsModelUpdate();
+}
+
+BrowserTabsModelProviderImpl::~BrowserTabsModelProviderImpl() {
+  multidevice_setup_client_->RemoveObserver(this);
+}
+
+base::Optional<std::string> BrowserTabsModelProviderImpl::GetSessionName()
+    const {
+  const multidevice_setup::MultiDeviceSetupClient::HostStatusWithDevice&
+      host_device_with_status = multidevice_setup_client_->GetHostStatus();
+  if (!host_device_with_status.second)
+    return base::nullopt;
+  // The pii_free_name field of the device matches the session name for
+  // sync.
+  return host_device_with_status.second->pii_free_name();
+}
+
+void BrowserTabsModelProviderImpl::OnHostStatusChanged(
+    const multidevice_setup::MultiDeviceSetupClient::HostStatusWithDevice&
+        host_device_with_status) {
+  AttemptBrowserTabsModelUpdate();
+}
+
+void BrowserTabsModelProviderImpl::AttemptBrowserTabsModelUpdate() {
+  base::Optional<std::string> session_name = GetSessionName();
+  sync_sessions::OpenTabsUIDelegate* open_tabs =
+      session_sync_service_->GetOpenTabsUIDelegate();
+  // Tab sync is disabled or no valid |pii_free_name_|.
+  if (!open_tabs || !session_name) {
+    InvalidateWeakPtrsAndClearTabMetadata(/*is_tab_sync_enabled=*/false);
+    return;
+  }
+
+  std::vector<const sync_sessions::SyncedSession*> sessions;
+  bool was_fetch_successful = open_tabs->GetAllForeignSessions(&sessions);
+  // No tabs were found, clear all tab metadata.
+  if (!was_fetch_successful) {
+    InvalidateWeakPtrsAndClearTabMetadata(/*is_tab_sync_enabled=*/true);
+    return;
+  }
+
+  // |phone_session| should have the same |session_name| as the
+  // |pii_free_name|. If there is more than one session with the same
+  // |session_name| as the |pii_free_name|, as would be the case if a user has
+  // multiple phones of the same type, |phone_session| will have the latest
+  // |modified_time|.
+  const sync_sessions::SyncedSession* phone_session = nullptr;
+  for (const auto* session : sessions) {
+    if (session->session_name != *session_name)
+      continue;
+
+    if (!phone_session ||
+        phone_session->modified_time < session->modified_time) {
+      phone_session = session;
+    }
+  }
+
+  // No session with the same name as |pii_free_name_| was found, clear all
+  // tab metadata.
+  if (!phone_session) {
+    InvalidateWeakPtrsAndClearTabMetadata(/*is_tab_sync_enabled=*/true);
+    return;
+  }
+
+  browser_tabs_metadata_fetcher_->Fetch(
+      phone_session,
+      base::BindOnce(&BrowserTabsModelProviderImpl::OnMetadataFetched,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+void BrowserTabsModelProviderImpl::InvalidateWeakPtrsAndClearTabMetadata(
+    bool is_tab_sync_enabled) {
+  weak_ptr_factory_.InvalidateWeakPtrs();
+  BrowserTabsModelProvider::NotifyBrowserTabsUpdated(
+      /*is_tab_sync_enabled=*/is_tab_sync_enabled, {});
+}
+
+void BrowserTabsModelProviderImpl::OnMetadataFetched(
+    base::Optional<std::vector<BrowserTabsModel::BrowserTabMetadata>>
+        metadata) {
+  // The operation to fetch metadata was cancelled.
+  if (!metadata)
+    return;
+  BrowserTabsModelProvider::NotifyBrowserTabsUpdated(
+      /*is_tab_sync_enabled=*/true, *metadata);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chrome/browser/chromeos/phonehub/browser_tabs_model_provider_impl.h

+// Copyright 2020 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 CHROME_BROWSER_CHROMEOS_PHONEHUB_BROWSER_TABS_MODEL_PROVIDER_IMPL_H_
+#define CHROME_BROWSER_CHROMEOS_PHONEHUB_BROWSER_TABS_MODEL_PROVIDER_IMPL_H_
+
+#include <ostream>
+
+#include "base/callback_list.h"
+#include "base/memory/weak_ptr.h"
+#include "chromeos/components/phonehub/browser_tabs_metadata_fetcher.h"
+#include "chromeos/components/phonehub/browser_tabs_model_provider.h"
+#include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
+
+namespace sync_sessions {
+class SessionSyncService;
+}  // namespace sync_sessions
+
+namespace chromeos {
+namespace phonehub {
+
+// Responsible for providing the BrowserTabsModel to observers.
+class BrowserTabsModelProviderImpl
+    : public BrowserTabsModelProvider,
+      public multidevice_setup::MultiDeviceSetupClient::Observer {
+ public:
+  BrowserTabsModelProviderImpl(
+      multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
+      sync_sessions::SessionSyncService* session_sync_service,
+      std::unique_ptr<BrowserTabsMetadataFetcher>
+          browser_tabs_metadata_fetcher);
+  ~BrowserTabsModelProviderImpl() override;
+
+ private:
+  friend class BrowserTabsModelProviderImplTest;
+
+  // multidevice_setup::MultiDeviceSetupClient::Observer:
+  void OnHostStatusChanged(
+      const multidevice_setup::MultiDeviceSetupClient::HostStatusWithDevice&
+          host_device_with_status) override;
+
+  void AttemptBrowserTabsModelUpdate();
+  void InvalidateWeakPtrsAndClearTabMetadata(bool is_tab_sync_enabled);
+  void OnMetadataFetched(
+      base::Optional<std::vector<BrowserTabsModel::BrowserTabMetadata>>
+          metadata);
+  base::Optional<std::string> GetSessionName() const;
+
+  multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_;
+  sync_sessions::SessionSyncService* session_sync_service_;
+  std::unique_ptr<BrowserTabsMetadataFetcher> browser_tabs_metadata_fetcher_;
+  std::unique_ptr<base::CallbackList<void()>::Subscription>
+      session_updated_subscription_;
+
+  base::WeakPtrFactory<BrowserTabsModelProviderImpl> weak_ptr_factory_{this};
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_PHONEHUB_BROWSER_TABS_MODEL_PROVIDER_IMPL_H_

chrome/browser/chromeos/phonehub/browser_tabs_model_provider_impl_unittest.cc

+// Copyright 2020 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 "chrome/browser/chromeos/phonehub/browser_tabs_model_provider_impl.h"
+
+#include "chromeos/components/multidevice/remote_device_test_util.h"
+#include "chromeos/components/phonehub/fake_browser_tabs_metadata_fetcher.h"
+#include "chromeos/components/phonehub/mutable_phone_model.h"
+#include "chromeos/components/phonehub/phone_model_test_util.h"
+#include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
+#include "components/sync_sessions/open_tabs_ui_delegate.h"
+#include "components/sync_sessions/session_sync_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+namespace phonehub {
+
+using testing::_;
+using testing::Return;
+
+namespace {
+
+const char kPhoneNameOne[] = "Pixel";
+const char kPhoneNameTwo[] = "Galaxy";
+
+class SessionSyncServiceMock : public sync_sessions::SessionSyncService {
+ public:
+  SessionSyncServiceMock() {}
+  ~SessionSyncServiceMock() override {}
+
+  MOCK_CONST_METHOD0(GetGlobalIdMapper, syncer::GlobalIdMapper*());
+  MOCK_METHOD0(GetOpenTabsUIDelegate, sync_sessions::OpenTabsUIDelegate*());
+  MOCK_METHOD1(SubscribeToForeignSessionsChanged,
+               std::unique_ptr<base::CallbackList<void()>::Subscription>(
+                   const base::RepeatingClosure& cb));
+  MOCK_METHOD0(ScheduleGarbageCollection, void());
+  MOCK_METHOD0(GetControllerDelegate,
+               base::WeakPtr<syncer::ModelTypeControllerDelegate>());
+  MOCK_METHOD1(ProxyTabsStateChanged,
+               void(syncer::DataTypeController::State state));
+  MOCK_METHOD1(SetSyncSessionsGUID, void(const std::string& guid));
+};
+
+class OpenTabsUIDelegateMock : public sync_sessions::OpenTabsUIDelegate {
+ public:
+  OpenTabsUIDelegateMock() {}
+  ~OpenTabsUIDelegateMock() override {}
+
+  MOCK_METHOD1(
+      GetAllForeignSessions,
+      bool(std::vector<const sync_sessions::SyncedSession*>* sessions));
+  MOCK_METHOD3(GetForeignTab,
+               bool(const std::string& tag,
+                    const SessionID tab_id,
+                    const sessions::SessionTab** tab));
+  MOCK_METHOD1(DeleteForeignSession, void(const std::string& tag));
+  MOCK_METHOD2(GetForeignSession,
+               bool(const std::string& tag,
+                    std::vector<const sessions::SessionWindow*>* windows));
+  MOCK_METHOD2(GetForeignSessionTabs,
+               bool(const std::string& tag,
+                    std::vector<const sessions::SessionTab*>* tabs));
+  MOCK_METHOD1(GetLocalSession,
+               bool(const sync_sessions::SyncedSession** local));
+};
+
+multidevice::RemoteDeviceRef CreatePhoneDevice(const std::string& pii_name) {
+  multidevice::RemoteDeviceRefBuilder builder;
+  builder.SetPiiFreeName(pii_name);
+  return builder.Build();
+}
+
+sync_sessions::SyncedSession* CreateNewSession(
+    const std::string& session_name,
+    const base::Time& session_time = base::Time::FromDoubleT(0)) {
+  sync_sessions::SyncedSession* session = new sync_sessions::SyncedSession();
+  session->session_name = session_name;
+  session->modified_time = session_time;
+  return session;
+}
+
+}  // namespace
+
+class BrowserTabsModelProviderImplTest
+    : public testing::Test,
+      public BrowserTabsModelProvider::Observer {
+ public:
+  BrowserTabsModelProviderImplTest() = default;
+
+  BrowserTabsModelProviderImplTest(const BrowserTabsModelProviderImplTest&) =
+      delete;
+  BrowserTabsModelProviderImplTest& operator=(
+      const BrowserTabsModelProviderImplTest&) = delete;
+  ~BrowserTabsModelProviderImplTest() override = default;
+
+  void OnBrowserTabsUpdated(
+      bool is_sync_enabled,
+      const std::vector<BrowserTabsModel::BrowserTabMetadata>&
+          browser_tabs_metadata) override {
+    phone_model_.SetBrowserTabsModel(BrowserTabsModel(
+        /*is_tab_sync_enabled=*/is_sync_enabled, browser_tabs_metadata));
+  }
+
+  // testing::Test:
+  void SetUp() override {
+    ON_CALL(mock_session_sync_service_, GetOpenTabsUIDelegate())
+        .WillByDefault(Invoke(
+            this, &BrowserTabsModelProviderImplTest::open_tabs_ui_delegate));
+
+    ON_CALL(open_tabs_ui_delegate_, GetAllForeignSessions(_))
+        .WillByDefault(Invoke(
+            this,
+            &BrowserTabsModelProviderImplTest::MockGetAllForeignSessions));
+
+    ON_CALL(mock_session_sync_service_, SubscribeToForeignSessionsChanged(_))
+        .WillByDefault(Invoke(this, &BrowserTabsModelProviderImplTest::
+                                        MockSubscribeToForeignSessionsChanged));
+
+    provider_ = std::make_unique<BrowserTabsModelProviderImpl>(
+        &fake_multidevice_setup_client_, &mock_session_sync_service_,
+        std::make_unique<FakeBrowserTabsMetadataFetcher>());
+    provider_->AddObserver(this);
+  }
+
+  void SetPiiFreeName(const std::string& pii_free_name) {
+    fake_multidevice_setup_client_.SetHostStatusWithDevice(std::make_pair(
+        multidevice_setup::mojom::HostStatus::kEligibleHostExistsButNoHostSet,
+        CreatePhoneDevice(/*pii_name=*/pii_free_name)));
+  }
+
+  std::unique_ptr<base::CallbackList<void()>::Subscription>
+  MockSubscribeToForeignSessionsChanged(const base::RepeatingClosure& cb) {
+    foreign_sessions_changed_callback_ = std::move(cb);
+    return nullptr;
+  }
+
+  bool MockGetAllForeignSessions(
+      std::vector<const sync_sessions::SyncedSession*>* sessions) {
+    if (sessions_) {
+      *sessions = *sessions_;
+      return !sessions->empty();
+    }
+    return false;
+  }
+
+  testing::NiceMock<OpenTabsUIDelegateMock>* open_tabs_ui_delegate() {
+    return enable_tab_sync_ ? &open_tabs_ui_delegate_ : nullptr;
+  }
+
+  void NotifySubscription() { foreign_sessions_changed_callback_.Run(); }
+
+  void set_synced_sessions(
+      std::vector<const sync_sessions::SyncedSession*>* sessions) {
+    sessions_ = sessions;
+  }
+
+  void set_enable_tab_sync(bool is_enabled) { enable_tab_sync_ = is_enabled; }
+
+  FakeBrowserTabsMetadataFetcher* fake_browser_tabs_metadata_fetcher() {
+    return static_cast<FakeBrowserTabsMetadataFetcher*>(
+        provider_->browser_tabs_metadata_fetcher_.get());
+  }
+
+  MutablePhoneModel phone_model_;
+  multidevice_setup::FakeMultiDeviceSetupClient fake_multidevice_setup_client_;
+  testing::NiceMock<SessionSyncServiceMock> mock_session_sync_service_;
+  std::unique_ptr<BrowserTabsModelProviderImpl> provider_;
+
+  testing::NiceMock<OpenTabsUIDelegateMock> open_tabs_ui_delegate_;
+
+  bool enable_tab_sync_ = true;
+  std::vector<const sync_sessions::SyncedSession*>* sessions_ = nullptr;
+  base::RepeatingClosure foreign_sessions_changed_callback_;
+};
+
+TEST_F(BrowserTabsModelProviderImplTest, AttemptBrowserTabsModelUpdate) {
+  // Test no Pii Free name despite sync being enabled.
+  set_enable_tab_sync(true);
+  set_synced_sessions(nullptr);
+  NotifySubscription();
+  EXPECT_FALSE(phone_model_.browser_tabs_model()->is_tab_sync_enabled());
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->most_recent_tabs().empty());
+  EXPECT_FALSE(
+      fake_browser_tabs_metadata_fetcher()->DoesPendingCallbackExist());
+
+  // Set name of phone. Tests that OnHostStatusChanged causes name change.
+  SetPiiFreeName(kPhoneNameOne);
+
+  // Test disabling tab sync with no browser tab metadata.
+  set_enable_tab_sync(false);
+  set_synced_sessions(nullptr);
+  NotifySubscription();
+  EXPECT_FALSE(phone_model_.browser_tabs_model()->is_tab_sync_enabled());
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->most_recent_tabs().empty());
+  EXPECT_FALSE(
+      fake_browser_tabs_metadata_fetcher()->DoesPendingCallbackExist());
+
+  // Test enabling tab sync with no browser tab metadata.
+  set_enable_tab_sync(true);
+  set_synced_sessions(nullptr);
+  NotifySubscription();
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->is_tab_sync_enabled());
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->most_recent_tabs().empty());
+  EXPECT_FALSE(
+      fake_browser_tabs_metadata_fetcher()->DoesPendingCallbackExist());
+
+  // Test enabling tab sync with no matching pii name with session_name.
+  std::vector<const sync_sessions::SyncedSession*> sessions;
+  sync_sessions::SyncedSession* session = CreateNewSession(kPhoneNameTwo);
+  sessions.emplace_back(session);
+  set_enable_tab_sync(true);
+  set_synced_sessions(&sessions);
+  NotifySubscription();
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->is_tab_sync_enabled());
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->most_recent_tabs().empty());
+  EXPECT_FALSE(
+      fake_browser_tabs_metadata_fetcher()->DoesPendingCallbackExist());
+
+  // Test enabling tab sync with matching pii name with session_name, which
+  // will cause the |fake_browser_tabs_metadata_fetcher()| to have a pending
+  // callback.
+  sync_sessions::SyncedSession* new_session = CreateNewSession(kPhoneNameOne);
+  sessions.emplace_back(new_session);
+  set_enable_tab_sync(true);
+  set_synced_sessions(&sessions);
+  NotifySubscription();
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->is_tab_sync_enabled());
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->most_recent_tabs().empty());
+  EXPECT_TRUE(fake_browser_tabs_metadata_fetcher()->DoesPendingCallbackExist());
+
+  // Test that once |fake_browser_tabs_metadata_fetcher()| responds, the phone
+  // model will be appropriately updated.
+  std::vector<BrowserTabsModel::BrowserTabMetadata> metadata;
+  metadata.push_back(CreateFakeBrowserTabMetadata());
+  fake_browser_tabs_metadata_fetcher()->RespondToCurrentFetchAttempt(
+      std::move(metadata));
+  EXPECT_EQ(phone_model_.browser_tabs_model()->most_recent_tabs().size(), 1U);
+}
+
+TEST_F(BrowserTabsModelProviderImplTest, ClearTabMetadataDuringMetadataFetch) {
+  SetPiiFreeName(kPhoneNameOne);
+  sync_sessions::SyncedSession* new_session = CreateNewSession(kPhoneNameOne);
+  std::vector<const sync_sessions::SyncedSession*> sessions({new_session});
+
+  set_enable_tab_sync(true);
+  set_synced_sessions(&sessions);
+  NotifySubscription();
+  EXPECT_TRUE(fake_browser_tabs_metadata_fetcher()->DoesPendingCallbackExist());
+
+  // Set to no synced sessions. Tab sync is still enabled.
+  set_synced_sessions(nullptr);
+  NotifySubscription();
+
+  // Test that if the Browser tab metadata is cleared while a browser tab
+  // metadata fetch is in progress, the in progress callback will be cancelled.
+  std::vector<BrowserTabsModel::BrowserTabMetadata> metadata;
+  metadata.push_back(CreateFakeBrowserTabMetadata());
+  fake_browser_tabs_metadata_fetcher()->RespondToCurrentFetchAttempt(
+      std::move(metadata));
+  EXPECT_TRUE(phone_model_.browser_tabs_model()->most_recent_tabs().empty());
+}
+
+TEST_F(BrowserTabsModelProviderImplTest, SessionCorrectlySelected) {
+  SetPiiFreeName(kPhoneNameOne);
+  sync_sessions::SyncedSession* session_a =
+      CreateNewSession(kPhoneNameOne, base::Time::FromDoubleT(1));
+  sync_sessions::SyncedSession* session_b =
+      CreateNewSession(kPhoneNameOne, base::Time::FromDoubleT(3));
+  sync_sessions::SyncedSession* session_c =
+      CreateNewSession(kPhoneNameOne, base::Time::FromDoubleT(2));
+  sync_sessions::SyncedSession* session_d =
+      CreateNewSession(kPhoneNameTwo, base::Time::FromDoubleT(10));
+
+  std::vector<const sync_sessions::SyncedSession*> sessions(
+      {session_a, session_b, session_c, session_d});
+
+  set_enable_tab_sync(true);
+  set_synced_sessions(&sessions);
+  NotifySubscription();
+  EXPECT_TRUE(fake_browser_tabs_metadata_fetcher()->DoesPendingCallbackExist());
+
+  // |session_b| should be the selected session because it is the has the same
+  // session_name as the set phone name and the latest modified time.
+  EXPECT_EQ(fake_browser_tabs_metadata_fetcher()->GetSession(), session_b);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chrome/browser/chromeos/phonehub/phone_hub_manager_factory.cc

@@ -7,9 +7,13 @@
 #include "ash/public/cpp/system_tray.h"
 #include "chrome/browser/chromeos/device_sync/device_sync_client_factory.h"
 #include "chrome/browser/chromeos/multidevice_setup/multidevice_setup_client_factory.h"
+#include "chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_impl.h"
+#include "chrome/browser/chromeos/phonehub/browser_tabs_model_provider_impl.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/secure_channel/secure_channel_client_provider.h"
+#include "chrome/browser/favicon/history_ui_favicon_request_handler_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sync/session_sync_service_factory.h"
 #include "chrome/browser/ui/webui/chromeos/multidevice_setup/multidevice_setup_dialog.h"
 #include "chromeos/components/phonehub/notification_access_manager_impl.h"
 #include "chromeos/components/phonehub/onboarding_ui_tracker_impl.h"
@@ -60,6 +64,8 @@ PhoneHubManagerFactory::PhoneHubManagerFactory()
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(device_sync::DeviceSyncClientFactory::GetInstance());
   DependsOn(multidevice_setup::MultiDeviceSetupClientFactory::GetInstance());
+  DependsOn(SessionSyncServiceFactory::GetInstance());
+  DependsOn(HistoryUiFaviconRequestHandlerFactory::GetInstance());
 }
 
 PhoneHubManagerFactory::~PhoneHubManagerFactory() = default;
@@ -83,6 +89,13 @@ KeyedService* PhoneHubManagerFactory::BuildServiceInstanceFor(
       device_sync::DeviceSyncClientFactory::GetForProfile(profile),
       multidevice_setup::MultiDeviceSetupClientFactory::GetForProfile(profile),
       secure_channel::SecureChannelClientProvider::GetInstance()->GetClient(),
+      std::make_unique<BrowserTabsModelProviderImpl>(
+          multidevice_setup::MultiDeviceSetupClientFactory::GetForProfile(
+              profile),
+          SessionSyncServiceFactory::GetInstance()->GetForProfile(profile),
+          std::make_unique<BrowserTabsMetadataFetcherImpl>(
+              HistoryUiFaviconRequestHandlerFactory::GetInstance()
+                  ->GetForBrowserContext(context))),
       base::BindRepeating(&multidevice_setup::MultiDeviceSetupDialog::Show));
 
   // Provide |phone_hub_manager| to the system tray so that it can be used by

chromeos/components/phonehub/BUILD.gn

@@ -11,6 +11,10 @@ static_library("phonehub") {
     "browser_tabs_metadata_fetcher.h",
     "browser_tabs_model.cc",
     "browser_tabs_model.h",
+    "browser_tabs_model_controller.cc",
+    "browser_tabs_model_controller.h",
+    "browser_tabs_model_provider.cc",
+    "browser_tabs_model_provider.h",
     "connection_manager.cc",
     "connection_manager.h",
     "connection_manager_impl.cc",

chromeos/components/phonehub/browser_tabs_model_controller.cc

+// Copyright 2020 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/phonehub/browser_tabs_model_controller.h"
+
+namespace chromeos {
+namespace phonehub {
+
+BrowserTabsModelController::BrowserTabsModelController(
+    BrowserTabsModelProvider* browser_tabs_model_provider,
+    MutablePhoneModel* mutable_phone_model)
+    : browser_tabs_model_provider_(browser_tabs_model_provider),
+      mutable_phone_model_(mutable_phone_model) {
+  browser_tabs_model_provider_->AddObserver(this);
+}
+
+BrowserTabsModelController::~BrowserTabsModelController() {
+  browser_tabs_model_provider_->RemoveObserver(this);
+}
+
+void BrowserTabsModelController::OnBrowserTabsUpdated(
+    bool is_sync_enabled,
+    const std::vector<BrowserTabsModel::BrowserTabMetadata>&
+        browser_tabs_metadata) {
+  mutable_phone_model_->SetBrowserTabsModel(BrowserTabsModel(
+      /*is_tab_sync_enabled=*/is_sync_enabled, browser_tabs_metadata));
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/browser_tabs_model_controller.h

+// Copyright 2020 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_PHONEHUB_BROWSER_TABS_MODEL_CONTROLLER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_BROWSER_TABS_MODEL_CONTROLLER_H_
+
+#include "chromeos/components/phonehub/browser_tabs_model.h"
+#include "chromeos/components/phonehub/browser_tabs_model_provider.h"
+#include "chromeos/components/phonehub/mutable_phone_model.h"
+
+namespace chromeos {
+namespace phonehub {
+
+// This class sets a MutablePhoneModel by observing info provided by the
+// BrowserTabsModelProvider.
+class BrowserTabsModelController : public BrowserTabsModelProvider::Observer {
+ public:
+  BrowserTabsModelController(
+      BrowserTabsModelProvider* browser_tabs_model_provider,
+      MutablePhoneModel* mutable_phone_model);
+  ~BrowserTabsModelController() override;
+
+ private:
+  // BrowserTabsModelProvider::Observer:
+  void OnBrowserTabsUpdated(
+      bool is_sync_enabled,
+      const std::vector<BrowserTabsModel::BrowserTabMetadata>&
+          browser_tabs_metadata) override;
+
+  BrowserTabsModelProvider* browser_tabs_model_provider_;
+  MutablePhoneModel* mutable_phone_model_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_BROWSER_TABS_MODEL_CONTROLLER_H_

chromeos/components/phonehub/browser_tabs_model_provider.cc

+// Copyright 2020 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/phonehub/browser_tabs_model_provider.h"
+
+namespace chromeos {
+namespace phonehub {
+
+BrowserTabsModelProvider::BrowserTabsModelProvider() = default;
+
+BrowserTabsModelProvider::~BrowserTabsModelProvider() = default;
+
+void BrowserTabsModelProvider::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void BrowserTabsModelProvider::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void BrowserTabsModelProvider::NotifyBrowserTabsUpdated(
+    bool is_sync_enabled,
+    const std::vector<BrowserTabsModel::BrowserTabMetadata>
+        browser_tabs_metadata) {
+  for (auto& observer : observer_list_)
+    observer.OnBrowserTabsUpdated(is_sync_enabled, browser_tabs_metadata);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/browser_tabs_model_provider.h

+// Copyright 2020 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_PHONEHUB_BROWSER_TABS_MODEL_PROVIDER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_BROWSER_TABS_MODEL_PROVIDER_H_
+
+#include <ostream>
+
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
+#include "chromeos/components/phonehub/browser_tabs_model.h"
+
+namespace chromeos {
+namespace phonehub {
+
+// Responsible for providing BrowserTabsModel information to observers.
+// Gets the browser tab model info by finding a SyncedSession (provided by
+// the SessionService) with a |session_name| that matches the |pii_free_name| of
+// the phone provided by a MultiDeviceSetupClient. If sync is enabled, the class
+// uses a BrowserTabsMetadataFetcher to actually fetch the browser tab metadata
+// once it finds the correct SyncedSession.
+class BrowserTabsModelProvider {
+ public:
+  class Observer : public base::CheckedObserver {
+   public:
+    ~Observer() override = default;
+
+    virtual void OnBrowserTabsUpdated(
+        bool is_sync_enabled,
+        const std::vector<BrowserTabsModel::BrowserTabMetadata>&
+            browser_tabs_metadata) = 0;
+  };
+
+  BrowserTabsModelProvider(const BrowserTabsModelProvider&) = delete;
+  BrowserTabsModelProvider* operator=(const BrowserTabsModelProvider&) = delete;
+  virtual ~BrowserTabsModelProvider();
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+ protected:
+  BrowserTabsModelProvider();
+
+  void NotifyBrowserTabsUpdated(
+      bool is_sync_enabled,
+      const std::vector<BrowserTabsModel::BrowserTabMetadata>
+          browser_tabs_metadata);
+
+ private:
+  base::ObserverList<Observer> observer_list_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_BROWSER_TABS_MODEL_PROVIDER_H_

chromeos/components/phonehub/fake_browser_tabs_metadata_fetcher.cc

@@ -14,6 +14,7 @@ FakeBrowserTabsMetadataFetcher::~FakeBrowserTabsMetadataFetcher() = default;
 void FakeBrowserTabsMetadataFetcher::Fetch(
     const sync_sessions::SyncedSession* session,
     base::OnceCallback<void(BrowserTabsMetadataResponse)> callback) {
+  session_ = session;
   callback_ = std::move(callback);
 }
 
@@ -22,5 +23,14 @@ void FakeBrowserTabsMetadataFetcher::RespondToCurrentFetchAttempt(
   std::move(callback_).Run(response);
 }
 
+bool FakeBrowserTabsMetadataFetcher::DoesPendingCallbackExist() {
+  return !callback_.is_null();
+}
+
+const sync_sessions::SyncedSession* FakeBrowserTabsMetadataFetcher::GetSession()
+    const {
+  return session_;
+}
+
 }  // namespace phonehub
 }  // namespace chromeos

chromeos/components/phonehub/fake_browser_tabs_metadata_fetcher.h

@@ -23,7 +23,12 @@ class FakeBrowserTabsMetadataFetcher : public BrowserTabsMetadataFetcher {
   void RespondToCurrentFetchAttempt(
       const BrowserTabsMetadataResponse& response);
 
+  bool DoesPendingCallbackExist();
+
+  const sync_sessions::SyncedSession* GetSession() const;
+
  private:
+  const sync_sessions::SyncedSession* session_;
   base::OnceCallback<void(BrowserTabsMetadataResponse)> callback_;
 };
 

chromeos/components/phonehub/phone_hub_manager_impl.cc

@@ -4,6 +4,9 @@
 
 #include "chromeos/components/phonehub/phone_hub_manager_impl.h"
 
+#include "chromeos/components/phonehub/browser_tabs_metadata_fetcher.h"
+#include "chromeos/components/phonehub/browser_tabs_model_controller.h"
+#include "chromeos/components/phonehub/browser_tabs_model_provider.h"
 #include "chromeos/components/phonehub/connection_manager_impl.h"
 #include "chromeos/components/phonehub/connection_scheduler_impl.h"
 #include "chromeos/components/phonehub/do_not_disturb_controller_impl.h"
@@ -27,6 +30,7 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
     device_sync::DeviceSyncClient* device_sync_client,
     multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
     chromeos::secure_channel::SecureChannelClient* secure_channel_client,
+    std::unique_ptr<BrowserTabsModelProvider> browser_tabs_model_provider,
     const base::RepeatingClosure& show_multidevice_setup_dialog_callback)
     : connection_manager_(
           std::make_unique<ConnectionManagerImpl>(multidevice_setup_client,
@@ -67,7 +71,12 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
           multidevice_setup_client,
           phone_model_.get())),
       tether_controller_(
-          std::make_unique<TetherControllerImpl>(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>(
+              browser_tabs_model_provider_.get(),
+              phone_model_.get())) {}
 
 PhoneHubManagerImpl::~PhoneHubManagerImpl() = default;
 
@@ -110,6 +119,8 @@ TetherController* PhoneHubManagerImpl::GetTetherController() {
 // These should be destroyed in the opposite order of how these objects are
 // initialized in the constructor.
 void PhoneHubManagerImpl::Shutdown() {
+  browser_tabs_model_controller_.reset();
+  browser_tabs_model_provider_.reset();
   tether_controller_.reset();
   phone_status_processor_.reset();
   phone_model_.reset();

chromeos/components/phonehub/phone_hub_manager_impl.h

@@ -29,6 +29,8 @@ class SecureChannelClient;
 
 namespace phonehub {
 
+class BrowserTabsModelController;
+class BrowserTabsModelProvider;
 class ConnectionManager;
 class MessageSender;
 class MessageReceiver;
@@ -43,7 +45,9 @@ class PhoneHubManagerImpl : public PhoneHubManager, public KeyedService {
       device_sync::DeviceSyncClient* device_sync_client,
       multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client,
       chromeos::secure_channel::SecureChannelClient* secure_channel_client,
+      std::unique_ptr<BrowserTabsModelProvider> browser_tabs_model_provider,
       const base::RepeatingClosure& show_multidevice_setup_dialog_callback);
+
   ~PhoneHubManagerImpl() override;
 
   // PhoneHubManager:
@@ -74,6 +78,8 @@ class PhoneHubManagerImpl : public PhoneHubManager, public KeyedService {
   std::unique_ptr<MutablePhoneModel> phone_model_;
   std::unique_ptr<PhoneStatusProcessor> phone_status_processor_;
   std::unique_ptr<TetherController> tether_controller_;
+  std::unique_ptr<BrowserTabsModelProvider> browser_tabs_model_provider_;
+  std::unique_ptr<BrowserTabsModelController> browser_tabs_model_controller_;
 };
 
 }  // namespace phonehub