[CrOS Settings] Add SectionMetata to Hierarchy

This class adds metadata regarding whether the section contains only a
link to a subpage. Additionally, this CL moves section result generation
to a public function on this class.

Bug: 1082249, 1071700
Change-Id: Ib191d6a2636adf102d0cd4fde5c1fd298a220e17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2219154Reviewed-by: Regan Hsu <hsuregan@chromium.org>
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772465}

chrome/browser/ui/webui/settings/chromeos/hierarchy.cc

@@ -19,8 +19,12 @@ namespace settings {
 class Hierarchy::PerSectionHierarchyGenerator
     : public OsSettingsSection::HierarchyGenerator {
  public:
-  PerSectionHierarchyGenerator(mojom::Section section, Hierarchy* hierarchy)
-      : section_(section), hierarchy_(hierarchy) {}
+  PerSectionHierarchyGenerator(mojom::Section section,
+                               bool* only_contains_link_to_subpage,
+                               Hierarchy* hierarchy)
+      : section_(section),
+        only_contains_link_to_subpage_(only_contains_link_to_subpage),
+        hierarchy_(hierarchy) {}
 
   void RegisterTopLevelSubpage(
       int name_message_id,
@@ -32,6 +36,13 @@ class Hierarchy::PerSectionHierarchyGenerator
         name_message_id, subpage, icon, default_rank, url_path_with_parameters);
     CHECK_EQ(section_, metadata.section)
         << "Subpage registered in multiple sections: " << subpage;
+
+    ++num_top_level_subpages_so_far_;
+
+    // If there are multiple top-level subpages, the section contains more than
+    // just a link to a subpage.
+    if (num_top_level_subpages_so_far_ > 1u)
+      *only_contains_link_to_subpage_ = false;
   }
 
   void RegisterNestedSubpage(
@@ -56,6 +67,10 @@ class Hierarchy::PerSectionHierarchyGenerator
         << "Setting registered in multiple primary sections: " << setting;
     CHECK(!metadata.primary.second)
         << "Setting registered in multiple primary locations: " << setting;
+
+    // If a top-level setting exists, the section contains more than just a link
+    // link to a subpage.
+    *only_contains_link_to_subpage_ = false;
   }
 
   void RegisterNestedSetting(mojom::Setting setting,
@@ -78,6 +93,10 @@ class Hierarchy::PerSectionHierarchyGenerator
           << "Setting has multiple identical alternate locations: " << setting;
     }
     metadata.alternates.emplace_back(section_, /*subpage=*/base::nullopt);
+
+    // If a top-level setting exists, the section contains more than just a link
+    // link to a subpage.
+    *only_contains_link_to_subpage_ = false;
   }
 
   void RegisterNestedAltSetting(mojom::Setting setting,
@@ -134,10 +153,28 @@ class Hierarchy::PerSectionHierarchyGenerator
     return pair.first->second;
   }
 
+  size_t num_top_level_subpages_so_far_ = 0u;
   mojom::Section section_;
+  bool* only_contains_link_to_subpage_;
   Hierarchy* hierarchy_;
 };
 
