[omnibox][grouping] Adds zps groups for Desktop

Adds an AdjustLimitsFromMatches() to `Section` that can be overridden by
subclasses to adjust the limits prior to allocating matches to groups.
Given that matches are seen in descending order of relevance, its
default implementation for ZPS `Section`s ensures that matches with
higher relevance scores do not fill up the `Section` if others with
lower scores should be placed earlier based on their `Group`s position.

Also adjusts Android zps groups to what's currently launched.

Bug: 1343512
Change-Id: Ib62ce3971ce145aa6a74529724cd0f3feae615ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4128228
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Reviewed-by: Tomasz Wiszkowski <ender@google.com>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096969}

components/omnibox/browser/autocomplete_grouper_groups.cc

@@ -26,17 +26,32 @@ bool Group::CanAdd(const AutocompleteMatch& match) const {
     return false;
   }
   const auto& limit_and_count = group_id_limits_and_counts_.at(group_id);
-  // Check this `Group`s total limit and the limit the particular `group_id`.
-  return matches_.size() < limit_ &&
-         limit_and_count.count < limit_and_count.limit;
+  // Check this `Group`'s total limit and the limit for the `group_id`.
+  return count_ < limit_ && limit_and_count.count < limit_and_count.limit;
 }
 
 void Group::Add(const AutocompleteMatch& match) {
   DCHECK(CanAdd(match));
   matches_.push_back(match);
+  Count(match);
+}
+
+void Group::Count(const AutocompleteMatch& match) {
+  count_++;
   group_id_limits_and_counts_[match.suggestion_group_id.value()].count++;
 }
 
