[ZPS] Update prefs-based cache to account for ZPS prefetching on SRP

The core prefs-based cache implementation used for ZPS prefetching has
historically only stored (prefetched) ZPS responses for non-URL-based
NTP contexts. Now that the code-path for ZPS prefetching on SRP has
been implemented, with appropriate feature flag gating, the cache
implementation needs to be updated to account for the presence of ZPS
responses in SRP/Web contexts.

This CL augments the existing prefs-based cache to utilize an
additional dictionary pref dedicated to storing SRP/Web ZPS responses
keyed off the associated page URL.

Bug: 1344004
Change-Id: Ied7b737f4352eac839ca12999602be550e97b1ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3794439
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Khalid Peer <khalidpeer@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034205}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -1131,8 +1131,11 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
           nullable_filter.is_null() || nullable_filter.Run(search_url);
     }
 
-    if (should_clear_zero_suggest_and_session_token)
+    if (should_clear_zero_suggest_and_session_token) {
       prefs->SetString(omnibox::kZeroSuggestCachedResults, std::string());
+      prefs->SetDict(omnibox::kZeroSuggestCachedResultsWithURL,
+                     base::Value::Dict());
+    }
 
     // |search_prefetch_service| is null if |profile_| is off the record.
     auto* search_prefetch_service =

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -2066,14 +2066,18 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest,
 
 TEST_F(ChromeBrowsingDataRemoverDelegateTest, ZeroSuggestCacheClear) {
   PrefService* prefs = GetProfile()->GetPrefs();
+  omnibox::SetUserPreferenceForZeroSuggestCachedResponse(
+      prefs, "https://google.com/search?q=chrome", R"(["", ["foo", "bar"]])");
   prefs->SetString(omnibox::kZeroSuggestCachedResults,
-                   "[\"\", [\"foo\", \"bar\"]]");
+                   R"(["", ["foo", "bar"]])");
   BlockUntilBrowsingDataRemoved(base::Time(), base::Time::Max(),
                                 content::BrowsingDataRemover::DATA_TYPE_COOKIES,
                                 false);
 
   // Expect the prefs to be cleared when cookies are removed.
   EXPECT_TRUE(prefs->GetString(omnibox::kZeroSuggestCachedResults).empty());
+  EXPECT_TRUE(
+      prefs->GetValueDict(omnibox::kZeroSuggestCachedResultsWithURL).empty());
   EXPECT_EQ(content::BrowsingDataRemover::DATA_TYPE_COOKIES, GetRemovalMask());
   EXPECT_EQ(content::BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB,
             GetOriginTypeMask());

chrome/browser/downgrade/snapshot_file_collector.cc

