[ContentIndex] Provide a quality score for offline items.

Use the site engagement service to rank content index entries when
handing off offline items.

Change-Id: Ie9929f73e0fb52edff297fd63f21e0069eb3c8f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866710
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707030}

chrome/browser/content_index/content_index_provider_factory.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/content_index/content_index_provider_factory.h"
 
 #include "chrome/browser/content_index/content_index_provider_impl.h"
+#include "chrome/browser/engagement/site_engagement_service_factory.h"
 #include "chrome/browser/metrics/ukm_background_recorder_service.h"
 #include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
@@ -29,6 +30,7 @@ ContentIndexProviderFactory::ContentIndexProviderFactory()
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(OfflineContentAggregatorFactory::GetInstance());
   DependsOn(ukm::UkmBackgroundRecorderFactory::GetInstance());
+  DependsOn(SiteEngagementServiceFactory::GetInstance());
 }
 
 ContentIndexProviderFactory::~ContentIndexProviderFactory() = default;

chrome/browser/content_index/content_index_provider_impl.cc

@@ -11,6 +11,9 @@
 #include "base/strings/string_split.h"
 #include "base/task/post_task.h"
 #include "build/build_config.h"
+#include "chrome/browser/engagement/site_engagement_score.h"
+#include "chrome/browser/engagement/site_engagement_service.h"
+#include "chrome/browser/engagement/site_engagement_service_factory.h"
 #include "chrome/browser/metrics/ukm_background_recorder_service.h"
 #include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -95,53 +98,15 @@ OfflineItemFilter CategoryToFilter(blink::mojom::ContentCategory category) {
   }
 }
 
-OfflineItem EntryToOfflineItem(const content::ContentIndexEntry& entry) {
-  OfflineItem item;
-  item.id = ContentId(kProviderNamespace, EntryKey(entry));
-  item.title = entry.description->title;
-  item.description = entry.description->description;
-  item.filter = CategoryToFilter(entry.description->category);
-  item.is_transient = false;
-  item.is_suggested = true;
-  item.creation_time = entry.registration_time;
-  item.is_openable = true;
-  item.state = offline_items_collection::OfflineItemState::COMPLETE;
-  item.is_resumable = false;
-  item.can_rename = false;
-  item.page_url = entry.launch_url;
-
-  return item;
-}
-
-void DidGetAllEntries(base::OnceClosure done_closure,
-                      ContentIndexProviderImpl::OfflineItemList* item_list,
-                      blink::mojom::ContentIndexError error,
-                      std::vector<content::ContentIndexEntry> entries) {
-  if (error != blink::mojom::ContentIndexError::NONE) {
-    std::move(done_closure).Run();
-    return;
-  }
-
-  for (const auto& entry : entries)
-    item_list->push_back(EntryToOfflineItem(entry));
-
-  std::move(done_closure).Run();
-}
-
-void DidGetAllEntriesAcrossStorageParitions(
-    std::unique_ptr<ContentIndexProviderImpl::OfflineItemList> item_list,
-    ContentIndexProviderImpl::MultipleItemCallback callback) {
-  ContentIndexMetrics::RecordContentIndexEntries(item_list->size());
-  std::move(callback).Run(*item_list);
-}
-
 }  // namespace
 
 ContentIndexProviderImpl::ContentIndexProviderImpl(Profile* profile)
     : profile_(profile),
       metrics_(ukm::UkmBackgroundRecorderFactory::GetForProfile(profile)),
