[App Management] GetApps returns array of initial apps.

This CL changes the GetApps function of the app management PageHandler
to return the initial apps directly rather than calling onAppsAdded,
which simplifies initialisation of the ApiListener. It also changes
the onAppsAdded function to onAppAdded.

Bug: 906508
Change-Id: I649530920dfcce9d3998d9b2eda0320244ced15c
Reviewed-on: https://chromium-review.googlesource.com/c/1393587
Commit-Queue: Reka Norman <rekanorman@google.com>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619894}

chrome/browser/resources/app_management/actions.js

@@ -9,12 +9,12 @@
 
 cr.define('app_management.actions', function() {
   /**
-   * @param {Array<App>} apps
+   * @param {App} app
    */
-  function addApps(apps) {
+  function addApp(app) {
     return {
-      name: 'add-apps',
-      apps: apps,
+      name: 'add-app',
+      app: app,
     };
   }
 
@@ -56,7 +56,7 @@ cr.define('app_management.actions', function() {
   }
 
   return {
-    addApps: addApps,
+    addApp: addApp,
     changeApp: changeApp,
     removeApp: removeApp,
     changePage: changePage,

chrome/browser/resources/app_management/api_listener.js

@@ -4,37 +4,22 @@
 cr.define('app_management.ApiListener', function() {
   let initialized = false;
 
-  let initialListenerId;
-
-  function init() {
+  async function init() {
     assert(!initialized);
 
-    const callbackRouter =
-        app_management.BrowserProxy.getInstance().callbackRouter;
-
-    initialListenerId =
-        callbackRouter.onAppsAdded.addListener(initialOnAppsAdded);
-
-    app_management.BrowserProxy.getInstance().handler.getApps();
-
-    initialized = true;
-  }
-
-  /**
-   * @param {!Array<App>} apps
-   */
-  function initialOnAppsAdded(apps) {
-    const initialState = app_management.util.createInitialState(apps);
+    const {apps: initialApps} =
+        await app_management.BrowserProxy.getInstance().handler.getApps();
+    const initialState = app_management.util.createInitialState(initialApps);
     app_management.Store.getInstance().init(initialState);
 
     const callbackRouter =
         app_management.BrowserProxy.getInstance().callbackRouter;
 
-    callbackRouter.onAppsAdded.addListener(onAppsAdded);
+    callbackRouter.onAppAdded.addListener(onAppAdded);
     callbackRouter.onAppChanged.addListener(onAppChanged);
     callbackRouter.onAppRemoved.addListener(onAppRemoved);
 
-    callbackRouter.removeListener(initialListenerId);
+    initialized = true;
   }
 
   /**
@@ -45,10 +30,10 @@ cr.define('app_management.ApiListener', function() {
   }
 
   /**
-   * @param {Array<App>} apps
+   * @param {App} app
    */
-  function onAppsAdded(apps) {
-    dispatch(app_management.actions.addApps(apps));
+  function onAppAdded(app) {
+    dispatch(app_management.actions.addApp(app));
   }
 
   /**

chrome/browser/resources/app_management/fake_page_handler.js

@@ -58,7 +58,7 @@ cr.define('app_management', function() {
     }
 
     getApps() {
-      this.page.onAppsAdded.dispatch_(this.apps_);
+      return Promise.resolve({apps: this.apps_});
     }
 
     /**

chrome/browser/resources/app_management/reducers.js

@@ -17,12 +17,10 @@ cr.define('app_management', function() {
    * @param {Object} action
    * @return {AppMap}
    */
-  AppState.addApps = function(apps, action) {
-    const newAppEntries = {};
-    for (const app of action.apps) {
-      newAppEntries[app.id] = app;
-    }
-    return Object.assign({}, apps, newAppEntries);
+  AppState.addApp = function(apps, action) {
+    const newAppEntry = {};
+    newAppEntry[action.app.id] = action.app;
+    return Object.assign({}, apps, newAppEntry);
   };
 
   /**
@@ -57,8 +55,8 @@ cr.define('app_management', function() {
    */
   AppState.updateApps = function(apps, action) {
     switch (action.name) {
-      case 'add-apps':
-        return AppState.addApps(apps, action);
+      case 'add-app':
+        return AppState.addApp(apps, action);
       case 'change-app':
         return AppState.changeApp(apps, action);
       case 'remove-app':

chrome/browser/ui/webui/app_management/app_management.mojom

@@ -27,12 +27,12 @@ interface PageHandlerFactory {
 
 // Browser interface.
 interface PageHandler {
-  GetApps();
+  GetApps() => (array<App> apps);
 };
 
 // Frontend interface.
 interface Page {
-  OnAppsAdded(array<App> apps);
+  OnAppAdded(App app);
   OnAppChanged(App update);
   OnAppRemoved(string app_id);
 };

chrome/browser/ui/webui/app_management/app_management_page_handler.cc

@@ -12,6 +12,18 @@
 #include "chrome/services/app_service/public/cpp/app_registry_cache.h"
 #include "chrome/services/app_service/public/mojom/types.mojom.h"
 
+namespace {
+
+app_management::mojom::AppPtr CreateUIAppPtr(const apps::AppUpdate& update) {
+  return app_management::mojom::App::New(
+      update.AppId(), update.AppType(), update.Name(),
+      base::nullopt /*description*/,
+      apps::mojom::OptionalBool::kUnknown /*is_pinned*/,
+      base::nullopt /*version*/, base::nullopt /*size*/);
+}
+
+}  // namespace
+
 AppManagementPageHandler::AppManagementPageHandler(
     app_management::mojom::PageHandlerRequest request,
     app_management::mojom::PagePtr page,
@@ -22,7 +34,7 @@ AppManagementPageHandler::AppManagementPageHandler(
 
 AppManagementPageHandler::~AppManagementPageHandler() {}
 
-void AppManagementPageHandler::GetApps() {
+void AppManagementPageHandler::GetApps(GetAppsCallback callback) {
   apps::AppServiceProxy* proxy = apps::AppServiceProxy::Get(profile_);
 
   // TODO(crbug.com/826982): revisit pending decision on AppServiceProxy in
@@ -32,13 +44,12 @@ void AppManagementPageHandler::GetApps() {
 
   std::vector<app_management::mojom::AppPtr> apps;
   proxy->Cache().ForEachApp([&apps](const apps::AppUpdate& update) {
-    apps.push_back(app_management::mojom::App::New(
-        update.AppId(), update.AppType(), update.Name(),
-        base::nullopt /*description*/,
-        apps::mojom::OptionalBool::kUnknown /*is_pinned*/,
-        base::nullopt /*version*/, base::nullopt /*size*/));
+    apps.push_back(CreateUIAppPtr(update));
   });
-  page_->OnAppsAdded(std::move(apps));
+
+  Observe(&proxy->Cache());
+
+  std::move(callback).Run(std::move(apps));
 }
 
 void AppManagementPageHandler::OnAppUpdate(const apps::AppUpdate& update) {
@@ -48,9 +59,10 @@ void AppManagementPageHandler::OnAppUpdate(const apps::AppUpdate& update) {
     return;
   }
 
-  page_->OnAppChanged(app_management::mojom::App::New(
-      update.AppId(), update.AppType(), update.Name(),
-      base::nullopt /*description*/,
-      apps::mojom::OptionalBool::kUnknown /*is_pinned*/,
-      base::nullopt /*version*/, base::nullopt /*size*/));
+  if (update.ReadinessChanged() &&
+      update.Readiness() == apps::mojom::Readiness::kReady) {
+    page_->OnAppAdded(CreateUIAppPtr(update));
+  } else {
+    page_->OnAppChanged(CreateUIAppPtr(update));
+  }
 }

chrome/browser/ui/webui/app_management/app_management_page_handler.h

@@ -25,7 +25,7 @@ class AppManagementPageHandler : public app_management::mojom::PageHandler,
   ~AppManagementPageHandler() override;
 
   // app_management::mojom::PageHandler:
-  void GetApps() override;
+  void GetApps(GetAppsCallback callback) override;
 
  private:
   // apps::AppRegistryCache::Observer overrides:

chrome/test/data/webui/app_management/app_test.js

@@ -3,11 +3,9 @@
 // found in the LICENSE file.
 
 suite('<app-management-app>', function() {
-  test('loads', function(done) {
-    app_management.BrowserProxy.getInstance().handler.getApps();
-
-    let callbackRouter =
-        app_management.BrowserProxy.getInstance().callbackRouter;
-    callbackRouter.onAppsAdded.addListener(() => done());
+  test('loads', async function() {
+    // Check that the browser responds to the getApps() message.
+    const {apps: initialApps} =
+        await app_management.BrowserProxy.getInstance().handler.getApps();
   });
 });

chrome/test/data/webui/app_management/main_view_test.js

@@ -41,9 +41,15 @@ suite('<app-management-main-view>', function() {
     return apps;
   }
 
+  function addApps(apps) {
+    for (const app of apps) {
+      callbackRouterProxy.onAppAdded(app);
+    }
+  }
+
   test('simple app addition', async function() {
     let apps = createTestApps(1);
-    callbackRouterProxy.onAppsAdded(apps);
+    addApps(apps);
     await callbackRouterProxy.flushForTesting();
     let appItems = mainView.root.querySelectorAll('app-management-item');
     expectEquals(1, appItems.length);
@@ -58,14 +64,14 @@ suite('<app-management-main-view>', function() {
     expectTrue(mainView.$['expander-row'].hidden);
 
     // The more apps bar shouldn't appear when there are 4 apps.
-    callbackRouterProxy.onAppsAdded(createTestApps(4));
+    addApps(createTestApps(4));
     await callbackRouterProxy.flushForTesting();
     expectEquals(
         4, mainView.root.querySelectorAll('app-management-item').length);
     expectTrue(mainView.$['expander-row'].hidden);
 
     // The more apps bar appears when there are 5 apps.
-    callbackRouterProxy.onAppsAdded(createTestApps(1));
+    addApps(createTestApps(1));
     await callbackRouterProxy.flushForTesting();
     expectEquals(
         5, mainView.root.querySelectorAll('app-management-item').length);

chrome/test/data/webui/app_management/reducers_test.js

@@ -25,20 +25,16 @@ suite('app state', function() {
     ]);
   });
 
-  test('updates when apps are added', function() {
-    appsToAdd = [
-      createApp('3', {type: 1, title: 'a'}),
-      createApp('4'),
-    ];
+  test('updates when an app is added', function() {
+    const newApp = createApp('3', {type: 1, title: 'a'});
 
-    action = app_management.actions.addApps(appsToAdd);
+    action = app_management.actions.addApp(newApp);
     apps = app_management.AppState.updateApps(apps, action);
 
     // Check that apps contains a key for each app id.
     assertTrue(!!apps['1']);
     assertTrue(!!apps['2']);
     assertTrue(!!apps['3']);
-    assertTrue(!!apps['4']);
 
     // Check that id corresponds to the right app.
     const app = apps['3'];