CrOS Settings: Revise ARC VM Manage USB subpage availability

Avoid plumbing the boolean through the DOM and instead contain the value as a property only for those elements that need it. Bug: b/280885989 Test: browser_tests --gtest_filter="OSSettingsAppsPage*" Change-Id: I0341f8fd51ee1f844568eaf45b49db1588135fe6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4548046 Commit-Queue: Wes Okuhara <wesokuhara@google.com> Reviewed-by: Tim Sergeant <tsergeant@chromium.org> Cr-Commit-Position: refs/heads/main@{#1146494}
chromium · May 19, 2023 · 48c7bc0 · 48c7bc0
1 parent 894971b
commit 48c7bc0
Show file tree

Hide file tree

Showing 14 changed files with 24 additions and 32 deletions.
diff --git a/chrome/browser/resources/settings/chromeos/common/load_time_booleans.ts b/chrome/browser/resources/settings/chromeos/common/load_time_booleans.ts
@@ -20,6 +20,10 @@ export function androidAppsVisible(): boolean {
   return loadTimeData.getBoolean('androidAppsVisible');
 }
 
+export function isArcVmEnabled(): boolean {
+  return loadTimeData.getBoolean('isArcVmEnabled');
+}
+
 export function isPlayStoreAvailable(): boolean {
   return loadTimeData.getBoolean('isPlayStoreAvailable');
 }

diff --git a/chrome/browser/resources/settings/chromeos/os_apps_page/android_apps_subpage.html b/chrome/browser/resources/settings/chromeos/os_apps_page/android_apps_subpage.html
@@ -8,7 +8,7 @@
   </cr-link-row>
 </template>
 
-<template is="dom-if" if="[[showArcvmManageUsb]]">
+<template is="dom-if" if="[[isArcVmManageUsbAvailable]]">
   <cr-link-row
       class="hr"
       label="$i18n{guestOsSharedUsbDevicesLabel}"

diff --git a/chrome/browser/resources/settings/chromeos/os_apps_page/android_apps_subpage.ts b/chrome/browser/resources/settings/chromeos/os_apps_page/android_apps_subpage.ts
@@ -71,7 +71,7 @@ class SettingsAndroidAppsSubpageElement extends
       },
 
       /** Whether Arc VM manage usb subpage should be shown. */