@@ -55,11 +55,12 @@ std::vector<SnapshotItemDetails> CollectUserDataItems() {
 // Returns a list of items to snapshot that should be under a profile directory.
 std::vector<SnapshotItemDetails> CollectProfileItems() {
   // Data mask to delete the pref files if any of the following types is
-  // deleted. When cookies are deleted, the kZeroSuggestCachedResults pref has
-  // to be reset. When history and isolated origins are deleted, the
-  // kPrefLastLaunchTime and kUserTriggeredIsolatedOrigins prefs have to be
-  // reset. When data type content is deleted, blocklisted sites are deleted
-  // from the translation prefs.
+  // deleted. When cookies are deleted, the kZeroSuggestCachedResults and
+  // kZeroSuggestCachedResultsWithURL prefs have to be reset. When history and
+  // isolated origins are deleted, the kPrefLastLaunchTime and
+  // kUserTriggeredIsolatedOrigins prefs have to be reset. When data type
+  // content is deleted, blocklisted sites are deleted from the translation
+  // prefs.
   uint64_t pref_data_type =
       content::BrowsingDataRemover::DATA_TYPE_COOKIES |
       chrome_browsing_data_remover::DATA_TYPE_ISOLATED_ORIGINS |

components/omnibox/browser/omnibox_prefs.cc

@@ -42,9 +42,15 @@ const char kSuggestionGroupVisibility[] = "omnibox.suggestionGroupVisibility";
 // Boolean that specifies whether to always show full URLs in the omnibox.
 const char kPreventUrlElisionsInOmnibox[] = "omnibox.prevent_url_elisions";
 
-// A cache of zero suggest results using JSON serialized into a string.
+// A cache of NTP zero suggest results using a JSON dictionary serialized into a
+// string.
 const char kZeroSuggestCachedResults[] = "zerosuggest.cachedresults";
 
+// A cache of SRP/Web zero suggest results using a JSON dictionary serialized
+// into a string keyed off the page URL.
+const char kZeroSuggestCachedResultsWithURL[] =
+    "zerosuggest.cachedresults_with_url";
+
 void RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(kSuggestionGroupVisibility);
   registry->RegisterBooleanPref(
@@ -88,4 +94,25 @@ void SetUserPreferenceForSuggestionGroupVisibility(
       ->Add(suggestion_group_id);
 }
 
+void SetUserPreferenceForZeroSuggestCachedResponse(
+    PrefService* prefs,
+    const std::string& page_url,
+    const std::string& response) {
+  DCHECK(prefs);
+
+  DictionaryPrefUpdate update(prefs, kZeroSuggestCachedResultsWithURL);
+  update->SetStringKey(page_url, response);
+}
+
+std::string GetUserPreferenceForZeroSuggestCachedResponse(
+    PrefService* prefs,
+    const std::string& page_url) {
+  DCHECK(prefs);
+
+  const base::Value::Dict& dictionary =
+      prefs->GetValueDict(omnibox::kZeroSuggestCachedResultsWithURL);
+  auto* value_ptr = dictionary.FindString(page_url);
+  return (value_ptr ? *value_ptr : std::string());
+}
+
 }  // namespace omnibox

components/omnibox/browser/omnibox_prefs.h

@@ -5,6 +5,8 @@
 #ifndef COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_PREFS_H_
 #define COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_PREFS_H_
 
+#include <string>
+
 class PrefRegistrySimple;
 class PrefService;
 
@@ -37,6 +39,7 @@ extern const char kKeywordSpaceTriggeringEnabled[];
 extern const char kSuggestionGroupVisibility[];
 extern const char kPreventUrlElisionsInOmnibox[];
 extern const char kZeroSuggestCachedResults[];
+extern const char kZeroSuggestCachedResultsWithURL[];
 
 void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
@@ -61,6 +64,18 @@ void SetUserPreferenceForSuggestionGroupVisibility(
     int suggestion_group_id,
     SuggestionGroupVisibility visibility);
 
+// Updates the ZPS dictionary preference to cache the given |response| value
+// using the |page_url| as the cache key.
+void SetUserPreferenceForZeroSuggestCachedResponse(PrefService* prefs,
+                                                   const std::string& page_url,
+                                                   const std::string& response);
+
+// Returns the cached response from the ZPS dictionary preference associated
+// with the given |page_url|.
+std::string GetUserPreferenceForZeroSuggestCachedResponse(
+    PrefService* prefs,
+    const std::string& page_url);
+
 }  // namespace omnibox
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_PREFS_H_

components/omnibox/browser/zero_suggest_provider.cc

@@ -195,6 +195,7 @@ bool StoreRemoteResponse(const std::string& response_json,
                          bool is_prefetch,
                          SearchSuggestionParser::Results* results) {
   DCHECK(results);
+  DCHECK_NE(ZeroSuggestProvider::ResultType::kNone, result_type);
 
   if (response_json.empty()) {
     return false;
@@ -213,11 +214,17 @@ bool StoreRemoteResponse(const std::string& response_json,
     return false;
   }
 
-  // Store the valid response only if running the kRemoteNoURL variant.
+  // Update the relevant prefs in the cache, based on |result_type|.
   if (result_type == ZeroSuggestProvider::ResultType::kRemoteNoURL) {
     client->GetPrefs()->SetString(omnibox::kZeroSuggestCachedResults,
                                   response_json);
     LogEvent(Event::kRemoteResponseCached, result_type, is_prefetch);
+  } else if (base::FeatureList::IsEnabled(
+                 omnibox::kZeroSuggestPrefetchingOnSRP) &&
+             result_type == ZeroSuggestProvider::ResultType::kRemoteSendURL) {
+    omnibox::SetUserPreferenceForZeroSuggestCachedResponse(
+        client->GetPrefs(), input.current_url().spec(), response_json);
+    LogEvent(Event::kRemoteResponseCached, result_type, is_prefetch);
   }
 
   return true;
@@ -233,14 +240,20 @@ bool ReadStoredResponse(const AutocompleteProviderClient* client,
                         ZeroSuggestProvider::ResultType result_type,
                         SearchSuggestionParser::Results* results) {
   DCHECK(results);
+  DCHECK_NE(ZeroSuggestProvider::ResultType::kNone, result_type);
 
-  // Use the stored response only if running the kRemoteNoURL variant.
-  if (result_type != ZeroSuggestProvider::ResultType::kRemoteNoURL) {
-    return false;
+  std::string response_json;
+
+  if (result_type == ZeroSuggestProvider::ResultType::kRemoteNoURL) {
+    response_json =
+        client->GetPrefs()->GetString(omnibox::kZeroSuggestCachedResults);
+  } else if (base::FeatureList::IsEnabled(
+                 omnibox::kZeroSuggestPrefetchingOnSRP) &&
+             result_type == ZeroSuggestProvider::ResultType::kRemoteSendURL) {
+    response_json = omnibox::GetUserPreferenceForZeroSuggestCachedResponse(
+        client->GetPrefs(), input.current_url().spec());
   }
 
-  const std::string response_json =
-      client->GetPrefs()->GetString(omnibox::kZeroSuggestCachedResults);
   if (response_json.empty()) {
     return false;
   }
@@ -364,6 +377,8 @@ ZeroSuggestProvider* ZeroSuggestProvider::Create(
 void ZeroSuggestProvider::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterStringPref(omnibox::kZeroSuggestCachedResults,
                                std::string());
+  registry->RegisterDictionaryPref(omnibox::kZeroSuggestCachedResultsWithURL,
+                                   base::Value::Dict());
 }
 
 void ZeroSuggestProvider::StartPrefetch(const AutocompleteInput& input) {
@@ -472,11 +487,13 @@ void ZeroSuggestProvider::Stop(bool clear_cached_results,
 void ZeroSuggestProvider::DeleteMatch(const AutocompleteMatch& match) {
   // Remove the deleted match from the cache, so it is not shown to the user
   // again. Since we cannot remove just one result, blow away the cache.
-  // Although the cache is currently only used for kRemoteNoURL, we have no
-  // easy way of checking the request type after-the-fact. It's safe though, to
-  // always clear the cache even if we are on a different request type.
+  // Even though we currently have no easy way of checking the request type
+  // after-the-fact, it's safe to always clear the cache even if we are on a
+  // different request type.
   client()->GetPrefs()->SetString(omnibox::kZeroSuggestCachedResults,
                                   std::string());
+  client()->GetPrefs()->SetDict(omnibox::kZeroSuggestCachedResultsWithURL,
+                                base::Value::Dict());
   BaseSearchProvider::DeleteMatch(match);
 }