Persist whether a visit has a url keyed image into HistoryService

Bug: b/265301309
Change-Id: Ic8bcfdade453beb127a4dcc8c2e9d5d2728489b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4241663
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1104183}

chrome/browser/optimization_guide/page_content_annotations_service_browsertest.cc

@@ -820,6 +820,104 @@ IN_PROC_BROWSER_TEST_F(PageContentAnnotationsServiceRemoteMetadataBrowserTest,
       0);
 }
 
+class PageContentAnnotationsServiceSalientImageMetadataBrowserTest
+    : public PageContentAnnotationsServiceBrowserTest {
+ public:
+  PageContentAnnotationsServiceSalientImageMetadataBrowserTest() {
+    scoped_feature_list_.InitWithFeaturesAndParameters(
+        {{features::kOptimizationHints, {}},
+         {features::kPageContentAnnotations, {}},
+         {features::kPageContentAnnotationsPersistSalientImageMetadata, {}}},
+        /*disabled_features=*/{});
+    set_load_model_on_startup(false);
+  }
+  ~PageContentAnnotationsServiceSalientImageMetadataBrowserTest() override =
+      default;
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(
+    PageContentAnnotationsServiceSalientImageMetadataBrowserTest,
+    EmptyMetadataNotStored) {
+  base::HistogramTester histogram_tester;
+
+  GURL url(embedded_test_server()->GetURL("a.com", "/hello.html"));
+
+  proto::SalientImageMetadata salient_image_metadata;
+  OptimizationMetadata metadata;
+  metadata.SetAnyMetadataForTesting(salient_image_metadata);
+  OptimizationGuideKeyedServiceFactory::GetForProfile(browser()->profile())
+      ->AddHintForTesting(url, proto::SALIENT_IMAGE, metadata);
+
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.PageContentAnnotationsService."
+      "ContentAnnotationsStorageStatus.SalientImageMetadata",
+      0);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    PageContentAnnotationsServiceSalientImageMetadataBrowserTest,
+    MetadataWithNoNonEmptyUrlNotStored) {
+  base::HistogramTester histogram_tester;
+
+  GURL url(embedded_test_server()->GetURL("a.com", "/hello.html"));
+
+  proto::SalientImageMetadata salient_image_metadata;
+  salient_image_metadata.add_thumbnails();
+  salient_image_metadata.add_thumbnails();
+  OptimizationMetadata metadata;
+  metadata.SetAnyMetadataForTesting(salient_image_metadata);
+  OptimizationGuideKeyedServiceFactory::GetForProfile(browser()->profile())
+      ->AddHintForTesting(url, proto::SALIENT_IMAGE, metadata);
+
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.PageContentAnnotationsService."
+      "ContentAnnotationsStorageStatus.SalientImageMetadata",
+      0);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    PageContentAnnotationsServiceSalientImageMetadataBrowserTest,
+    MetadataWithNonEmptyUrlStored) {
+  base::HistogramTester histogram_tester;
+
+  GURL url(embedded_test_server()->GetURL("a.com", "/hello.html"));
+
+  proto::SalientImageMetadata salient_image_metadata;
+  salient_image_metadata.add_thumbnails();
+  salient_image_metadata.add_thumbnails()->set_image_url(
+      "http://gstatic.com/image");
+  OptimizationMetadata metadata;
+  metadata.SetAnyMetadataForTesting(salient_image_metadata);
+  OptimizationGuideKeyedServiceFactory::GetForProfile(browser()->profile())
+      ->AddHintForTesting(url, proto::SALIENT_IMAGE, metadata);
+
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+  RetryForHistogramUntilCountReached(
+      &histogram_tester,
+      "OptimizationGuide.PageContentAnnotationsService."
+      "ContentAnnotationsStorageStatus.SalientImageMetadata",
+      1);
+
+  histogram_tester.ExpectUniqueSample(
+      "OptimizationGuide.PageContentAnnotationsService."
+      "ContentAnnotationsStorageStatus.SalientImageMetadata",
+      PageContentAnnotationsStorageStatus::kSuccess, 1);
+
+  absl::optional<history::VisitContentAnnotations> got_content_annotations =
+      GetContentAnnotationsForURL(url);
+  ASSERT_TRUE(got_content_annotations.has_value());
+  EXPECT_TRUE(got_content_annotations->has_url_keyed_image);
+}
+
 class PageContentAnnotationsServiceNoHistoryTest
     : public PageContentAnnotationsServiceBrowserTest {
  public:

components/history/core/browser/history_backend.cc

@@ -738,6 +738,29 @@ void HistoryBackend::AddPageMetadataForVisit(
   }
 }
 
