[CrOS PhoneHub] Get Browser tab metadata including favicons.

Creates a helper class that gets the metadata needed to return
a vector of the latest visited BrowserTabMetadata to the caller.

Creates a fake version that can be used for unit tests of
callers to this class.

Note the class requires a FaviconService to be passed in the ctor.

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

chrome/browser/chromeos/BUILD.gn

@@ -234,6 +234,7 @@ source_set("chromeos") {
     "//components/drive:drive_chromeos",
     "//components/enterprise",
     "//components/exo",
+    "//components/favicon/core",
     "//components/feedback",
     "//components/flags_ui",
     "//components/gcm_driver",
@@ -283,6 +284,7 @@ source_set("chromeos") {
     "//components/sync",
     "//components/sync_device_info",
     "//components/sync_preferences",
+    "//components/sync_sessions",
     "//components/tracing:startup_tracing",
     "//components/translate/core/browser",
     "//components/ukm/content",
@@ -1936,6 +1938,8 @@ source_set("chromeos") {
     "ownership/owner_settings_service_chromeos.h",
     "ownership/owner_settings_service_chromeos_factory.cc",
     "ownership/owner_settings_service_chromeos_factory.h",
+    "phonehub/browser_tabs_metadata_fetcher_impl.cc",
+    "phonehub/browser_tabs_metadata_fetcher_impl.h",
     "phonehub/phone_hub_manager_factory.cc",
     "phonehub/phone_hub_manager_factory.h",
     "platform_keys/extension_platform_keys_service.cc",
@@ -3432,6 +3436,7 @@ source_set("unit_tests") {
     "night_light/night_light_client_unittest.cc",
     "note_taking_helper_unittest.cc",
     "ownership/owner_settings_service_chromeos_unittest.cc",
+    "phonehub/browser_tabs_metadata_fetcher_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",
@@ -3774,6 +3779,7 @@ source_set("unit_tests") {
     "//components/drive",
     "//components/drive:test_support",
     "//components/exo",
+    "//components/favicon/core/test:test_support",
     "//components/invalidation/impl:test_support",
     "//components/invalidation/public",
     "//components/keyed_service/content",
@@ -3788,9 +3794,11 @@ source_set("unit_tests") {
     "//components/resources",
     "//components/services/app_service/public/cpp:test_support",
     "//components/session_manager/core",
+    "//components/sessions:test_support",
     "//components/signin/public/identity_manager:test_support",
     "//components/sync",
     "//components/sync_preferences",
+    "//components/sync_sessions",
     "//components/user_prefs",
     "//components/variations:test_support",
     "//content/test:test_support",

chrome/browser/chromeos/phonehub/DEPS

+include_rules = [
+  "+components/favicon",
+  "+components/sessions/core",
+  "+components/sync_sessions",
+  "+third_party/skia/include/core",
+]

chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_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_metadata_fetcher_impl.h"
+
+#include "base/barrier_closure.h"
+#include "components/favicon/core/favicon_service.h"
+#include "components/sync_sessions/synced_session.h"
+
+namespace chromeos {
+namespace phonehub {
+namespace {
+
+std::vector<BrowserTabsModel::BrowserTabMetadata>
+GetSortedMetadataWithoutFavicons(const sync_sessions::SyncedSession* session) {
+  std::vector<BrowserTabsModel::BrowserTabMetadata> browser_tab_metadata;
+
+  using WindowPair =
+      std::pair<const SessionID,
+                std::unique_ptr<sync_sessions::SyncedSessionWindow>>;
+  for (const WindowPair& window_pair : session->windows) {
+    const sessions::SessionWindow& window = window_pair.second->wrapped_window;
+    for (const std::unique_ptr<sessions::SessionTab>& tab : window.tabs) {
+      int selected_index = tab->normalized_navigation_index();
+      const sessions::SerializedNavigationEntry& current_navigation =
+          tab->navigations.at(selected_index);
+
+      GURL tab_url = current_navigation.virtual_url();
+
+      // If the url is incorrectly formatted, or is empty, do not proceed with
+      // storing its metadata.
+      if (!tab_url.is_valid())
+        continue;
+
+      const base::string16& title = current_navigation.title();
+      const base::Time last_accessed_timestamp = tab->timestamp;
+      browser_tab_metadata.emplace_back(tab_url, title, last_accessed_timestamp,
+                                        gfx::Image());
+    }
+  }
+
+  // Sorts the |browser_tab_metadata| from most recently visited to least
+  // recently visited.
+  std::sort(browser_tab_metadata.begin(), browser_tab_metadata.end());
+
+  // At most |kMaxMostRecentTabs| tab metadata can be displayed.
+  size_t num_tabs_to_display = std::min(browser_tab_metadata.size(),
+                                        BrowserTabsModel::kMaxMostRecentTabs);
+  return std::vector<BrowserTabsModel::BrowserTabMetadata>(
+      browser_tab_metadata.begin(),
+      browser_tab_metadata.begin() + num_tabs_to_display);
+}
+
+}  // namespace
+
+BrowserTabsMetadataFetcherImpl::BrowserTabsMetadataFetcherImpl(
+    favicon::FaviconService* favicon_service)
+    : favicon_service_(favicon_service) {}
+
+BrowserTabsMetadataFetcherImpl::~BrowserTabsMetadataFetcherImpl() = default;
+
+void BrowserTabsMetadataFetcherImpl::Fetch(
+    const sync_sessions::SyncedSession* session,
+    base::OnceCallback<void(BrowserTabsMetadataResponse)> callback) {
+  // 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);
+  }
+
+  results_ = GetSortedMetadataWithoutFavicons(session);
+  callback_ = std::move(callback);
+
+  // When |barrier| is run |num_tabs_to_display| times, it will run
+  // |OnAllFaviconsFetched|.
+  base::RepeatingClosure barrier = base::BarrierClosure(
+      results_.size(),
+      base::BindOnce(&BrowserTabsMetadataFetcherImpl::OnAllFaviconsFetched,
+                     weak_ptr_factory_.GetWeakPtr()));
+
+  for (size_t i = 0; i < results_.size(); ++i) {
+    favicon_service_->GetFaviconImageForPageURL(
+        results_[i].url,
+        base::BindOnce(&BrowserTabsMetadataFetcherImpl::OnFaviconReady,
+                       weak_ptr_factory_.GetWeakPtr(), i, barrier),
+        &favicon_tracker_);
+  }
+}
+
+void BrowserTabsMetadataFetcherImpl::OnAllFaviconsFetched() {
+  std::move(callback_).Run(std::move(results_));
+}
+
+void BrowserTabsMetadataFetcherImpl::OnFaviconReady(
+    size_t index_in_results,
+    base::OnceClosure done_closure,
+    const favicon_base::FaviconImageResult& favicon_image_result) {
+  DCHECK(index_in_results < results_.size());
+
+  results_[index_in_results].favicon = std::move(favicon_image_result.image);
+  std::move(done_closure).Run();
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_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_METADATA_FETCHER_IMPL_H_
+#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 {
+struct FaviconImageResult;
+}  // namespace favicon_base
+
+namespace favicon {
+class FaviconService;
+}  // namespace favicon
+
+namespace chromeos {
+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
+// 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);
+  ~BrowserTabsMetadataFetcherImpl() override;
+
+  // BrowserTabsMetadataFetcher:
+  void Fetch(
+      const sync_sessions::SyncedSession* session,
+      base::OnceCallback<void(BrowserTabsMetadataResponse)> callback) override;
+
+ private:
+  void OnAllFaviconsFetched();
+  void OnFaviconReady(
+      size_t index_in_results,
+      base::OnceClosure done_closure,
+      const favicon_base::FaviconImageResult& favicon_image_result);
+
+  favicon::FaviconService* const favicon_service_;
+  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};
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_PHONEHUB_BROWSER_TABS_METADATA_FETCHER_IMPL_H_

chrome/browser/chromeos/phonehub/browser_tabs_metadata_fetcher_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_metadata_fetcher_impl.h"
+
+#include "base/strings/utf_string_conversions.h"
+#include "components/favicon/core/test/mock_favicon_service.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"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/favicon_size.h"
+#include "ui/gfx/image/image_unittest_util.h"
+
+namespace chromeos {
+namespace phonehub {
+namespace {
+
+using testing::_;
+
+const base::Time kTimeA = base::Time::FromDoubleT(1);
+const base::Time kTimeB = base::Time::FromDoubleT(2);
+const base::Time kTimeC = base::Time::FromDoubleT(3);
+const base::Time kTimeD = base::Time::FromDoubleT(4);
+const base::Time kTimeE = base::Time::FromDoubleT(5);
+
+gfx::Image GetDummyImage() {
+  SkBitmap bitmap;
+  bitmap.allocN32Pixels(gfx::kFaviconSize, gfx::kFaviconSize);
+  bitmap.eraseColor(SK_ColorBLUE);
+  return gfx::Image::CreateFrom1xBitmap(bitmap);
+}
+
+favicon_base::FaviconImageResult GetDummyFaviconResult() {
+  favicon_base::FaviconImageResult result;
+  result.icon_url = GURL("http://example.com/favicon.ico");
+  result.image = GetDummyImage();
+  return result;
+}
+}  // namespace
+
+class BrowserTabsMetadataFetcherImplTest : public testing::Test {
+ public:
+  BrowserTabsMetadataFetcherImplTest()
+      : browser_tabs_metadata_job_(&favicon_service_),
+        synced_session_(std::make_unique<sync_sessions::SyncedSession>()) {}
+
+  BrowserTabsMetadataFetcherImplTest(
+      const BrowserTabsMetadataFetcherImplTest&) = delete;
+  BrowserTabsMetadataFetcherImplTest& operator=(
+      const BrowserTabsMetadataFetcherImplTest&) = delete;
+  ~BrowserTabsMetadataFetcherImplTest() override = default;
+
+  using BrowserTabMetadata = BrowserTabsModel::BrowserTabMetadata;
+
+  void OnBrowserTabMetadataFetched(
+      base::Optional<std::vector<BrowserTabsModel::BrowserTabMetadata>>
+          browser_tab_metadatas) {
+    actual_browser_tabs_metadata_ = browser_tab_metadatas;
+  }
+
+  void AddTab(sync_sessions::SyncedSessionWindow* synced_session_window,
+              const base::string16& title,
+              const GURL& url,
+              const base::Time& time) {
+    auto tab1 = std::make_unique<sessions::SessionTab>();
+
+    tab1->current_navigation_index = 0;
+    auto navigation = sessions::SerializedNavigationEntryTestHelper::
+        CreateNavigationForTest();
+
+    navigation.set_title(title);
+    navigation.set_virtual_url(url);
+    navigation.set_timestamp(time);
+
+    tab1->timestamp = time;
+    tab1->navigations.push_back(navigation);
+    tab1->navigations.back().set_encoded_page_state("");
+
+    synced_session_window->wrapped_window.tabs.push_back(std::move(tab1));
+  }
+
+  void AddWindow(std::unique_ptr<sync_sessions::SyncedSessionWindow>
+                     synced_session_window) {
+    SessionID session_id = SessionID::NewUnique();
+    synced_session_->windows.insert(
+        std::pair<SessionID,
+                  std::unique_ptr<sync_sessions::SyncedSessionWindow>>{
+            session_id, std::move(synced_session_window)});
+  }
+
+  void ExpectFaviconUrlFetchAttempt(const GURL& url) {
+    EXPECT_CALL(favicon_service_, GetFaviconImageForPageURL(url, /*callback=*/_,
+                                                            /*tracker=*/_))
+        .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));
+              else
+                favicon_service_responses_.emplace_back(std::move(callback));
+              return base::CancelableTaskTracker::kBadTaskId;
+            });
+  }
+
+  void AttemptFetch() {
+    browser_tabs_metadata_job_.Fetch(
+        synced_session_.get(),
+        base::BindOnce(
+            &BrowserTabsMetadataFetcherImplTest::OnBrowserTabMetadataFetched,
+            base::Unretained(this)));
+  }
+
+  void InvokeNextFaviconCallbacks(size_t num_successful_fetches) {
+    for (size_t i = 0; i < num_successful_fetches; i++) {
+      std::move(favicon_service_responses_.front())
+          .Run(GetDummyFaviconResult());
+      favicon_service_responses_.pop_front();
+    }
+  }
+
+  void CheckIsExpectedMetadata(
+      const std::vector<BrowserTabMetadata> browser_tabs_metadata) {
+    EXPECT_EQ(browser_tabs_metadata, actual_browser_tabs_metadata_);
+
+    // Check favicons as they are not includes in equality.
+    for (size_t i = 0; i < browser_tabs_metadata.size(); i++) {
+      EXPECT_TRUE(gfx::test::AreImagesEqual(
+          browser_tabs_metadata[i].favicon,
+          (*actual_browser_tabs_metadata_)[i].favicon));
+    }
+  }
+
+  const base::Optional<std::vector<BrowserTabsModel::BrowserTabMetadata>>&
+  actual_browser_tabs_metadata() const {
+    return actual_browser_tabs_metadata_;
+  }
+
+ private:
+  testing::NiceMock<favicon::MockFaviconService> favicon_service_;
+  BrowserTabsMetadataFetcherImpl browser_tabs_metadata_job_;
+  base::Optional<std::vector<BrowserTabsModel::BrowserTabMetadata>>
+      actual_browser_tabs_metadata_;
+
+  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_;
+};
+
+TEST_F(BrowserTabsMetadataFetcherImplTest, NewFetchDuringOldFetchInProgress) {
+  const base::string16 kTitleA = base::UTF8ToUTF16("A");
+  const GURL kUrlA = GURL("http://a.com");
+
+  const base::string16 kTitleB = base::UTF8ToUTF16("B");
+  const GURL kUrlB = GURL("http://b.com");
+
+  const base::string16 kTitleC = base::UTF8ToUTF16("C");
+  const GURL kUrlC = GURL("http://c.com");
+
+  const base::string16 kTitleD = base::UTF8ToUTF16("D");
+  const GURL kUrlD = GURL("http://d.com");
+
+  auto synced_session_window =
+      std::make_unique<sync_sessions::SyncedSessionWindow>();
+  AddTab(synced_session_window.get(), kTitleB, kUrlB, kTimeB);
+  AddTab(synced_session_window.get(), kTitleA, kUrlA, kTimeA);
+  AddWindow(std::move(synced_session_window));
+
+  ExpectFaviconUrlFetchAttempt(kUrlB);
+  ExpectFaviconUrlFetchAttempt(kUrlA);
+
+  AttemptFetch();
+  InvokeNextFaviconCallbacks(/*num_successful_fetches=*/1);
+
+  auto synced_session_window_two =
+      std::make_unique<sync_sessions::SyncedSessionWindow>();
+  AddTab(synced_session_window_two.get(), kTitleD, kUrlD, kTimeD);
+  AddTab(synced_session_window_two.get(), kTitleC, kUrlC, kTimeC);
+  AddWindow(std::move(synced_session_window_two));
+
+  ExpectFaviconUrlFetchAttempt(kUrlD);
+  ExpectFaviconUrlFetchAttempt(kUrlC);
+  ExpectFaviconUrlFetchAttempt(kUrlB);
+  ExpectFaviconUrlFetchAttempt(kUrlA);
+
+  AttemptFetch();
+  EXPECT_FALSE(actual_browser_tabs_metadata());
+
+  // 5 callbacks called accounting for the additional missed one for tab A.
+  InvokeNextFaviconCallbacks(/*num_successful_fetches=*/5);
+  CheckIsExpectedMetadata(std::vector<BrowserTabMetadata>({
+      BrowserTabMetadata(kUrlD, kTitleD, kTimeD, GetDummyImage()),
+      BrowserTabMetadata(kUrlC, kTitleC, kTimeC, GetDummyImage()),
+      BrowserTabMetadata(kUrlB, kTitleB, kTimeB, GetDummyImage()),
+      BrowserTabMetadata(kUrlA, kTitleA, kTimeA, GetDummyImage()),
+  }));
+}
+
+TEST_F(BrowserTabsMetadataFetcherImplTest, NoTabsOpen) {
+  auto synced_session_window =
+      std::make_unique<sync_sessions::SyncedSessionWindow>();
+  AddWindow(std::move(synced_session_window));
+
+  AttemptFetch();
+  CheckIsExpectedMetadata({});
+}
+
+TEST_F(BrowserTabsMetadataFetcherImplTest, BelowMaximumNumberOfTabs) {
+  const base::string16 kTitleC = base::UTF8ToUTF16("C");
+  const GURL kUrlC = GURL("http://c.com");
+
+  const base::string16 kTitleD = base::UTF8ToUTF16("D");
+  const GURL kUrlD = GURL("http://d.com");
+
+  auto synced_session_window =
+      std::make_unique<sync_sessions::SyncedSessionWindow>();
+  AddTab(synced_session_window.get(), kTitleD, kUrlD, kTimeD);
+  AddTab(synced_session_window.get(), kTitleC, kUrlC, kTimeC);
+  AddWindow(std::move(synced_session_window));
+
+  ExpectFaviconUrlFetchAttempt(kUrlC);
+  ExpectFaviconUrlFetchAttempt(kUrlD);
+
+  AttemptFetch();
+  InvokeNextFaviconCallbacks(/*num_successful_fetches=*/2);
+  CheckIsExpectedMetadata(std::vector<BrowserTabMetadata>({
+      BrowserTabMetadata(kUrlD, kTitleD, kTimeD, GetDummyImage()),
+      BrowserTabMetadata(kUrlC, kTitleC, kTimeC, GetDummyImage()),
+  }));
+}
+
+TEST_F(BrowserTabsMetadataFetcherImplTest, ExceedMaximumNumberOfTabs) {
+  const base::string16 kTitleA = base::UTF8ToUTF16("A");
+  const GURL kUrlA = GURL("http://a.com");
+
+  const base::string16 kTitleB = base::UTF8ToUTF16("B");
+  const GURL kUrlB = GURL("http://b.com");
+
+  const base::string16 kTitleC = base::UTF8ToUTF16("C");
+  const GURL kUrlC = GURL("http://c.com");
+
+  const base::string16 kTitleD = base::UTF8ToUTF16("D");
+  const GURL kUrlD = GURL("http://d.com");
+
+  const base::string16 kTitleE = base::UTF8ToUTF16("E");
+  const GURL kUrlE = GURL("http://e.com");
+
+  auto synced_session_window =
+      std::make_unique<sync_sessions::SyncedSessionWindow>();
+  AddTab(synced_session_window.get(), kTitleA, kUrlA, kTimeA);
+  AddTab(synced_session_window.get(), kTitleE, kUrlE, kTimeE);
+  AddTab(synced_session_window.get(), kTitleB, kUrlB, kTimeB);
+  AddTab(synced_session_window.get(), kTitleD, kUrlD, kTimeD);
+  AddTab(synced_session_window.get(), kTitleC, kUrlC, kTimeC);
+  AddWindow(std::move(synced_session_window));
+
+  ExpectFaviconUrlFetchAttempt(kUrlB);
+  ExpectFaviconUrlFetchAttempt(kUrlC);
+  ExpectFaviconUrlFetchAttempt(kUrlD);
+  ExpectFaviconUrlFetchAttempt(kUrlE);
+
+  AttemptFetch();
+  InvokeNextFaviconCallbacks(/*num_successful_fetches=*/4);
+
+  // Tab A is not present because it has the oldest timestamp, and the maximum
+  // number of BrowserTabMetadata has been met.
+  CheckIsExpectedMetadata(std::vector<BrowserTabMetadata>({
+      BrowserTabMetadata(kUrlE, kTitleE, kTimeE, GetDummyImage()),
+      BrowserTabMetadata(kUrlD, kTitleD, kTimeD, GetDummyImage()),
+      BrowserTabMetadata(kUrlC, kTitleC, kTimeC, GetDummyImage()),
+      BrowserTabMetadata(kUrlB, kTitleB, kTimeB, GetDummyImage()),
+  }));
+}
+
+TEST_F(BrowserTabsMetadataFetcherImplTest, MultipleWindows) {
+  const base::string16 kTitleB = base::UTF8ToUTF16("B");
+  const GURL kUrlB = GURL("http://b.com");
+
+  const base::string16 kTitleC = base::UTF8ToUTF16("C");
+  const GURL kUrlC = GURL("http://c.com");
+
+  const base::string16 kTitleD = base::UTF8ToUTF16("D");
+  const GURL kUrlD = GURL("http://d.com");
+
+  const base::string16 kTitleE = base::UTF8ToUTF16("E");
+  const GURL kUrlE = GURL("http://e.com");
+
+  auto synced_session_window_one =
+      std::make_unique<sync_sessions::SyncedSessionWindow>();
+  AddTab(synced_session_window_one.get(), kTitleE, kUrlE, kTimeE);
+  AddTab(synced_session_window_one.get(), kTitleB, kUrlB, kTimeB);
+  AddWindow(std::move(synced_session_window_one));
+
+  auto synced_session_window_two =
+      std::make_unique<sync_sessions::SyncedSessionWindow>();
+  AddTab(synced_session_window_two.get(), kTitleD, kUrlD, kTimeD);
+  AddTab(synced_session_window_two.get(), kTitleC, kUrlC, kTimeC);
+  AddWindow(std::move(synced_session_window_two));
+
+  ExpectFaviconUrlFetchAttempt(kUrlB);
+  ExpectFaviconUrlFetchAttempt(kUrlC);
+  ExpectFaviconUrlFetchAttempt(kUrlD);
+  ExpectFaviconUrlFetchAttempt(kUrlE);
+
+  AttemptFetch();
+  InvokeNextFaviconCallbacks(/*num_successful_fetches=*/4);
+  CheckIsExpectedMetadata(std::vector<BrowserTabMetadata>({
+      BrowserTabMetadata(kUrlE, kTitleE, kTimeE, GetDummyImage()),
+      BrowserTabMetadata(kUrlD, kTitleD, kTimeD, GetDummyImage()),
+      BrowserTabMetadata(kUrlC, kTitleC, kTimeC, GetDummyImage()),
+      BrowserTabMetadata(kUrlB, kTitleB, kTimeB, GetDummyImage()),
+  }));
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/BUILD.gn

@@ -8,6 +8,7 @@ assert(is_chromeos, "Phone Hub is Chrome OS only")
 
 static_library("phonehub") {
   sources = [
+    "browser_tabs_metadata_fetcher.h",
     "browser_tabs_model.cc",
     "browser_tabs_model.h",
     "connection_manager.cc",
@@ -121,6 +122,8 @@ static_library("test_support") {
   testonly = true
 
   sources = [
+    "fake_browser_tabs_metadata_fetcher.cc",
+    "fake_browser_tabs_metadata_fetcher.h",
     "fake_connection_manager.cc",
     "fake_connection_manager.h",
     "fake_message_receiver.h",

chromeos/components/phonehub/browser_tabs_metadata_fetcher.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_METADATA_FETCHER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_BROWSER_TABS_METADATA_FETCHER_H_
+
+#include <vector>
+#include "base/callback.h"
+#include "chromeos/components/phonehub/browser_tabs_model.h"
+
+namespace sync_sessions {
+struct SyncedSession;
+}  // namespace sync_sessions
+
+namespace chromeos {
+namespace phonehub {
+
+// Used to collect the most recently visited tab metadata from a
+// sync_sessions::SyncedSession, fetch their respective favicon images, and
+// return a list of BrowserTabMetadata to its caller.
+class BrowserTabsMetadataFetcher {
+ public:
+  virtual ~BrowserTabsMetadataFetcher() = default;
+
+  BrowserTabsMetadataFetcher(const BrowserTabsMetadataFetcher&) = delete;
+  BrowserTabsMetadataFetcher& operator=(const BrowserTabsMetadataFetcher&) =
+      delete;
+
+  using BrowserTabsMetadataResponse =
+      base::Optional<std::vector<BrowserTabsModel::BrowserTabMetadata>>;
+
+  // Fetches the metadata of the most recently visited tabs. Only one fetch is
+  // possible at a given time, and if a new fetch is started when another is
+  // already in progress, the previous fetch will be passed a base::nullopt.
+  virtual void Fetch(
+      const sync_sessions::SyncedSession* session,
+      base::OnceCallback<void(BrowserTabsMetadataResponse)> callback) = 0;
+
+ protected:
+  BrowserTabsMetadataFetcher() = default;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_BROWSER_TABS_METADATA_FETCHER_H_

chromeos/components/phonehub/browser_tabs_model.cc

@@ -26,9 +26,10 @@ BrowserTabsModel::BrowserTabMetadata::BrowserTabMetadata(
 
 bool BrowserTabsModel::BrowserTabMetadata::operator==(
     const BrowserTabMetadata& other) const {
+  // The favicon is not compared because equality of gfx::Image is defined
+  // by the same storage space rather than the image itself.
   return url == other.url && title == other.title &&
-         last_accessed_timestamp == other.last_accessed_timestamp &&
-         favicon == other.favicon;
+         last_accessed_timestamp == other.last_accessed_timestamp;
 }
 
 bool BrowserTabsModel::BrowserTabMetadata::operator!=(

chromeos/components/phonehub/fake_browser_tabs_metadata_fetcher.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/fake_browser_tabs_metadata_fetcher.h"
+
+namespace chromeos {
+namespace phonehub {
+
+FakeBrowserTabsMetadataFetcher::FakeBrowserTabsMetadataFetcher() = default;
+
+FakeBrowserTabsMetadataFetcher::~FakeBrowserTabsMetadataFetcher() = default;
+
+void FakeBrowserTabsMetadataFetcher::Fetch(
+    const sync_sessions::SyncedSession* session,
+    base::OnceCallback<void(BrowserTabsMetadataResponse)> callback) {
+  callback_ = std::move(callback);
+}
+
+void FakeBrowserTabsMetadataFetcher::RespondToCurrentFetchAttempt(
+    const BrowserTabsMetadataResponse& response) {
+  std::move(callback_).Run(response);
+}
+
+}  // namespace phonehub
+}  // namespace chromeos

chromeos/components/phonehub/fake_browser_tabs_metadata_fetcher.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_FAKE_BROWSER_TABS_METADATA_FETCHER_H_
+#define CHROMEOS_COMPONENTS_PHONEHUB_FAKE_BROWSER_TABS_METADATA_FETCHER_H_
+
+#include "chromeos/components/phonehub/browser_tabs_metadata_fetcher.h"
+
+namespace chromeos {
+namespace phonehub {
+
+class FakeBrowserTabsMetadataFetcher : public BrowserTabsMetadataFetcher {
+ public:
+  FakeBrowserTabsMetadataFetcher();
+  ~FakeBrowserTabsMetadataFetcher() override;
+
+  // BrowserTabsMetadataFetcher:
+  void Fetch(
+      const sync_sessions::SyncedSession* session,
+      base::OnceCallback<void(BrowserTabsMetadataResponse)> callback) override;
+
+  void RespondToCurrentFetchAttempt(
+      const BrowserTabsMetadataResponse& response);
+
+ private:
+  base::OnceCallback<void(BrowserTabsMetadataResponse)> callback_;
+};
+
+}  // namespace phonehub
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_PHONEHUB_FAKE_BROWSER_TABS_METADATA_FETCHER_H_