-      showArcvmManageUsb: Boolean,
+      isArcVmManageUsbAvailable: Boolean,
 
       /**
        * Used by DeepLinkingMixin to focus this page's deep links.
@@ -87,7 +87,7 @@ class SettingsAndroidAppsSubpageElement extends
   }
 
   androidAppsInfo: AndroidAppsInfo;
-  showArcvmManageUsb: boolean;
+  isArcVmManageUsbAvailable: boolean;
   private dialogBody_: string;
   private playStoreEnabled_: boolean;
 

diff --git a/chrome/browser/resources/settings/chromeos/os_apps_page/os_apps_page.html b/chrome/browser/resources/settings/chromeos/os_apps_page/os_apps_page.html
@@ -119,15 +119,15 @@
     <template is="dom-if" route-path="/androidAppsDetails">
       <os-settings-subpage page-title="$i18n{androidAppsPageLabel}">
         <settings-android-apps-subpage
-          android-apps-info="[[androidAppsInfo]]"
           prefs="{{prefs}}"
-          show-arcvm-manage-usb="[[showArcvmManageUsb]]">
+          android-apps-info="[[androidAppsInfo]]"
+          is-arc-vm-manage-usb-available="[[isArcVmManageUsbAvailable_]]">
         </settings-android-apps-subpage>
       </os-settings-subpage>
     </template>
   </template>
 
-  <template is="dom-if" if="[[showArcvmManageUsb]]" restamp>
+  <template is="dom-if" if="[[isArcVmManageUsbAvailable_]]" restamp>
     <template is="dom-if" route-path="/androidAppsDetails/sharedUsbDevices">
       <os-settings-subpage page-title="$i18n{guestOsSharedUsbDevicesLabel}">
          <settings-guest-os-shared-usb-devices guest-os-type="arcvm">

diff --git a/chrome/browser/resources/settings/chromeos/os_apps_page/os_apps_page.ts b/chrome/browser/resources/settings/chromeos/os_apps_page/os_apps_page.ts
@@ -35,7 +35,7 @@ import {assert} from 'chrome://resources/js/assert_ts.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
 import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {androidAppsVisible, isPlayStoreAvailable, isPluginVmAvailable} from '../common/load_time_booleans.js';
+import {androidAppsVisible, isArcVmEnabled, isPlayStoreAvailable, isPluginVmAvailable} from '../common/load_time_booleans.js';
 import {DeepLinkingMixin} from '../deep_linking_mixin.js';
 import {App as AppWithNotifications, AppNotificationsHandlerInterface, AppNotificationsObserverReceiver, Readiness} from '../mojom-webui/app_notification_handler.mojom-webui.js';
 import {Setting} from '../mojom-webui/setting.mojom-webui.js';
@@ -100,10 +100,12 @@ class OsSettingsAppsPageElement extends OsSettingsAppsPageElementBase {
         },
       },
 
-      /**
-       * Show ARCVM Manage USB related settings and sub-page.
-       */
-      showArcvmManageUsb: Boolean,
+      isArcVmManageUsbAvailable_: {
+        type: Boolean,
+        value: () => {
+          return isArcVmEnabled();
+        },
+      },
 
       /**
        * Whether the App Notifications page should be shown.
@@ -192,11 +194,11 @@ class OsSettingsAppsPageElement extends OsSettingsAppsPageElementBase {
 
   androidAppsInfo: AndroidAppsInfo;
   searchTerm: string;
-  showArcvmManageUsb: boolean;
   private app_: App;
   private appNotificationsObserverReceiver_: AppNotificationsObserverReceiver;
   private appsWithNotifications_: AppWithNotifications[];
   private focusConfig_: Map<string, string>;
+  private isArcVmManageUsbAvailable_: boolean;
   private isDndEnabled_: boolean;
   private isPlayStoreAvailable_: boolean;
   private isPluginVmAvailable_: boolean;

diff --git a/chrome/browser/resources/settings/chromeos/os_settings_main/os_settings_main.html b/chrome/browser/resources/settings/chromeos/os_settings_main/os_settings_main.html
@@ -41,7 +41,6 @@
       class="cr-centered-card-container"
       prefs="{{prefs}}"
       page-availability="[[pageAvailability]]"
-      show-arcvm-manage-usb="[[showArcvmManageUsb]]";
       on-showing-section="onShowingSection_"
       on-subpage-expand="onShowingSubpage_"
       on-showing-main-page="onShowingMainPage_"

diff --git a/chrome/browser/resources/settings/chromeos/os_settings_main/os_settings_main.ts b/chrome/browser/resources/settings/chromeos/os_settings_main/os_settings_main.ts
@@ -93,17 +93,13 @@ export class OsSettingsMainElement extends OsSettingsMainElementBase {
        * Dictionary defining page availability.
        */
       pageAvailability: Object,
-
-      showArcvmManageUsb: Boolean,
-
     };
   }
 
   prefs: Object;
   advancedToggleExpanded: boolean;
   toolbarSpinnerActive: boolean;
   pageAvailability: OsPageAvailability;
-  showArcvmManageUsb: boolean;
   private overscroll_: number;
   private showPages_: MainPageVisibility;
   private showingSubpage_: boolean;

diff --git a/chrome/browser/resources/settings/chromeos/os_settings_page/os_settings_page.html b/chrome/browser/resources/settings/chromeos/os_settings_page/os_settings_page.html
@@ -207,8 +207,7 @@
       <os-settings-section page-title="$i18n{appsPageTitle}" section="apps">
         <os-settings-apps-page
             prefs="{{prefs}}"
-            android-apps-info="[[androidAppsInfo]]"
-            show-arcvm-manage-usb="[[showArcvmManageUsb]]">
+            android-apps-info="[[androidAppsInfo]]">
         </os-settings-apps-page>
       </os-settings-section>
     </template>

diff --git a/chrome/browser/resources/settings/chromeos/os_settings_page/os_settings_page.ts b/chrome/browser/resources/settings/chromeos/os_settings_page/os_settings_page.ts
@@ -71,8 +71,6 @@ export class OsSettingsPageElement extends OsSettingsPageElementBase {
         notify: true,
       },
 
-      showArcvmManageUsb: Boolean,
-
       androidAppsInfo: Object,
 
       /**

diff --git a/chrome/browser/resources/settings/chromeos/os_settings_routes.ts b/chrome/browser/resources/settings/chromeos/os_settings_routes.ts
@@ -5,7 +5,7 @@
 import {assert} from 'chrome://resources/js/assert_ts.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
 
-import {androidAppsVisible, isCrostiniSupported, isGuest, isKerberosEnabled, isPluginVmAvailable, isPowerwashAllowed} from './common/load_time_booleans.js';
+import {androidAppsVisible, isArcVmEnabled, isCrostiniSupported, isGuest, isKerberosEnabled, isPluginVmAvailable, isPowerwashAllowed} from './common/load_time_booleans.js';
 import * as routesMojom from './mojom-webui/routes.mojom-webui.js';
 
 /** Class for navigable routes. */