+void HistoryBackend::SetHasUrlKeyedImageForVisit(VisitID visit_id,
+                                                 bool has_url_keyed_image) {
+  TRACE_EVENT0("browser", "HistoryBackend::SetHasUrlKeyedImageForVisit");
+
+  if (!db_) {
+    return;
+  }
+  // Only add to the annotations table if the visit_id exists in the visits
+  // table.
+  VisitRow visit_row;
+  if (db_->GetRowForVisit(visit_id, &visit_row)) {
+    VisitContentAnnotations annotations;
+    if (db_->GetContentAnnotationsForVisit(visit_id, &annotations)) {
+      annotations.has_url_keyed_image = has_url_keyed_image;
+      db_->UpdateContentAnnotationsForVisit(visit_id, annotations);
+    } else {
+      annotations.has_url_keyed_image = has_url_keyed_image;
+      db_->AddContentAnnotationsForVisit(visit_id, annotations);
+    }
+    ScheduleCommit();
+  }
+}
+
 void HistoryBackend::UpdateVisitDuration(VisitID visit_id, const Time end_ts) {
   if (!db_)
     return;

components/history/core/browser/history_backend.h

@@ -281,6 +281,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
                                  const std::u16string& search_terms);
   void AddPageMetadataForVisit(VisitID visit_id,
                                const std::string& alternative_title);
+  void SetHasUrlKeyedImageForVisit(VisitID visit_id, bool has_url_keyed_image);
 
   // Querying ------------------------------------------------------------------
 

components/history/core/browser/history_backend_unittest.cc

@@ -2065,6 +2065,50 @@ TEST_F(HistoryBackendTest, AddPageMetadata) {
       visit_id, &got_content_annotations));
 }
 
+TEST_F(HistoryBackendTest, SetHasUrlKeyedImage) {
+  ASSERT_TRUE(backend_.get());
+
+  GURL url("http://pagewithvisit.com");
+  ContextID context_id = 1;
+  int nav_entry_id = 1;
+
+  HistoryAddPageArgs request(url, base::Time::Now(), context_id, nav_entry_id,
+                             GURL(), RedirectList(), ui::PAGE_TRANSITION_TYPED,
+                             false, SOURCE_BROWSED, false, true);
+  backend_->AddPage(request);
+
+  VisitVector visits;
+  URLRow row;
+  URLID id = backend_->db()->GetRowForURL(url, &row);
+  ASSERT_TRUE(backend_->db()->GetVisitsForURL(id, &visits));
+  ASSERT_EQ(1U, visits.size());
+  VisitID visit_id = visits[0].visit_id;
+
+  backend_->SetHasUrlKeyedImageForVisit(visit_id, /*has_url_keyed_image=*/true);
+
+  VisitContentAnnotations got_content_annotations;
+  EXPECT_TRUE(backend_->db()->GetContentAnnotationsForVisit(
+      visit_id, &got_content_annotations));
+
+  EXPECT_EQ(VisitContentAnnotationFlag::kNone,
+            got_content_annotations.annotation_flags);
+  EXPECT_EQ(-1.0f, got_content_annotations.model_annotations.visibility_score);
+  EXPECT_TRUE(got_content_annotations.model_annotations.categories.empty());
+  EXPECT_EQ(
+      -1, got_content_annotations.model_annotations.page_topics_model_version);
+  EXPECT_TRUE(got_content_annotations.model_annotations.entities.empty());
+  EXPECT_TRUE(got_content_annotations.related_searches.empty());
+  EXPECT_TRUE(got_content_annotations.search_normalized_url.is_empty());
+  EXPECT_TRUE(got_content_annotations.search_terms.empty());
+  EXPECT_TRUE(got_content_annotations.alternative_title.empty());
+  EXPECT_TRUE(got_content_annotations.has_url_keyed_image);
+
+  // Now, delete the URL. Content Annotations should be deleted.
+  backend_->DeleteURL(url);
+  ASSERT_FALSE(backend_->db()->GetContentAnnotationsForVisit(
+      visit_id, &got_content_annotations));
+}
+
 TEST_F(HistoryBackendTest, MixedContentAnnotationsRequestTypes) {
   ASSERT_TRUE(backend_.get());
 

components/history/core/browser/history_service.cc

@@ -622,6 +622,16 @@ void HistoryService::AddPageMetadataForVisit(
                               history_backend_, visit_id, alternative_title));
 }
 
