[CrOS Settings] Add support for observing search availability

This CL adds a new Mojo API for observing changes to search
availability. It will be used by the search UI so that the UI updates
itself when its results become stale and no longer usable.

Bug: 1096867
Change-Id: I89e77d01d86d6d55aa69548f267dfb88f8d0090a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2261191Reviewed-by: Regan Hsu <hsuregan@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Auto-Submit: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781899}

chrome/browser/ui/webui/settings/chromeos/search/search.mojom

@@ -97,6 +97,18 @@ struct SearchResult {
   SearchResultIdentifier id;
 };
 
+// Used to observe changes to search results.
+interface SearchResultsObserver {
+  // Called when the availability of one or more search results has changed. In
+  // this context, "availability" refers to whether a search result can be
+  // returned based on the user's current state. E.g., "Cellular" results are
+  // only shown if the device has an attached modem, so this function would be
+  // called whenever the user plugs in or unplugs a USB modem. Clients can use
+  // this function to ensure that they do not show "stale" results which are no
+  // longer actionable by the user.
+  OnSearchResultAvailabilityChanged();
+};
+
 // Provides settings search results. Implemented in the browser process;
 // intended to be called from settings JS and Launcher C++.
 interface SearchHandler {
@@ -112,4 +124,8 @@ interface SearchHandler {
          uint32 max_num_results,
          ParentResultBehavior parent_result_behavior) =>
              (array<SearchResult> results);
+
+  // Adds an observer of search results. Disconnected observers are discarded;
+  // to stop observing, close the connection.
+  Observe(pending_remote<SearchResultsObserver> observer);
 };

chrome/browser/ui/webui/settings/chromeos/search/search_handler.cc

@@ -12,7 +12,6 @@
 #include "chrome/browser/ui/webui/settings/chromeos/os_settings_sections.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search_concept.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search_result_icon.mojom.h"
-#include "chrome/browser/ui/webui/settings/chromeos/search/search_tag_registry.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -49,9 +48,13 @@ SearchHandler::SearchHandler(
       sections_(sections),
       hierarchy_(hierarchy),
       index_(local_search_service->GetIndex(
-          local_search_service::IndexId::kCrosSettings)) {}
+          local_search_service::IndexId::kCrosSettings)) {
+  search_tag_registry_->AddObserver(this);
+}
 
-SearchHandler::~SearchHandler() = default;
+SearchHandler::~SearchHandler() {
+  search_tag_registry_->RemoveObserver(this);
+}
 
 void SearchHandler::BindInterface(
     mojo::PendingReceiver<mojom::SearchHandler> pending_receiver) {
@@ -92,6 +95,16 @@ void SearchHandler::Search(const base::string16& query,
       Search(query, max_num_results, parent_result_behavior));
 }
 