@@ -378,8 +378,7 @@ function createOsSettingsRoutes(): OsSettingsRoutes {
     r.ANDROID_APPS_DETAILS = createSubpage(
         r.APPS, routesMojom.GOOGLE_PLAY_STORE_SUBPAGE_PATH,
         Subpage.kGooglePlayStore);
-    if (loadTimeData.valueExists('showArcvmManageUsb') &&
-        loadTimeData.getBoolean('showArcvmManageUsb')) {
+    if (isArcVmEnabled()) {
       r.ANDROID_APPS_DETAILS_ARC_VM_SHARED_USB_DEVICES = createSubpage(
           r.ANDROID_APPS_DETAILS,
           routesMojom.ARC_VM_USB_PREFERENCES_SUBPAGE_PATH,

diff --git a/chrome/browser/resources/settings/chromeos/os_settings_ui/os_settings_ui.html b/chrome/browser/resources/settings/chromeos/os_settings_ui/os_settings_ui.html
@@ -141,7 +141,6 @@
       prefs="{{prefs}}"
       toolbar-spinner-active="{{toolbarSpinnerActive_}}"
       page-availability="[[pageAvailability_]]"
-      show-arcvm-manage-usb="[[showArcvmManageUsb_]]"
       advanced-toggle-expanded="{{advancedOpenedInMain_}}">
   </os-settings-main>
   <!-- An additional child of the flex #container to take up space,

diff --git a/chrome/browser/resources/settings/chromeos/os_settings_ui/os_settings_ui.ts b/chrome/browser/resources/settings/chromeos/os_settings_ui/os_settings_ui.ts
@@ -140,8 +140,6 @@ export class OsSettingsUiElement extends OsSettingsUiElementBase {
         },
       },
 
-      showArcvmManageUsb_: Boolean,
-
       showToolbar_: Boolean,
 
       showNavMenu_: Boolean,
@@ -163,7 +161,6 @@ export class OsSettingsUiElement extends OsSettingsUiElementBase {
   private advancedOpenedInMenu_: boolean;
   private toolbarSpinnerActive_: boolean;
   private pageAvailability_: OsPageAvailability;
-  private showArcvmManageUsb_: boolean;
   private showToolbar_: boolean;
   private showNavMenu_: boolean;
   private narrowThreshold_: number;
@@ -217,7 +214,6 @@ export class OsSettingsUiElement extends OsSettingsUiElementBase {
           loadTimeData.getString('controlledSettingChildRestriction'),
     };
 
-    this.showArcvmManageUsb_ = loadTimeData.getBoolean('showArcvmManageUsb');
     this.showNavMenu_ = !loadTimeData.getBoolean('isKioskModeActive');
     this.showToolbar_ = !loadTimeData.getBoolean('isKioskModeActive');
 

diff --git a/chrome/browser/ui/webui/settings/ash/apps_section.cc b/chrome/browser/ui/webui/settings/ash/apps_section.cc
@@ -453,7 +453,7 @@ void AppsSection::AddLoadTimeData(content::WebUIDataSource* html_source) {
   html_source->AddBoolean(
       "showOsSettingsAppBadgingToggle",
       base::FeatureList::IsEnabled(features::kOsSettingsAppBadgingToggle));
-  html_source->AddBoolean("showArcvmManageUsb", arc::IsArcVmEnabled());
+  html_source->AddBoolean("isArcVmEnabled", arc::IsArcVmEnabled());
 
   AddAppManagementStrings(html_source);
   AddGuestOsStrings(html_source);

diff --git a/chrome/test/data/webui/settings/chromeos/apps_page_test.js b/chrome/test/data/webui/settings/chromeos/apps_page_test.js
@@ -594,13 +594,13 @@ suite('AppsPageTests', function() {
 
     test('ManageUsbDevice', function() {
       // ARCVM is not enabled
-      subpage.showArcvmManageUsb = false;
+      subpage.isArcVmManageUsbAvailable = false;
       flush();
       assertFalse(
           !!subpage.shadowRoot.querySelector('#manageArcvmShareUsbDevices'));
 
       // ARCMV is enabled
-      subpage.showArcvmManageUsb = true;
+      subpage.isArcVmManageUsbAvailable = true;
       flush();
       assertTrue(
           !!subpage.shadowRoot.querySelector('#manageArcvmShareUsbDevices'));