+void HistoryService::SetHasUrlKeyedImageForVisit(bool has_url_keyed_image,
+                                                 VisitID visit_id) {
+  TRACE_EVENT0("browser", "HistoryService::SetHasUrlKeyedImageForVisit");
+  DCHECK(backend_task_runner_) << "History service being called after cleanup";
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  ScheduleTask(PRIORITY_NORMAL,
+               base::BindOnce(&HistoryBackend::SetHasUrlKeyedImageForVisit,
+                              history_backend_, visit_id, has_url_keyed_image));
+}
+
 void HistoryService::AddPageWithDetails(const GURL& url,
                                         const std::u16string& title,
                                         int visit_count,

components/history/core/browser/history_service.h

@@ -264,6 +264,10 @@ class HistoryService : public KeyedService {
   void AddPageMetadataForVisit(const std::string& alternative_title,
                                VisitID visit_id);
 
+  // Updates the history database by setting the `has_url_keyed_image` bit for
+  // the visit.
+  void SetHasUrlKeyedImageForVisit(bool has_url_keyed_image, VisitID visit_id);
+
   // Querying ------------------------------------------------------------------
 
   // Returns the information about the requested URL. If the URL is found,

components/optimization_guide/content/browser/page_content_annotations_service.cc

@@ -58,6 +58,8 @@ std::string PageContentAnnotationsTypeToString(
       return "SearchMetadata";
     case PageContentAnnotationsType::kRemoteMetdata:
       return "RemoteMetadata";
+    case PageContentAnnotationsType::kSalientImageMetadata:
+      return "SalientImageMetadata";
   }
 }
 
@@ -755,6 +757,26 @@ void PageContentAnnotationsService::PersistRemotePageMetadata(
   }
 }
 
+void PageContentAnnotationsService::PersistSalientImageMetadata(
+    const HistoryVisit& visit,
+    const proto::SalientImageMetadata& salient_image_metadata) {
+  if (salient_image_metadata.thumbnails_size() <= 0) {
+    return;
+  }
+
+  // Persist the detail if at least one thumbnail has a non-empty URL.
+  for (const auto& thumbnail : salient_image_metadata.thumbnails()) {
+    if (!thumbnail.image_url().empty()) {
+      QueryURL(visit,
+               base::BindOnce(
+                   &history::HistoryService::SetHasUrlKeyedImageForVisit,
+                   history_service_->AsWeakPtr(), /*has_url_keyed_image=*/true),
+               PageContentAnnotationsType::kSalientImageMetadata);
+      return;
+    }
+  }
+}
+
 void PageContentAnnotationsService::OnEntityMetadataRetrieved(
     const GURL& url,
     const std::string& entity_id,

components/optimization_guide/content/browser/page_content_annotations_service.h

@@ -34,6 +34,7 @@
 #include "components/optimization_guide/core/page_content_annotations_common.h"
 #include "components/optimization_guide/machine_learning_tflite_buildflags.h"
 #include "components/optimization_guide/proto/page_entities_metadata.pb.h"
+#include "components/optimization_guide/proto/salient_image_metadata.pb.h"
 #include "components/search_engines/template_url_service.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "url/gurl.h"
@@ -98,6 +99,11 @@ enum class PageContentAnnotationsType {
   kSearchMetadata = 3,
   // Metadata received from the remote Optimization Guide service.
   kRemoteMetdata = 4,
+  // Salient image metadata.
+  kSalientImageMetadata = 5,
+
+  // New entries should be added to the PageContentAnnotationsStorageType in
+  // optimization/histograms.xml.
 };
 
 // A KeyedService that annotates page content.
@@ -255,6 +261,13 @@ class PageContentAnnotationsService : public KeyedService,
       const HistoryVisit& visit,
       const proto::PageEntitiesMetadata& page_entities_metadata);
 