+void Group::AdjustLimitsAndResetCounts(size_t max_limit) {
+  DCHECK(matches_.empty()) << "Must be called once before adding the matches.";
+  limit_ = std::min({limit_, max_limit, count_});
+  count_ = 0;
+  for (auto& [group_id, limit_and_count] : group_id_limits_and_counts_) {
+    limit_and_count.limit =
+        std::min({limit_and_count.limit, limit_, limit_and_count.count});
+    limit_and_count.count = 0;
+  }
+}
+
 DefaultGroup::DefaultGroup()
     : Group(1,
             GroupIdLimitsAndCounts{{omnibox::GROUP_STARTER_PACK, {1}},

components/omnibox/browser/autocomplete_grouper_groups.h

@@ -30,10 +30,23 @@ class Group {
   Group(size_t limit, omnibox::GroupId group_id);
   virtual ~Group();
 
-  // Returns if `match` can be added to this `Group`.
+  // Returns if `match` can be added to this `Group`. Checks if the `GroupId` of
+  // the match is permitted in this `Group`, this `Group`'s total limit, and the
+  // limit for the `GroupId` of the match.
   virtual bool CanAdd(const AutocompleteMatch& match) const;
-  // Adds `match` to this `Group`. `CanAdd()` should be verified by the caller.
+  // Adds `match` to this `Group` and increments this `Group`'s total count and
+  // the count for the `GroupId` of the match.
+  // `CanAdd()` should be verified by the caller.
   void Add(const AutocompleteMatch& match);
+  // Increments this `Group`'s total count and the count for the `GroupId` of
+  // the match but does not add `match` to this `Group`.
+  void Count(const AutocompleteMatch& match);
+  // Adjusts the `Group`'s total limit and the limits for the `GroupId`s in the
+  // `Group` based on the number of matches counted and the given max limit.
+  // Ensures that the limits are less than or equal to their original values.
+  // Resets the `Group`'s total count and the counts for the `GroupId`s in the
+  // `Group` so that matches can actually be added to the `Group`.
+  void AdjustLimitsAndResetCounts(size_t max_limit);
 
   size_t limit() { return limit_; }
   void set_limit(size_t limit) { limit_ = limit; }
@@ -42,6 +55,8 @@ class Group {
  private:
   // Max number of matches this `Group` can contain.
   size_t limit_{0};
+  // The number of matches this `Group` contains.
+  size_t count_{0};
   // The limit and count per `GroupId`.
   GroupIdLimitsAndCounts group_id_limits_and_counts_;
   // The matches this `Group` contains.

components/omnibox/browser/autocomplete_grouper_sections.cc

@@ -20,6 +20,10 @@ Section::~Section() = default;
 
 // static
 ACMatches Section::GroupMatches(PSections sections, ACMatches matches) {
+  for (auto& section : sections) {
+    section->InitFromMatches(matches);
+  }
+
   for (const auto& match : matches) {
     DCHECK(match.suggestion_group_id.has_value());
     for (const auto& section : sections) {
@@ -39,9 +43,7 @@ ACMatches Section::GroupMatches(PSections sections, ACMatches matches) {
   return grouped_matches;
 }
 
-Group* Section::CanAdd(const AutocompleteMatch& match) {
-  if (size_ >= limit_)
-    return nullptr;
+Group* Section::FindGroup(const AutocompleteMatch& match) {
   auto iter = base::ranges::find_if(
       groups_, [&](const auto& group) { return group->CanAdd(match); });
   if (iter == groups_.end())
@@ -50,35 +52,79 @@ Group* Section::CanAdd(const AutocompleteMatch& match) {
 }
 
 bool Section::Add(const AutocompleteMatch& match) {
-  Group* group = CanAdd(match);
+  if (count_ >= limit_) {
+    return false;
+  }
+  Group* group = FindGroup(match);
   if (group) {
     group->Add(match);
-    size_++;
+    count_++;
   }
   return group;
 }
 
-MobileZeroInputSection::MobileZeroInputSection() : Section(20) {
-  groups_.push_back(std::make_unique<Group>(
-      10, Group::GroupIdLimitsAndCounts{
-              {omnibox::GROUP_MOBILE_SEARCH_READY_OMNIBOX, {1}},
-              {omnibox::GROUP_MOBILE_CLIPBOARD, {1}},
-              {omnibox::GROUP_MOBILE_MOST_VISITED, {8}},
-              {omnibox::GROUP_PERSONALIZED_ZERO_SUGGEST, {10}}}));
-  groups_.push_back(std::make_unique<Group>(5, omnibox::GROUP_TRENDS));
+ZpsSection::ZpsSection(size_t limit) : Section(limit) {}
+
+void ZpsSection::InitFromMatches(const ACMatches& matches) {
+  // Iterate over the matches to see if they can be added to any `Group` in this
+  // `Section`. If so, increment the total count for this `Section` and for the
+  // respective group.
+  for (const auto& match : matches) {
+    Group* group = FindGroup(match);
+    if (group) {
+      count_++;
+      group->Count(match);
+    }
+  }
+
+  // Adjust the `Section`'s total limit based on the number of matches in the
+  // `Section`. Ensure the limit is less than or equal to the original value.
+  // Reset the count so that matches can actually be added to this `Section`.
+  limit_ = std::min(limit_, count_);
+  count_ = 0;
+
+  size_t remaining = limit_;
+  for (const auto& group : groups_) {
+    group->AdjustLimitsAndResetCounts(remaining);
+    remaining -= group->limit();
+  }
+  DCHECK_EQ(remaining, 0U);
+}
+
+AndroidZpsSection::AndroidZpsSection() : ZpsSection(15) {
+  groups_.push_back(
+      std::make_unique<Group>(1, omnibox::GROUP_MOBILE_SEARCH_READY_OMNIBOX));
+  groups_.push_back(
+      std::make_unique<Group>(1, omnibox::GROUP_MOBILE_CLIPBOARD));
+  groups_.push_back(
+      std::make_unique<Group>(1, omnibox::GROUP_MOBILE_MOST_VISITED));
+  groups_.push_back(
+      std::make_unique<Group>(15, omnibox::GROUP_PREVIOUS_SEARCH_RELATED));
+  groups_.push_back(
+      std::make_unique<Group>(15, omnibox::GROUP_PERSONALIZED_ZERO_SUGGEST));
+}
+
+DesktopZpsSection::DesktopZpsSection() : ZpsSection(8) {
+  groups_.push_back(
+      std::make_unique<Group>(8, omnibox::GROUP_PREVIOUS_SEARCH_RELATED));
+  groups_.push_back(
+      std::make_unique<Group>(8, omnibox::GROUP_PERSONALIZED_ZERO_SUGGEST));
+  groups_.push_back(std::make_unique<Group>(8, omnibox::GROUP_TRENDS));
 }
 
-DesktopNonZpsSection::DesktopNonZpsSection(const ACMatches& matches)
-    : Section(10) {
-  // Create the 4 groups with reasonable placeholder limits. Some of the limits
-  // will be adjusted below.
-  auto default_group = std::make_unique<DefaultGroup>();
-  auto starter_pack_group =
-      std::make_unique<Group>(9, omnibox::GROUP_STARTER_PACK);
-  auto search_group = std::make_unique<Group>(
+DesktopNonZpsSection::DesktopNonZpsSection() : Section(10) {
+  groups_.push_back(std::make_unique<DefaultGroup>());
+  groups_.push_back(std::make_unique<Group>(9, omnibox::GROUP_STARTER_PACK));
+  groups_.push_back(std::make_unique<Group>(
       9, Group::GroupIdLimitsAndCounts{{omnibox::GROUP_SEARCH, {9}},
-                                       {omnibox::GROUP_HISTORY_CLUSTER, {1}}});
-  auto nav_group = std::make_unique<Group>(7, omnibox::GROUP_OTHER_NAVS);
+                                       {omnibox::GROUP_HISTORY_CLUSTER, {1}}}));
+  groups_.push_back(std::make_unique<Group>(7, omnibox::GROUP_OTHER_NAVS));
+}
+
+void DesktopNonZpsSection::InitFromMatches(const ACMatches& matches) {
+  auto* default_group = groups_[0].get();
+  auto* search_group = groups_[2].get();
+  auto* nav_group = groups_[3].get();
 
   // Determine if `matches` contains any searches.
   bool has_search = base::ranges::any_of(
@@ -101,11 +147,8 @@ DesktopNonZpsSection::DesktopNonZpsSection(const ACMatches& matches)
   limit_ = std::clamp<size_t>(first_nav_index, 8, 10);
 
   // Show at least 1 search, either in the default group or the search group.
-  if (has_search && !default_is_search)
+  if (has_search && !default_is_search) {
+    DCHECK_GE(limit_, 2U);
     nav_group->set_limit(limit_ - 2);
-
-  groups_.push_back(std::move(default_group));
-  groups_.push_back(std::move(starter_pack_group));
-  groups_.push_back(std::move(search_group));
-  groups_.push_back(std::move(nav_group));
+  }
 }

components/omnibox/browser/autocomplete_grouper_sections.h

@@ -26,30 +26,57 @@ class Section {
   // Returns `matches` ranked and culled according to `sections`. All `matches`
   // should have `suggestion_group_id` set and be sorted by relevance.
   static ACMatches GroupMatches(PSections sections, ACMatches matches);
+  // Used to adjust this `Section`'s total limit and the total limits for the
+  // `Group`s in this `Section` based on the given matches.
+  virtual void InitFromMatches(const ACMatches& matches) {}
 
  protected:
-  // Return the `Group` `match` can be added to, or `nullptr` if it can't be
-  // added to any group in `groups_`.
-  virtual Group* CanAdd(const AutocompleteMatch& match);
-  // Tries to add `match` to the appropriate `groups_`. Returns if it was added
-  // to any group in `groups_`.
+  // Returns the first `Group` in this `Section` `match` can be added to or
+  // `nullptr` if none can be found. Does not take the total limit into account.
+  Group* FindGroup(const AutocompleteMatch& match);
+  // Returns whether `match` was added to a `Group` in this `Section`. Does not
+  // add a match beyond the total limit.
   bool Add(const AutocompleteMatch& match);
 
   // Max number of matches this `Section` can contain across `groups_`.
-  size_t limit_ = 0;
+  size_t limit_{0};
   // The number of matches this `Section` contains across `groups_`.
-  size_t size_ = 0;
-  // The `groups_` this `Section` contains.
-  PGroups groups_ = {};
+  size_t count_{0};
+  // The `Group`s this `Section` contains.
+  PGroups groups_{};
 };
 
-// Section containing up to 15 searches and 5 trending suggestions.
-class MobileZeroInputSection : public Section {
+// Base section for zps limits and grouping.
+// Since zero-prefix matches are seen in descending order of relevance, the
+// default implementation of `InitFromMatches()` ensures that matches with
+// higher relevance scores do not fill up the section if others with lower
+// scores are expected to be placed earlier based on their `Group`s position.
+class ZpsSection : public Section {
  public:
-  MobileZeroInputSection();
+  explicit ZpsSection(size_t limit);
+  // Section:
+  void InitFromMatches(const ACMatches& matches) override;
 };
 
-// Section expressing the desktop, non-zps limits and grouping. The rules are:
+// Section expressing the Android zps limits and grouping. The rules are:
+// - Contains up to 1 verbatim, 1 clipboard, 1 most visited, 8 related search
+//   suggestions, and 15 personalized suggestions.
+// - Allow up to 15 suggestions total.
+class AndroidZpsSection : public ZpsSection {
+ public:
+  AndroidZpsSection();
+};
+
+// Section expressing the Desktop zps limits and grouping. The rules are:
+// - Containing up to 8 related search suggestions, 8 personalized suggestions,
+//   and 8 trending search suggestions.
+// - Allow up to 8 suggestions total.
+class DesktopZpsSection : public ZpsSection {
+ public:
+  DesktopZpsSection();
+};
+
+// Section expressing the Desktop, non-zps limits and grouping. The rules are:
 // - Contains up to 1 default, 10 starer packs, 10 search, 8 nav, and 1 history
 //   cluster suggestions.
 // - Allow up to 10 suggestions total.
@@ -60,7 +87,9 @@ class MobileZeroInputSection : public Section {
 // - Group defaults 1st, then searches and history clusters, then navs.
 class DesktopNonZpsSection : public Section {
  public:
-  explicit DesktopNonZpsSection(const ACMatches& matches);
+  DesktopNonZpsSection();
+  // Section:
+  void InitFromMatches(const ACMatches& matches) override;
 };
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_AUTOCOMPLETE_GROUPER_SECTIONS_H_