+void SearchHandler::Observe(
+    mojo::PendingRemote<mojom::SearchResultsObserver> observer) {
+  observers_.Add(std::move(observer));
+}
+
+void SearchHandler::OnRegistryUpdated() {
+  for (auto& observer : observers_)
+    observer->OnSearchResultAvailabilityChanged();
+}
+
 std::vector<mojom::SearchResultPtr> SearchHandler::GenerateSearchResultsArray(
     const std::vector<local_search_service::Result>&
         local_search_service_results,

chrome/browser/ui/webui/settings/chromeos/search/search_handler.h

@@ -11,9 +11,12 @@
 #include "base/optional.h"
 #include "chrome/browser/chromeos/local_search_service/index.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search.mojom.h"
+#include "chrome/browser/ui/webui/settings/chromeos/search/search_tag_registry.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/bindings/remote_set.h"
 
 namespace local_search_service {
 class LocalSearchService;
@@ -25,7 +28,6 @@ namespace settings {
 class Hierarchy;
 class OsSettingsSections;
 struct SearchConcept;
-class SearchTagRegistry;
 
 // Handles search queries for Chrome OS settings. Search() is expected to be
 // invoked by the settings UI as well as the the Launcher search UI. Search
@@ -34,7 +36,8 @@ class SearchTagRegistry;
 // SearchTagRegistry.
 //
 // Searches which do not provide any matches result in an empty results array.
-class SearchHandler : public mojom::SearchHandler {
+class SearchHandler : public mojom::SearchHandler,
+                      public SearchTagRegistry::Observer {
  public:
   SearchHandler(SearchTagRegistry* search_tag_registry,
                 OsSettingsSections* sections,
@@ -59,10 +62,15 @@ class SearchHandler : public mojom::SearchHandler {
               uint32_t max_num_results,
               mojom::ParentResultBehavior parent_result_behavior,
               SearchCallback callback) override;
+  void Observe(
+      mojo::PendingRemote<mojom::SearchResultsObserver> observer) override;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(SearchHandlerTest, CompareSearchResults);
 
+  // SearchTagRegistry::Observer:
+  void OnRegistryUpdated() override;
+
   std::vector<mojom::SearchResultPtr> GenerateSearchResultsArray(
       const std::vector<local_search_service::Result>&
           local_search_service_results,
@@ -99,8 +107,9 @@ class SearchHandler : public mojom::SearchHandler {
   Hierarchy* hierarchy_;
   local_search_service::Index* index_;
 
-  // Note: Expected to have multiple clients, so a ReceiverSet is used.
+  // Note: Expected to have multiple clients, so ReceiverSet/RemoteSet are used.
   mojo::ReceiverSet<mojom::SearchHandler> receivers_;
+  mojo::RemoteSet<mojom::SearchResultsObserver> observers_;
 };
 
 }  // namespace settings

chrome/browser/ui/webui/settings/chromeos/search/search_handler_unittest.cc

@@ -24,6 +24,27 @@ namespace chromeos {
 namespace settings {
 namespace {
 
+class FakeObserver : public mojom::SearchResultsObserver {
+ public:
+  FakeObserver() = default;
+  ~FakeObserver() override = default;
+
+  mojo::PendingRemote<mojom::SearchResultsObserver> GenerateRemote() {
+    mojo::PendingRemote<mojom::SearchResultsObserver> remote;
+    receiver_.Bind(remote.InitWithNewPipeAndPassReceiver());
+    return remote;
+  }
+
+  size_t num_calls() const { return num_calls_; }
+
+ private:
+  // mojom::SearchResultsObserver:
+  void OnSearchResultAvailabilityChanged() override { ++num_calls_; }
+
+  size_t num_calls_;
+  mojo::Receiver<mojom::SearchResultsObserver> receiver_{this};
+};
+
 // Note: Copied from printing_section.cc but does not need to stay in sync with
 // it.
 const std::vector<SearchConcept>& GetPrintingSearchConcepts() {
@@ -92,6 +113,9 @@ class SearchHandlerTest : public testing::Test {
                                        mojom::Setting::kAddPrinter);
     fake_hierarchy_.AddSettingMetadata(mojom::Section::kPrinting,
                                        mojom::Setting::kSavedPrinters);
+
+    handler_remote_->Observe(observer_.GenerateRemote());
+    handler_remote_.FlushForTesting();
   }
 
   base::test::TaskEnvironment task_environment_;
@@ -102,11 +126,15 @@ class SearchHandlerTest : public testing::Test {
   FakeHierarchy fake_hierarchy_;
   SearchHandler handler_;
   mojo::Remote<mojom::SearchHandler> handler_remote_;
+  FakeObserver observer_;
 };
 
 TEST_F(SearchHandlerTest, AddAndRemove) {
   // Add printing search tags to registry and search for "Print".
   search_tag_registry_.AddSearchTags(GetPrintingSearchConcepts());
+  handler_remote_.FlushForTesting();
+  EXPECT_EQ(1u, observer_.num_calls());
+
   std::vector<mojom::SearchResultPtr> search_results;
 
   // 3 results should be available for a "Print" query.
@@ -142,6 +170,7 @@ TEST_F(SearchHandlerTest, AddAndRemove) {
               mojom::ParentResultBehavior::kDoNotIncludeParentResults,
               &search_results);
   EXPECT_TRUE(search_results.empty());
+  EXPECT_EQ(2u, observer_.num_calls());
 }
 
 TEST_F(SearchHandlerTest, UrlModification) {

chrome/browser/ui/webui/settings/chromeos/search/search_tag_registry.cc

@@ -41,6 +41,14 @@ SearchTagRegistry::SearchTagRegistry(
 
 SearchTagRegistry::~SearchTagRegistry() = default;
 
+void SearchTagRegistry::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void SearchTagRegistry::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
 void SearchTagRegistry::AddSearchTags(
     const std::vector<SearchConcept>& search_tags) {
   if (!base::FeatureList::IsEnabled(features::kNewOsSettingsSearch))
@@ -54,6 +62,8 @@ void SearchTagRegistry::AddSearchTags(
   for (const auto& concept : search_tags) {
     result_id_to_metadata_list_map_[ToResultId(concept)] = &concept;
   }
+
+  NotifyRegistryUpdated();
 }
 
 void SearchTagRegistry::RemoveSearchTags(
@@ -69,6 +79,8 @@ void SearchTagRegistry::RemoveSearchTags(
   }
 
   index_->Delete(data_ids);
+
+  NotifyRegistryUpdated();
 }
 
 const SearchConcept* SearchTagRegistry::GetTagMetadata(
@@ -120,5 +132,10 @@ SearchTagRegistry::ConceptVectorToDataVector(
   return data_list;
 }
 
+void SearchTagRegistry::NotifyRegistryUpdated() {
+  for (auto& observer : observer_list_)
+    observer.OnRegistryUpdated();
+}
+
 }  // namespace settings
 }  // namespace chromeos

chrome/browser/ui/webui/settings/chromeos/search/search_tag_registry.h

@@ -9,6 +9,8 @@
 #include <vector>
 
 #include "base/gtest_prod_util.h"
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
 #include "chrome/browser/chromeos/local_search_service/index.h"
 #include "chrome/browser/ui/webui/settings/chromeos/os_settings_section.h"
 
@@ -26,12 +28,21 @@ struct SearchConcept;
 // LocalSearchService and providing metadata via GetTagMetadata().
 class SearchTagRegistry {
  public:
+  class Observer : public base::CheckedObserver {
+   public:
+    ~Observer() override = default;
+    virtual void OnRegistryUpdated() = 0;
+  };
+
   SearchTagRegistry(
       local_search_service::LocalSearchService* local_search_service);
   SearchTagRegistry(const SearchTagRegistry& other) = delete;
   SearchTagRegistry& operator=(const SearchTagRegistry& other) = delete;
   virtual ~SearchTagRegistry();
 
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   void AddSearchTags(const std::vector<SearchConcept>& search_tags);
   void RemoveSearchTags(const std::vector<SearchConcept>& search_tags);
 
@@ -47,6 +58,7 @@ class SearchTagRegistry {
 
   std::vector<local_search_service::Data> ConceptVectorToDataVector(
       const std::vector<SearchConcept>& search_tags);
+  void NotifyRegistryUpdated();
 
   // Index used by the LocalSearchService for string matching.
   local_search_service::Index* index_;
@@ -55,6 +67,8 @@ class SearchTagRegistry {
   // LocalSearchService. Contents are kept in sync with |index_|.
   std::unordered_map<std::string, const SearchConcept*>
       result_id_to_metadata_list_map_;
+
+  base::ObserverList<Observer> observer_list_;
 };
 
 }  // namespace settings

chrome/browser/ui/webui/settings/chromeos/search/search_tag_registry_unittest.cc

@@ -16,6 +16,20 @@ namespace chromeos {
 namespace settings {
 namespace {
 
+class FakeObserver : public SearchTagRegistry::Observer {
+ public:
+  FakeObserver() = default;
+  ~FakeObserver() override = default;
+
+  size_t num_calls() const { return num_calls_; }
+
+ private:
+  // SearchTagRegistry::Observer:
+  void OnRegistryUpdated() override { ++num_calls_; }
+
+  size_t num_calls_;
+};
+
 // Note: Copied from printing_section.cc but does not need to stay in sync with
 // it.
 const std::vector<SearchConcept>& GetPrintingSearchConcepts() {
@@ -53,12 +67,16 @@ class SearchTagRegistryTest : public testing::Test {
 
   // testing::Test:
   void SetUp() override {
+    search_tag_registry_.AddObserver(&observer_);
     index_ = local_search_service_.GetIndex(
         local_search_service::IndexId::kCrosSettings);
   }
 
+  void TearDown() override { search_tag_registry_.RemoveObserver(&observer_); }
+
   local_search_service::LocalSearchService local_search_service_;
   SearchTagRegistry search_tag_registry_;
+  FakeObserver observer_;
   local_search_service::Index* index_;
 };
 
@@ -66,6 +84,7 @@ TEST_F(SearchTagRegistryTest, AddAndRemove) {
   // Add search tags; size of the index should increase.
   search_tag_registry_.AddSearchTags(GetPrintingSearchConcepts());
   EXPECT_EQ(3u, index_->GetSize());
+  EXPECT_EQ(1u, observer_.num_calls());
 
   std::string first_tag_id =
       SearchTagRegistry::ToResultId(GetPrintingSearchConcepts()[0]);
@@ -79,6 +98,7 @@ TEST_F(SearchTagRegistryTest, AddAndRemove) {
   // Remove search tag; size should go back to 0.
   search_tag_registry_.RemoveSearchTags(GetPrintingSearchConcepts());
   EXPECT_EQ(0u, index_->GetSize());
+  EXPECT_EQ(2u, observer_.num_calls());
 
   // The tag should no longer be accessible via GetTagMetadata().
   add_printer_concept = search_tag_registry_.GetTagMetadata(first_tag_id);

chrome/test/data/webui/settings/chromeos/fake_settings_search_handler.js

@@ -15,6 +15,9 @@ cr.define('settings', function() {
     constructor() {
       /** @private {!Array<chromeos.settings.mojom.SearchResult>} */
       this.fakeResults_ = [];
+
+      /** @type {!chromeos.settings.mojom.SearchResultsObserverInterface} */
+      this.observer;
     }
 
     /**
@@ -29,6 +32,11 @@ cr.define('settings', function() {
     async search(query, maxNumResults, parentResultBehavior) {
       return {results: this.fakeResults_};
     }
+
+    /** override */
+    observe(observer) {
+      this.observer = observer;
+    }
   }
 
   // #cr_define_end