+  // Persist |salient_image_metadata| for |visit| in |history_service_|.
+  //
+  // Virtualized for testing.
+  virtual void PersistSalientImageMetadata(
+      const HistoryVisit& visit,
+      const proto::SalientImageMetadata& salient_image_metadata);
+
   // Called when entity metadata for |entity_id| that had weight |weight| on
   // page with |url| has been retrieved.
   void OnEntityMetadataRetrieved(

components/optimization_guide/content/browser/page_content_annotations_web_contents_observer.cc

@@ -82,9 +82,15 @@ PageContentAnnotationsWebContentsObserver::
       no_state_prefetch_manager_(no_state_prefetch_manager) {
   DCHECK(page_content_annotations_service_);
 
-  if (features::RemotePageMetadataEnabled() && optimization_guide_decider_) {
-    optimization_guide_decider_->RegisterOptimizationTypes(
-        {proto::PAGE_ENTITIES});
+  std::vector<proto::OptimizationType> optimization_types;
+  if (features::RemotePageMetadataEnabled()) {
+    optimization_types.emplace_back(proto::PAGE_ENTITIES);
+  }
+  if (features::ShouldPersistSalientImageMetadata()) {
+    optimization_types.emplace_back(proto::SALIENT_IMAGE);
+  }
+  if (optimization_guide_decider_ && !optimization_types.empty()) {
+    optimization_guide_decider_->RegisterOptimizationTypes(optimization_types);
   }
 }
 
@@ -125,8 +131,18 @@ void PageContentAnnotationsWebContentsObserver::DidFinishNavigation(
     optimization_guide_decider_->CanApplyOptimizationAsync(
         navigation_handle, proto::PAGE_ENTITIES,
         base::BindOnce(&PageContentAnnotationsWebContentsObserver::
-                           OnRemotePageMetadataReceived,
-                       weak_ptr_factory_.GetWeakPtr(), history_visit));
+                           OnOptimizationGuideResponseReceived,
+                       weak_ptr_factory_.GetWeakPtr(), history_visit,
+                       proto::PAGE_ENTITIES));
+  }
+  if (features::ShouldPersistSalientImageMetadata() &&
+      optimization_guide_decider_) {
+    optimization_guide_decider_->CanApplyOptimizationAsync(
+        navigation_handle, proto::SALIENT_IMAGE,
+        base::BindOnce(&PageContentAnnotationsWebContentsObserver::
+                           OnOptimizationGuideResponseReceived,
+                       weak_ptr_factory_.GetWeakPtr(), history_visit,
+                       proto::SALIENT_IMAGE));
   }
 
   bool is_google_search_url =
@@ -231,21 +247,38 @@ void PageContentAnnotationsWebContentsObserver::
   }
 }
 
-void PageContentAnnotationsWebContentsObserver::OnRemotePageMetadataReceived(
-    const HistoryVisit& history_visit,
-    OptimizationGuideDecision decision,
-    const OptimizationMetadata& metadata) {
-  if (decision != OptimizationGuideDecision::kTrue)
-    return;
-
-  absl::optional<proto::PageEntitiesMetadata> page_entities_metadata =
-      metadata.ParsedMetadata<proto::PageEntitiesMetadata>();
-  if (!page_entities_metadata)
+void PageContentAnnotationsWebContentsObserver::
+    OnOptimizationGuideResponseReceived(
+        const HistoryVisit& history_visit,
+        proto::OptimizationType optimization_type,
+        OptimizationGuideDecision decision,
+        const OptimizationMetadata& metadata) {
+  if (decision != OptimizationGuideDecision::kTrue) {
     return;
+  }
 
-  // Persist remote page metadata.
-  page_content_annotations_service_->PersistRemotePageMetadata(
-      history_visit, *page_entities_metadata);
+  switch (optimization_type) {
+    case proto::OptimizationType::PAGE_ENTITIES: {
+      absl::optional<proto::PageEntitiesMetadata> page_entities_metadata =
+          metadata.ParsedMetadata<proto::PageEntitiesMetadata>();
+      if (page_entities_metadata) {
+        page_content_annotations_service_->PersistRemotePageMetadata(
+            history_visit, *page_entities_metadata);
+      }
+      break;
+    }
+    case proto::OptimizationType::SALIENT_IMAGE: {
+      absl::optional<proto::SalientImageMetadata> salient_image_metadata =
+          metadata.ParsedMetadata<proto::SalientImageMetadata>();
+      if (salient_image_metadata) {
+        page_content_annotations_service_->PersistSalientImageMetadata(
+            history_visit, *salient_image_metadata);
+      }
+      break;
+    }
+    default:
+      NOTREACHED();
+  }
 }
 
 WEB_CONTENTS_USER_DATA_KEY_IMPL(PageContentAnnotationsWebContentsObserver);