[App Management] Remove app management router.

This CL removes the existing App Management router and uses the existing
settings router.

Bug: 999443, 1009397
Change-Id: Ife6232a27fb042b26d0eec3582388a2e8fbcaa26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1833393
Commit-Queue: Jeevan Shikaram <jshikaram@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704114}

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/BUILD.gn

@@ -22,7 +22,6 @@ js_type_check("closure_compile") {
     ":pin_to_shelf_item",
     ":pwa_permission_view",
     ":reducers",
-    ":router",
     ":store",
     ":store_client",
     ":toggle_row",
@@ -61,7 +60,6 @@ js_library("app_management_page") {
     ":actions",
     ":browser_proxy",
     ":main_view",
-    ":router",
     ":store",
     ":store_client",
   ]
@@ -74,6 +72,7 @@ js_library("app_permission_view") {
     ":dom_switch",
     ":pwa_permission_view",
     ":store_client",
+    "../../..:route",
   ]
 }
 
@@ -164,15 +163,6 @@ js_library("reducers") {
   ]
 }
 
-js_library("router") {
-  deps = [
-    ":actions",
-    ":constants",
-    ":store_client",
-    "../../..:route",
-  ]
-}
-
 js_library("store") {
   deps = [
     ":reducers",

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/actions.js

@@ -39,19 +39,12 @@ cr.define('app_management.actions', function() {
   }
 
   /**
-   * @param {PageType} pageType
-   * @param {string=} id
+   * @param {?string} appId
    */
-  function changePage(pageType, id) {
-    if (pageType === PageType.DETAIL && !id) {
-      console.warn(
-          'Tried to load app detail page without providing an app id.');
-    }
-
+  function updateSelectedAppId(appId) {
     return {
-      name: 'change-page',
-      pageType: pageType,
-      id: id,
+      name: 'update-selected-app-id',
+      value: appId,
     };
   }
 
@@ -71,7 +64,7 @@ cr.define('app_management.actions', function() {
     addApp: addApp,
     changeApp: changeApp,
     removeApp: removeApp,
-    changePage: changePage,
     updateArcSupported: updateArcSupported,
+    updateSelectedAppId: updateSelectedAppId,
   };
 });

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/app_item.js

@@ -23,8 +23,7 @@ Polymer({
    * @private
    */
   onClick_: function() {
-    this.dispatch(
-        app_management.actions.changePage(PageType.DETAIL, this.app.id));
+    app_management.util.openAppDetailPage(this.app.id);
   },
 
   /**

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/app_management_page.html

@@ -3,7 +3,6 @@
 <link rel="import" href="actions.html">
 <link rel="import" href="browser_proxy.html">
 <link rel="import" href="main_view.html">
-<link rel="import" href="router.html">
 <link rel="import" href="store_client.html">
 <link rel="import" href="store.html">
 <link rel="import" href="../../../settings_shared_css.html">
@@ -19,7 +18,6 @@
       <app-management-main-view search-term="[[searchTerm]]">
       </app-management-main-view>
     </div>
-    <app-management-router></app-management-router>
   </template>
   <script src="app_management_page.js"></script>
 </dom-module>

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/app_permission_view.js

@@ -8,6 +8,7 @@ Polymer({
 
   behaviors: [
     app_management.StoreClient,
+    settings.RouteObserverBehavior,
   ],
 
   properties: {
@@ -15,22 +16,46 @@ Polymer({
      * @type {App}
      * @private
      */
-    app_: Object,
+    app_: {
+      type: Object,
+      observer: 'appChanged_',
+    }
   },
 
   attached: function() {
-    if (!this.app_) {
-      const appId = settings.getQueryParameters().get('id');
-      // TODO(crbug.com/999443): move this changePage call to router.js
-      this.dispatch(app_management.actions.changePage(PageType.DETAIL, appId));
-    }
     this.watch('app_', state => app_management.util.getSelectedApp(state));
-    this.watch('currentPage_', state => state.currentPage);
     this.updateFromStore();
   },
 
+  detached: function() {
+    this.dispatch(app_management.actions.updateSelectedAppId(null));
+  },
+
+  /**
+   * Updates selected app ID based on the URL query params.
+   *
+   * settings.RouteObserverBehavior
+   * @param {!settings.Route} currentRoute
+   * @protected
+   */
+  currentRouteChanged: function(currentRoute) {
+    if (currentRoute !== settings.routes.APP_MANAGEMENT_DETAIL) {
+      return;
+    }
+
+    const appId = settings.getQueryParameters().get('id');
+
+    if (!this.getState().apps[appId]) {
+      // TODO(crbug.com/1010398): this call does not open the main page.
+      app_management.util.openMainPage();
+      return;
+    }
+
+    this.dispatch(app_management.actions.updateSelectedAppId(appId));
+  },
+
   /**
-   * @param {App} app
+   * @param {?App} app
    * @return {?string}
    * @private
    */
@@ -51,4 +76,14 @@ Polymer({
         assertNotReached();
     }
   },
+
+  /**
+   * @param {?App} app
+   * @private
+   */
+  appChanged_: function(app) {
+    if (app === null) {
+      app_management.util.openMainPage();
+    }
+  }
 });

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/reducers.js

@@ -74,74 +74,40 @@ cr.define('app_management', function() {
     }
   };
 
-  const CurrentPageState = {};
-
-  /**
-   * @param {AppMap} apps
-   * @param {Object} action
-   * @return {Page}
-   */
-  CurrentPageState.changePage = function(apps, action) {
-    if (action.pageType === PageType.DETAIL && apps[action.id]) {
-      return {
-        pageType: PageType.DETAIL,
-        selectedAppId: action.id,
-      };
-    } else {
-      return {
-        pageType: PageType.MAIN,
-        selectedAppId: null,
-      };
-    }
-  };
-
-  /**
-   * @param {Page} currentPage
-   * @param {Object} action
-   * @return {Page}
-   */
-  CurrentPageState.removeApp = function(currentPage, action) {
-    if (currentPage.pageType === PageType.DETAIL &&
-        currentPage.selectedAppId === action.id) {
-      return {
-        pageType: PageType.MAIN,
-        selectedAppId: null,
-      };
-    } else {
-      return currentPage;
-    }
-  };
+  const ArcSupported = {};
 
   /**
-   * @param {AppMap} apps
-   * @param {Page} currentPage
+   * @param {boolean} arcSupported
    * @param {Object} action
-   * @return {Page}
+   * @return {boolean}
    */
-  CurrentPageState.updateCurrentPage = function(apps, currentPage, action) {
+  ArcSupported.updateArcSupported = function(arcSupported, action) {
     switch (action.name) {
-      case 'change-page':
-        return CurrentPageState.changePage(apps, action);
-      case 'remove-app':
-        return CurrentPageState.removeApp(currentPage, action);
+      case 'update-arc-supported':
+        return action.value;
       default:
-        return currentPage;
+        return arcSupported;
     }
   };
 
-  const ArcSupported = {};
+  const SelectedAppId = {};
 
   /**
-   * @param {boolean} arcSupported
+   * @param {?string} selectedAppId
    * @param {Object} action
-   * @return {boolean}
+   * @return {?string}
    */
-  ArcSupported.updateArcSupported = function(arcSupported, action) {
+  SelectedAppId.updateSelectedAppId = function(selectedAppId, action) {
     switch (action.name) {
-      case 'update-arc-supported':
+      case 'update-selected-app-id':
         return action.value;
+      case 'remove-app':
+        if (selectedAppId === action.id) {
+          return null;
+        }
+        return selectedAppId;
       default:
-        return arcSupported;
+        return selectedAppId;
     }
   };
 
@@ -155,16 +121,16 @@ cr.define('app_management', function() {
   function reduceAction(state, action) {
     return {
       apps: AppState.updateApps(state.apps, action),
-      currentPage: CurrentPageState.updateCurrentPage(
-          state.apps, state.currentPage, action),
       arcSupported: ArcSupported.updateArcSupported(state.arcSupported, action),
+      selectedAppId:
+          SelectedAppId.updateSelectedAppId(state.selectedAppId, action),
     };
   }
 
   return {
     reduceAction: reduceAction,
     AppState: AppState,
-    CurrentPageState: CurrentPageState,
     ArcSupported: ArcSupported,
+    SelectedAppId: SelectedAppId,
   };
 });

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/router.html

-<link rel="import" href="chrome://resources/html/polymer.html">
-
-<link rel="import" href="actions.html">
-<link rel="import" href="store_client.html">
-<link rel="import" href="constants.html">
-
-<dom-module id="app-management-router">
-  <template>
-  </template>
-  <script src="router.js"></script>
-</dom-module>

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/router.js

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-Polymer({
-  is: 'app-management-router',
-
-  behaviors: [
-    app_management.StoreClient,
-  ],
-  //TODO (crbug.com/999443): Watch URL and update state.
-  properties: {
-    currentPage_: {
-      type: Object,
-      observer: 'onCurrentPageChanged_',
-    },
-  },
-
-  attached: function() {
-    this.watch('currentPage_', state => {
-      return state.currentPage;
-    });
-    this.updateFromStore();
-  },
-
-  onCurrentPageChanged_: function() {
-    const pageType = this.currentPage_.pageType;
-    const appId = this.currentPage_.selectedAppId;
-    switch(pageType) {
-      case PageType.DETAIL:
-        const params = new URLSearchParams;
-        params.append('id', appId);
-        settings.navigateTo(settings.routes.APP_MANAGEMENT_DETAIL, params);
-        return;
-      case PageType.MAIN:
-        settings.navigateTo(settings.routes.APP_MANAGEMENT);
-        return;
-      default:
-        assertNotReached();
-    }
-  }
-});

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/types.js

@@ -27,19 +27,11 @@ let Permission;
  */
 let AppMap;
 
-/**
- * @typedef {{
- *   pageType: PageType,
- *   selectedAppId: ?string,
- * }}
- */
-let Page;
-
 /**
  * @typedef {{
  *   apps: !AppMap,
- *   currentPage: !Page,
  *   arcSupported: boolean,
+ *   selectedAppId: ?string,
  * }}
  */
 let AppManagementPageState;

chrome/browser/resources/settings/chromeos/os_apps_page/app_management_page/util.js

@@ -13,11 +13,8 @@ cr.define('app_management.util', function() {
   function createEmptyState() {
     return {
       apps: {},
-      currentPage: {
-        pageType: PageType.MAIN,
-        selectedAppId: null,
-      },
       arcSupported: false,
+      selectedAppId: null,
     };
   }
 
@@ -144,7 +141,7 @@ cr.define('app_management.util', function() {
    * @return {?App}
    */
   function getSelectedApp(state) {
-    const selectedAppId = state.currentPage.selectedAppId;
+    const selectedAppId = state.selectedAppId;
     return selectedAppId ? state.apps[selectedAppId] : null;
   }
 
@@ -190,6 +187,24 @@ cr.define('app_management.util', function() {
     }
   }
 
+  /**
+   * Navigates to the App Detail page.
+   *
+   * @param {string} appId
+   */
+  function openAppDetailPage(appId) {
+    const params = new URLSearchParams;
+    params.append('id', appId);
+    settings.navigateTo(settings.routes.APP_MANAGEMENT_DETAIL, params);
+  }
+
+  /**
+   * Navigates to the main App Management list page.
+   */
+  function openMainPage() {
+    settings.navigateTo(settings.routes.APP_MANAGEMENT);
+  }
+
   return {
     addIfNeeded: addIfNeeded,
     alphabeticalSort: alphabeticalSort,
@@ -201,6 +216,8 @@ cr.define('app_management.util', function() {
     getPermission: getPermission,
     getPermissionValueBool: getPermissionValueBool,
     getSelectedApp: getSelectedApp,
+    openAppDetailPage: openAppDetailPage,
+    openMainPage: openMainPage,
     permissionTypeHandle: permissionTypeHandle,
     removeIfNeeded: removeIfNeeded,
     toggleOptionalBool: toggleOptionalBool,

chrome/browser/resources/settings/os_settings_resources.grd

@@ -151,12 +151,6 @@
       <structure name="IDR_OS_SETTINGS_APP_MANAGEMENT_PAGE_CHROME_APP_PERMISSION_VIEW_HTML"
                  file="chromeos/os_apps_page/app_management_page/chrome_app_permission_view.html"
                  type="chrome_html" />
-      <structure name="IDR_OS_SETTINGS_APP_MANAGEMENT_PAGE_ROUTER_JS"
-                 file="chromeos/os_apps_page/app_management_page/router.js"
-                 type="chrome_html" />
-      <structure name="IDR_OS_SETTINGS_APP_MANAGEMENT_PAGE_ROUTER_HTML"
-                 file="chromeos/os_apps_page/app_management_page/router.html"
-                 type="chrome_html" />
       <structure name="IDR_OS_SETTINGS_APP_MANAGEMENT_PAGE_ICONS_HTML"
                  file="chromeos/os_apps_page/app_management_page/icons.html"
                  type="chrome_html" />

chrome/test/data/webui/settings/chromeos/app_management/arc_permission_view_test.js

@@ -48,7 +48,7 @@ suite('<app-management-arc-permission-view>', () => {
     // Add an arc app, and make it the currently selected app.
     const app = await fakeHandler.addApp(null, arcOptions);
     app_management.Store.getInstance().dispatch(
-        app_management.actions.changePage(PageType.DETAIL, app.id));
+        app_management.actions.updateSelectedAppId(app.id));
 
     arcPermissionView =
         document.createElement('app-management-arc-permission-view');
@@ -57,7 +57,7 @@ suite('<app-management-arc-permission-view>', () => {
 
   test('App is rendered correctly', () => {
     assertEquals(
-        app_management.Store.getInstance().data.currentPage.selectedAppId,
+        app_management.Store.getInstance().data.selectedAppId,
         arcPermissionView.app_.id);
   });
 

chrome/test/data/webui/settings/chromeos/app_management/managed_apps_test.js

@@ -35,7 +35,7 @@ suite('<app-management-managed-apps>', () => {
     const app = await fakeHandler.addApp(null, policyAppOptions);
     // Select created app.
     app_management.Store.getInstance().dispatch(
-        app_management.actions.changePage(PageType.DETAIL, app.id));
+        app_management.actions.updateSelectedAppId(app.id));
     appDetailView =
         document.createElement('app-management-pwa-permission-view');
     replaceBody(appDetailView);

chrome/test/data/webui/settings/chromeos/app_management/pwa_permission_view_test.js

@@ -20,8 +20,7 @@ suite('<app-management-pwa-permission-view>', function() {
 
   function getSelectedAppFromStore() {
     const storeData = app_management.Store.getInstance().data;
-    const selectedAppId = storeData.currentPage.selectedAppId;
-    return storeData.apps[selectedAppId];
+    return storeData.apps[storeData.selectedAppId];
   }
 
   setup(async function() {
@@ -31,7 +30,7 @@ suite('<app-management-pwa-permission-view>', function() {
     // Add an app, and make it the currently selected app.
     const app = await fakeHandler.addApp();
     app_management.Store.getInstance().dispatch(
-        app_management.actions.changePage(PageType.DETAIL, app.id));
+        app_management.actions.updateSelectedAppId(app.id));
 
     pwaPermissionView =
         document.createElement('app-management-pwa-permission-view');
@@ -40,7 +39,7 @@ suite('<app-management-pwa-permission-view>', function() {
 
   test('App is rendered correctly', function() {
     assertEquals(
-        app_management.Store.getInstance().data.currentPage.selectedAppId,
+        app_management.Store.getInstance().data.selectedAppId,
         pwaPermissionView.app_.id);
   });
 

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

@@ -57,7 +57,7 @@ suite('app state', function() {
   });
 });
 
-suite('current page state', function() {
+suite('selected app id', function() {
   let state;
 
   setup(function() {
@@ -67,65 +67,31 @@ suite('current page state', function() {
     ]);
   });
 
-  test(
-      'returns to main page if an app is removed while in its detail page',
-      function() {
-        state.currentPage.selectedAppId = '1';
-        state.currentPage.pageType = PageType.DETAIL;
-
-        let action = app_management.actions.removeApp('1');
-        state = app_management.reduceAction(state, action);
-
-        assertEquals(null, state.currentPage.selectedAppId);
-        assertEquals(PageType.MAIN, state.currentPage.pageType);
-
-        // Page doesn't change if a different app is removed.
-        state.apps['1'] = createApp('1');
-        state.currentPage.selectedAppId = '1';
-        state.currentPage.pageType = PageType.DETAIL;
-
-        action = app_management.actions.removeApp('2');
-        state = app_management.reduceAction(state, action);
-
-        assertEquals('1', state.currentPage.selectedAppId);
-        assertEquals(PageType.DETAIL, state.currentPage.pageType);
-      });
-
-  test('current page updates when changing to main page', function() {
-    // Returning to main page results in no selected app.
-    state.currentPage.selectedAppId = '1';
-    state.currentPage.pageType = PageType.DETAIL;
+  test('initial state has no selected app', function() {
+    assertEquals(null, state.selectedAppId);
+  });
 
-    let action = app_management.actions.changePage(PageType.MAIN);
+  test('updates selected app id', function() {
+    let action = app_management.actions.updateSelectedAppId('1');
     state = app_management.reduceAction(state, action);
+    assertEquals('1', state.selectedAppId);
 
-    assertEquals(null, state.currentPage.selectedAppId);
-    assertEquals(PageType.MAIN, state.currentPage.pageType);
-
-    // Id is disregarded when changing to main page.
-    action = app_management.actions.changePage(PageType.MAIN, '1');
+    action = app_management.actions.updateSelectedAppId('2');
     state = app_management.reduceAction(state, action);
+    assertEquals('2', state.selectedAppId);
 
-    assertEquals(null, state.currentPage.selectedAppId);
-    assertEquals(PageType.MAIN, state.currentPage.pageType);
+    action = app_management.actions.updateSelectedAppId(null);
+    state = app_management.reduceAction(state, action);
+    assertEquals(null, state.selectedAppId);
   });
 
-  test('current page updates when changing to app detail page', function() {
-    // State updates when a valid app detail page is selected.
-    let action = app_management.actions.changePage(PageType.DETAIL, '2');
+  test('removing an app resets selected app id', function() {
+    let action = app_management.actions.updateSelectedAppId('1');
     state = app_management.reduceAction(state, action);
+    assertEquals('1', state.selectedAppId);
 
-    assertEquals('2', state.currentPage.selectedAppId);
-    assertEquals(PageType.DETAIL, state.currentPage.pageType);
-
-    // State returns to main page if invalid app id is given.
-    state.currentPage.selectedAppId = '2';
-    state.currentPage.pageType = PageType.DETAIL;
-
-    action = app_management.actions.changePage(PageType.DETAIL, '3');
+    action = app_management.actions.removeApp('1');
     state = app_management.reduceAction(state, action);
-
-    assertEquals(null, state.currentPage.selectedAppId);
-    assertEquals(PageType.MAIN, state.currentPage.pageType);
+    assertEquals(null, state.selectedAppId);
   });
 });

chrome/test/data/webui/settings/chromeos/app_management/router_test.js

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-suite('<app-management-router>', () => {
-  let store;
-  let router;
-  let fakeHandler;
-
-  setup(async () => {
-    fakeHandler = setupFakeHandler();
-    store = replaceStore();
-    await fakeHandler.addApp('1');
-    router = document.createElement('app-management-router');
-    replaceBody(router);
-  });
-
-  test('Search updates from route', async () => {
-    await navigateTo('/?q=beep');
-    const expected = app_management.actions.setSearchTerm('beep');
-    assertDeepEquals(expected, store.lastAction);
-  });
-
-  test('Selected app updates from route', async () => {
-    await navigateTo('/detail?id=1');
-    const expected = app_management.actions.changePage(PageType.DETAIL, '1');
-
-    assertDeepEquals(expected, store.lastAction);
-  });
-
-  test('Notifications view appears from route', async () => {
-    await navigateTo('/notifications');
-    const expected = app_management.actions.changePage(PageType.NOTIFICATIONS);
-    assertDeepEquals(expected, store.lastAction);
-  });
-
-  test('Route updates from state change', async () => {
-    // The application needs an initial url to start with.
-    await navigateTo('/');
-
-    store.data.currentPage = {
-      pageType: PageType.DETAIL,
-      selectedAppId: '1',
-    };
-    store.notifyObservers();
-
-    await test_util.flushTasks();
-    expectEquals('/detail?id=1', getCurrentUrlSuffix());
-
-    // Returning main page clears the route.
-    store.data.currentPage = {
-      pageType: PageType.MAIN,
-      selectedAppId: null,
-    };
-    store.notifyObservers();
-    await test_util.flushTasks();
-    expectEquals('/', getCurrentUrlSuffix());
-
-    store.data.currentPage = {
-      pageType: PageType.NOTIFICATIONS,
-      selectedAppId: null,
-    };
-    store.notifyObservers();
-    await test_util.flushTasks();
-    expectEquals('/notifications', getCurrentUrlSuffix());
-  });
-
-  test('Route updates from home to search', async () => {
-    await navigateTo('/');
-    store.data.search = {term: 'bloop'};
-    store.notifyObservers();
-    await test_util.flushTasks();
-
-    expectEquals('/?q=bloop', getCurrentUrlSuffix());
-  });
-
-  test('Route updates from detail to search', async () => {
-    await navigateTo('/detail?id=1');
-    store.data.search = {term: 'bloop'};
-    store.notifyObservers();
-    await test_util.flushTasks();
-
-    expectEquals('/detail?id=1&q=bloop', getCurrentUrlSuffix());
-  });
-});