+// Note: |only_contains_link_to_subpage| starts out as true and is set to false
+// if other content is added.
+Hierarchy::SectionMetadata::SectionMetadata(mojom::Section section,
+                                            const Hierarchy* hierarchy)
+    : only_contains_link_to_subpage(true),
+      section_(section),
+      hierarchy_(hierarchy) {}
+
+Hierarchy::SectionMetadata::~SectionMetadata() = default;
+
+mojom::SearchResultPtr Hierarchy::SectionMetadata::ToSearchResult(
+    double relevance_score) const {
+  return hierarchy_->sections_->GetSection(section_)
+      ->GenerateSectionSearchResult(relevance_score);
+}
+
 Hierarchy::SubpageMetadata::SubpageMetadata(
     int name_message_id,
     mojom::Section section,
@@ -177,7 +214,13 @@ Hierarchy::SettingMetadata::~SettingMetadata() = default;
 
 Hierarchy::Hierarchy(const OsSettingsSections* sections) : sections_(sections) {
   for (const auto& section : constants::AllSections()) {
-    PerSectionHierarchyGenerator generator(section, this);
+    auto pair = section_map_.emplace(std::piecewise_construct,
+                                     std::forward_as_tuple(section),
+                                     std::forward_as_tuple(section, this));
+    CHECK(pair.second);
+
+    PerSectionHierarchyGenerator generator(
+        section, &pair.first->second.only_contains_link_to_subpage, this);
     sections->GetSection(section)->RegisterHierarchy(&generator);
   }
 }
@@ -186,11 +229,12 @@ Hierarchy::Hierarchy() = default;
 
 Hierarchy::~Hierarchy() = default;
 
-mojom::SearchResultPtr Hierarchy::GenerateSectionSearchResult(
-    mojom::Section section,
-    double relevance_score) const {
-  return sections_->GetSection(section)->GenerateSectionSearchResult(
-      relevance_score);
+const Hierarchy::SectionMetadata& Hierarchy::GetSectionMetadata(
+    mojom::Section section) const {
+  const auto it = section_map_.find(section);
+  CHECK(it != section_map_.end())
+      << "Section missing from settings hierarchy: " << section;
+  return it->second;
 }
 
 const Hierarchy::SubpageMetadata& Hierarchy::GetSubpageMetadata(

chrome/browser/ui/webui/settings/chromeos/hierarchy.h

@@ -43,6 +43,25 @@ class Hierarchy {
   Hierarchy& operator=(const Hierarchy& other) = delete;
   virtual ~Hierarchy();
 
+  class SectionMetadata {
+   public:
+    SectionMetadata(mojom::Section section, const Hierarchy* hierarchy);
+    ~SectionMetadata();
+
+    // Whether the only contents of the section is a link to a subpage.
+    bool only_contains_link_to_subpage;
+
+    // Generates a search result for this section, using the canonical search
+    // tag as the search result text. |relevance_score| must be passed by the
+    // client, since this result is being created manually instead of via query
+    // matching.
+    mojom::SearchResultPtr ToSearchResult(double relevance_score) const;
+
+   private:
+    mojom::Section section_;
+    const Hierarchy* hierarchy_;
+  };
+
   class SubpageMetadata {
    public:
     SubpageMetadata(int name_message_id,
@@ -104,13 +123,7 @@ class Hierarchy {
     std::vector<SettingLocation> alternates;
   };
 
-  // Generates a search result corresponding to |section|. |relevance_score|
-  // must be passed by the client, since this result is being created manually
-  // instead of via query matching.
-  mojom::SearchResultPtr GenerateSectionSearchResult(
-      mojom::Section section,
-      double relevance_score) const;
-
+  const SectionMetadata& GetSectionMetadata(mojom::Section section) const;
   const SubpageMetadata& GetSubpageMetadata(mojom::Subpage subpage) const;
   const SettingMetadata& GetSettingMetadata(mojom::Setting setting) const;
 
@@ -118,6 +131,7 @@ class Hierarchy {
   // Used by tests.
   Hierarchy();
 
+  std::unordered_map<mojom::Section, SectionMetadata> section_map_;
   std::unordered_map<mojom::Subpage, SubpageMetadata> subpage_map_;
   std::unordered_map<mojom::Setting, SettingMetadata> setting_map_;
 

chrome/browser/ui/webui/settings/chromeos/os_settings_manager_unittest.cc

@@ -49,6 +49,16 @@ TEST_F(OsSettingsManagerTest, Initialization) {
         << "No OsSettingsSection instance created for " << section << ".";
   }
 
+  // Each mojom::Section should be registered in the hierarchy.
+  for (const auto& section : constants::AllSections()) {
+    const Hierarchy::SectionMetadata& metadata =
+        manager_->hierarchy_->GetSectionMetadata(section);
+
+    // Only the "About Chrome OS" section contains only a link to a subpage.
+    EXPECT_EQ(metadata.only_contains_link_to_subpage,
+              section == mojom::Section::kAboutChromeOs);
+  }
+
   // Each mojom::Subpage should be registered in the hierarchy. Note that
   // GetSubpageMetadata() internally CHECK()s that the metadata exists before
   // returning it.