arc: Optimize app search provider.

App search provider already has optimization that runs update result
deferred in certain cases. This also includes cases when new app
registered or existing app updated. However in last case we always do
refresh apps inline. This CL also defers Refresh app when it
possible. Additionally that has protection not to list not yet
registered ARC default apps.

TEST=Manually, using 2 step fix. On first step verified that
     ArcAppListPrefs did not return not yet registered default app. At
     second step verified that app data search provider refreshes apps
     deferred and functionality is not broken
BUG=907590

Change-Id: I12b015c02e0c59d19e49926f11d5632adf25ad9a
Reviewed-on: https://chromium-review.googlesource.com/c/1356012Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612817}

chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc

@@ -633,8 +633,15 @@ std::vector<std::string> ArcAppListPrefs::GetAppIds() const {
   if (!IsArcAlive() || !IsArcAndroidEnabledForProfile(profile_)) {
     // Default ARC apps available before OptIn.
     std::vector<std::string> ids;
-    for (const auto& default_app : default_apps_->GetActiveApps())
+    for (const auto& default_app : default_apps_->GetActiveApps()) {
+      // Default apps are iteratively added to prefs. That generates
+      // |OnAppRegistered| event per app. Consumer may use this event to request
+      // list of all apps. Although this practice is discouraged due the
+      // performance reason, let be safe and in order to prevent listing of not
+      // yet registered apps, filter out default apps based of tracked state.
+      if (tracked_apps_.count(default_app.first))
         ids.push_back(default_app.first);
+    }
     return ids;
   }
   return GetAppIdsNoArcEnabledCheck();
@@ -662,8 +669,9 @@ std::unique_ptr<ArcAppListPrefs::AppInfo> ArcAppListPrefs::GetApp(
     const std::string& app_id) const {
   // Information for default app is available before ARC enabled.
   if ((!IsArcAlive() || !IsArcAndroidEnabledForProfile(profile_)) &&
-      !default_apps_->HasApp(app_id))
+      !default_apps_->HasApp(app_id)) {
     return std::unique_ptr<AppInfo>();
+  }
 
   return GetAppFromPrefs(app_id);
 }

chrome/browser/ui/app_list/search/app_search_provider.cc

@@ -276,13 +276,13 @@ class ExtensionDataSource : public AppSearchProvider::DataSource,
   // extensions::ExtensionRegistryObserver overrides:
   void OnExtensionLoaded(content::BrowserContext* browser_context,
                          const extensions::Extension* extension) override {
-    owner()->RefreshAppsAndUpdateResults(false);
+    owner()->RefreshAppsAndUpdateResultsDeferred();
   }
 
   void OnExtensionUninstalled(content::BrowserContext* browser_context,
                               const extensions::Extension* extension,
                               extensions::UninstallReason reason) override {
-    owner()->RefreshAppsAndUpdateResults(true);
+    owner()->RefreshAppsAndUpdateResults();
   }
 
  private:
@@ -369,21 +369,21 @@ class ArcDataSource : public AppSearchProvider::DataSource,
   // ArcAppListPrefs::Observer overrides:
   void OnAppRegistered(const std::string& app_id,
                        const ArcAppListPrefs::AppInfo& app_info) override {
-    owner()->RefreshAppsAndUpdateResults(false);
+    owner()->RefreshAppsAndUpdateResultsDeferred();
   }
 
   void OnAppStatesChanged(const std::string& app_id,
                           const ArcAppListPrefs::AppInfo& app_info) override {
-    owner()->RefreshAppsAndUpdateResults(false);
+    owner()->RefreshAppsAndUpdateResultsDeferred();
   }
 
   void OnAppRemoved(const std::string& id) override {
-    owner()->RefreshAppsAndUpdateResults(true);
+    owner()->RefreshAppsAndUpdateResults();
   }
 
   void OnAppNameUpdated(const std::string& id,
                         const std::string& name) override {
-    owner()->RefreshAppsAndUpdateResults(false);
+    owner()->RefreshAppsAndUpdateResultsDeferred();
   }
 
  private:
@@ -396,18 +396,17 @@ class InternalDataSource : public AppSearchProvider::DataSource {
       : AppSearchProvider::DataSource(profile, owner) {
     sync_sessions::SessionSyncService* service =
         SessionSyncServiceFactory::GetInstance()->GetForProfile(profile);
-    if (service) {
+    if (!service)
+      return;
     // base::Unretained() is safe below because the subscription itself is a
     // class member field and handles destruction well.
     foreign_session_updated_subscription_ =
         service->SubscribeToForeignSessionsChanged(base::BindRepeating(
-              &AppSearchProvider::RefreshAppsAndUpdateResults,
-              base::Unretained(owner),
-              /*force_inline=*/false));
-    }
+            &AppSearchProvider::RefreshAppsAndUpdateResultsDeferred,
+            base::Unretained(owner)));
   }
 