-      aggregator_(OfflineContentAggregatorFactory::GetForKey(
-          profile->GetProfileKey())) {
+      aggregator_(
+          OfflineContentAggregatorFactory::GetForKey(profile->GetProfileKey())),
+      site_engagement_service_(
+          SiteEngagementServiceFactory::GetForProfile(profile)) {
   aggregator_->RegisterProvider(kProviderNamespace, this);
 }
 
@@ -153,6 +118,7 @@ ContentIndexProviderImpl::~ContentIndexProviderImpl() {
 void ContentIndexProviderImpl::Shutdown() {
   aggregator_->UnregisterProvider(kProviderNamespace);
   aggregator_ = nullptr;
+  site_engagement_service_ = nullptr;
 }
 
 std::vector<gfx::Size> ContentIndexProviderImpl::GetIconSizes(
@@ -287,15 +253,17 @@ void ContentIndexProviderImpl::GetItemById(const ContentId& id,
 
   storage_partition->GetContentIndexContext()->GetEntry(
       components.service_worker_registration_id, components.description_id,
-      base::BindOnce(
-          [](SingleItemCallback callback,
-             base::Optional<content::ContentIndexEntry> entry) {
-            if (!entry)
-              std::move(callback).Run(base::nullopt);
-            else
-              std::move(callback).Run(EntryToOfflineItem(*entry));
-          },
-          std::move(callback)));
+      base::BindOnce(&ContentIndexProviderImpl::DidGetItem,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+void ContentIndexProviderImpl::DidGetItem(
+    SingleItemCallback callback,
+    base::Optional<content::ContentIndexEntry> entry) {
+  if (!entry)
+    std::move(callback).Run(base::nullopt);
+  else
+    std::move(callback).Run(EntryToOfflineItem(*entry));
 }
 
 void ContentIndexProviderImpl::GetAllItems(MultipleItemCallback callback) {
@@ -317,8 +285,10 @@ void ContentIndexProviderImpl::GetAllItems(MultipleItemCallback callback) {
   // Get the all entries from every partition.
   auto barrier_closure = base::BarrierClosure(
       storage_paritions.size(),
-      base::BindOnce(&DidGetAllEntriesAcrossStorageParitions,
-                     std::move(item_list), std::move(callback)));
+      base::BindOnce(
+          &ContentIndexProviderImpl::DidGetAllEntriesAcrossStorageParitions,
+          weak_ptr_factory_.GetWeakPtr(), std::move(item_list),
+          std::move(callback)));
 
   for (auto* storage_partition : storage_paritions) {
     if (!storage_partition || !storage_partition->GetContentIndexContext()) {
@@ -328,11 +298,35 @@ void ContentIndexProviderImpl::GetAllItems(MultipleItemCallback callback) {
 
     // |item_list_ptr| is safe to use since it is owned by the barrier
     // closure.
-    storage_partition->GetContentIndexContext()->GetAllEntries(
-        base::BindOnce(&DidGetAllEntries, barrier_closure, item_list_ptr));
+    storage_partition->GetContentIndexContext()->GetAllEntries(base::BindOnce(
+        &ContentIndexProviderImpl::DidGetAllEntries,
+        weak_ptr_factory_.GetWeakPtr(), barrier_closure, item_list_ptr));
   }
 }
 
+void ContentIndexProviderImpl::DidGetAllEntriesAcrossStorageParitions(
+    std::unique_ptr<OfflineItemList> item_list,
+    MultipleItemCallback callback) {
+  ContentIndexMetrics::RecordContentIndexEntries(item_list->size());
+  std::move(callback).Run(*item_list);
+}
+
+void ContentIndexProviderImpl::DidGetAllEntries(
+    base::OnceClosure done_closure,
+    OfflineItemList* item_list,
+    blink::mojom::ContentIndexError error,
+    std::vector<content::ContentIndexEntry> entries) {
+  if (error != blink::mojom::ContentIndexError::NONE) {
+    std::move(done_closure).Run();
+    return;
+  }
+
+  for (const auto& entry : entries)
+    item_list->push_back(EntryToOfflineItem(entry));
+
+  std::move(done_closure).Run();
+}
+
 void ContentIndexProviderImpl::GetVisualsForItem(const ContentId& id,
                                                  GetVisualsOptions options,
                                                  VisualsCallback callback) {
@@ -353,6 +347,31 @@ void ContentIndexProviderImpl::GetVisualsForItem(const ContentId& id,
                      weak_ptr_factory_.GetWeakPtr(), id, std::move(callback)));
 }
 
+OfflineItem ContentIndexProviderImpl::EntryToOfflineItem(
+    const content::ContentIndexEntry& entry) {
+  OfflineItem item;
+  item.id = ContentId(kProviderNamespace, EntryKey(entry));
+  item.title = entry.description->title;
+  item.description = entry.description->description;
+  item.filter = CategoryToFilter(entry.description->category);
+  item.is_transient = false;
+  item.is_suggested = true;
+  item.creation_time = entry.registration_time;
+  item.is_openable = true;
+  item.state = offline_items_collection::OfflineItemState::COMPLETE;
+  item.is_resumable = false;
+  item.can_rename = false;
+  item.page_url = entry.launch_url;
+
+  if (site_engagement_service_) {
+    item.content_quality_score =
+        site_engagement_service_->GetScore(entry.launch_url.GetOrigin()) /
+        SiteEngagementScore::kMaxPoints;
+  }
+
+  return item;
+}
+
 void ContentIndexProviderImpl::DidGetIcons(const ContentId& id,
                                            VisualsCallback callback,
                                            std::vector<SkBitmap> icons) {

chrome/browser/content_index/content_index_provider_impl.h

@@ -26,6 +26,7 @@ class OfflineContentAggregator;
 }  // namespace offline_items_collection
 
 class Profile;
+class SiteEngagementService;
 
 class ContentIndexProviderImpl
     : public KeyedService,
@@ -73,16 +74,28 @@ class ContentIndexProviderImpl
   }
 
  private:
+  void DidGetItem(SingleItemCallback callback,
+                  base::Optional<content::ContentIndexEntry> entry);
+  void DidGetAllEntriesAcrossStorageParitions(
+      std::unique_ptr<OfflineItemList> item_list,
+      MultipleItemCallback callback);
+  void DidGetAllEntries(base::OnceClosure done_closure,
+                        OfflineItemList* item_list,
+                        blink::mojom::ContentIndexError error,
+                        std::vector<content::ContentIndexEntry> entries);
   void DidGetIcons(const offline_items_collection::ContentId& id,
                    VisualsCallback callback,
                    std::vector<SkBitmap> icons);
   void DidGetEntryToOpen(base::Optional<content::ContentIndexEntry> entry);
   void DidOpenTab(content::ContentIndexEntry entry,
                   content::WebContents* web_contents);
+  offline_items_collection::OfflineItem EntryToOfflineItem(
+      const content::ContentIndexEntry& entry);
 
   Profile* profile_;
   ContentIndexMetrics metrics_;
   offline_items_collection::OfflineContentAggregator* aggregator_;
+  SiteEngagementService* site_engagement_service_;
   base::ObserverList<Observer>::Unchecked observers_;
   base::Optional<std::vector<gfx::Size>> icon_sizes_for_testing_;
   base::WeakPtrFactory<ContentIndexProviderImpl> weak_ptr_factory_{this};

chrome/browser/content_index/content_index_provider_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "base/time/time.h"
+#include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/offline_items_collection/core/offline_content_provider.h"
 #include "content/public/browser/content_index_provider.h"
@@ -30,6 +31,7 @@ using offline_items_collection::UpdateDelta;
 using testing::_;
 
 constexpr int64_t kServiceWorkerRegistrationId = 42;
+constexpr double kEngagementScore = 42.0;
 const GURL kLaunchURL = GURL("https://example.com/foo");
 const url::Origin kOrigin = url::Origin::Create(kLaunchURL.GetOrigin());
 
@@ -39,6 +41,9 @@ class ContentIndexProviderImplTest : public testing::Test,
   void SetUp() override {
     ASSERT_TRUE(profile_.CreateHistoryService(/* delete_file= */ true,
                                               /* no_db= */ false));
+
+    auto* service = SiteEngagementService::Get(&profile_);
+    service->ResetBaseScoreForURL(kOrigin.GetURL(), kEngagementScore);
     provider_ = std::make_unique<ContentIndexProviderImpl>(&profile_);
     provider_->AddObserver(this);
   }
@@ -86,6 +91,7 @@ TEST_F(ContentIndexProviderImplTest, OfflineItemCreation) {
   EXPECT_TRUE(item.is_suggested);
   EXPECT_TRUE(item.is_openable);
   EXPECT_EQ(item.page_url, kLaunchURL);
+  EXPECT_EQ(item.content_quality_score, kEngagementScore / 100.0);
 }
 
 TEST_F(ContentIndexProviderImplTest, ObserverUpdates) {