-  ~InternalDataSource() override {}
+  ~InternalDataSource() override = default;
 
   // AppSearchProvider::DataSource overrides:
   void AddApps(AppSearchProvider::Apps* apps) override {
@@ -511,7 +510,10 @@ class CrostiniDataSource : public AppSearchProvider::DataSource,
       const std::vector<std::string>& updated_apps,
       const std::vector<std::string>& removed_apps,
       const std::vector<std::string>& inserted_apps) override {
-    owner()->RefreshAppsAndUpdateResults(!removed_apps.empty());
+    if (removed_apps.empty())
+      owner()->RefreshAppsAndUpdateResultsDeferred();
+    else
+      owner()->RefreshAppsAndUpdateResults();
   }
 
  private:
@@ -531,6 +533,7 @@ AppSearchProvider::AppSearchProvider(Profile* profile,
       ranker_(std::make_unique<AppSearchResultRanker>(
           profile->GetPath(),
           chromeos::ProfileHelper::IsEphemeralUserProfile(profile))),
+      refresh_apps_factory_(this),
       update_results_factory_(this) {
   data_sources_.emplace_back(
       std::make_unique<ExtensionDataSource>(profile, this));
@@ -552,8 +555,8 @@ void AppSearchProvider::Start(const base::string16& query) {
   // Refresh list of apps to ensure we have the latest launch time information.
   // This will also cause the results to update.
   if (show_recommendations || apps_.empty())
-    RefreshApps();
-
+    RefreshAppsAndUpdateResults();
+  else
     UpdateResults();
 }
 
@@ -561,11 +564,25 @@ void AppSearchProvider::Train(const std::string& id) {
   ranker_->Train(id);
 }
 
-void AppSearchProvider::RefreshApps() {
+void AppSearchProvider::RefreshAppsAndUpdateResults() {
+  // Clear any pending requests if any.
+  refresh_apps_factory_.InvalidateWeakPtrs();
+
   apps_.clear();
   apps_.reserve(kMinimumReservedAppsContainerCapacity);
   for (auto& data_source : data_sources_)
     data_source->AddApps(&apps_);
+  UpdateResults();
+}
+
+void AppSearchProvider::RefreshAppsAndUpdateResultsDeferred() {
+  // Check if request is pending.
+  if (refresh_apps_factory_.HasWeakPtrs())
+    return;
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(&AppSearchProvider::RefreshAppsAndUpdateResults,
+                                refresh_apps_factory_.GetWeakPtr()));
 }
 
 void AppSearchProvider::UpdateRecommendedResults(
@@ -681,18 +698,4 @@ void AppSearchProvider::UpdateResults() {
   }
 }
 
-void AppSearchProvider::RefreshAppsAndUpdateResults(bool force_inline) {
-  RefreshApps();
-
-  if (force_inline) {
-    UpdateResults();
-  } else {
-    if (!update_results_factory_.HasWeakPtrs()) {
-      base::ThreadTaskRunnerHandle::Get()->PostTask(
-          FROM_HERE, base::Bind(&AppSearchProvider::UpdateResults,
-                                update_results_factory_.GetWeakPtr()));
-    }
-  }
-}
-
 }  // namespace app_list

chrome/browser/ui/app_list/search/app_search_provider.h

@@ -47,14 +47,15 @@ class AppSearchProvider : public SearchProvider {
   void Start(const base::string16& query) override;
   void Train(const std::string& id) override;
 
-  // Refresh indexed app data and update search results. When |force_inline| is
-  // set to true, search results is updated before returning from the function.
-  // Otherwise, search results would be grouped, i.e. multiple calls would only
-  // update search results once.
-  void RefreshAppsAndUpdateResults(bool force_inline);
+  // Refreshes apps and updates results inline
+  void RefreshAppsAndUpdateResults();
+
+  // Refreshes apps deferred to prevent multiple redundant refreshes in case of
+  // batch update events from app providers. Used in case when no removed app is
+  // detected.
+  void RefreshAppsAndUpdateResultsDeferred();
 
  private:
-  void RefreshApps();
   void UpdateResults();
   void UpdateRecommendedResults(
       const base::flat_map<std::string, uint16_t>& id_to_app_list_index);
@@ -68,6 +69,7 @@ class AppSearchProvider : public SearchProvider {
   base::Clock* clock_;
   std::vector<std::unique_ptr<DataSource>> data_sources_;
   std::unique_ptr<AppSearchResultRanker> ranker_;
+  base::WeakPtrFactory<AppSearchProvider> refresh_apps_factory_;
   base::WeakPtrFactory<AppSearchProvider> update_results_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AppSearchProvider);