plugin_vm: settings pages respond to uninstallation

Open Plugin VM settings pages should revert back to the top level os
settings when Plugin VM is uninstalled.

In order accomplish this the top level settings cr-link-row needs to be
always visible, as os_settings can't dynamically hide rows. Instead the
contents of the row have become dynamic, and will either say Turn On if
Plugin VM is not yet installed, or show an arrow if it is installed. The
resulting UI is now the same as Google Play Store and Linux (Beta).

Bug: 1023256
Change-Id: I9ade77a4dff3e7799732608430f299b400338ec8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2007803Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Julian Watson <juwa@google.com>
Cr-Commit-Position: refs/heads/master@{#734321}

chrome/browser/extensions/api/settings_private/prefs_util.cc

@@ -49,6 +49,7 @@
 #include "chrome/browser/chromeos/guest_os/guest_os_pref_names.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
+#include "chrome/browser/chromeos/plugin_vm/plugin_vm_pref_names.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
@@ -445,6 +446,10 @@ const PrefsUtil::TypedPrefMap& PrefsUtil::GetWhitelistedKeys() {
   (*s_whitelist)[guest_os::prefs::kGuestOSPathsSharedToVms] =
       settings_api::PrefType::PREF_TYPE_DICTIONARY;
 
+  // Plugin Vm
+  (*s_whitelist)[plugin_vm::prefs::kPluginVmImageExists] =
+      settings_api::PrefType::PREF_TYPE_BOOLEAN;
+
   // Android Apps.
   (*s_whitelist)[arc::prefs::kArcEnabled] =
       settings_api::PrefType::PREF_TYPE_BOOLEAN;

chrome/browser/resources/settings/chromeos/plugin_vm_page/BUILD.gn

@@ -22,6 +22,7 @@ js_library("plugin_vm_page") {
   deps = [
     "..:os_route",
     "../..:router",
+    "../../prefs:prefs_behavior",
     "//ui/webui/resources/js:i18n_behavior",
   ]
   externs_list = [ "$externs_path/settings_private.js" ]
@@ -38,6 +39,7 @@ js_library("plugin_vm_subpage") {
   deps = [
     "..:os_route",
     "../..:router",
+    "../../prefs:prefs_behavior",
   ]
 }
 

chrome/browser/resources/settings/chromeos/plugin_vm_page/plugin_vm_browser_proxy.js

@@ -23,6 +23,9 @@ cr.define('settings', function() {
 
     /* Removes the default vm if it is installed. */
     removePluginVm() {}
+
+    /* Show Plugin Vm installer. */
+    requestPluginVmInstallerView() {}
   }
 
   /** @implements {settings.PluginVmBrowserProxy} */
@@ -41,6 +44,11 @@ cr.define('settings', function() {
     removePluginVm() {
       chrome.send('removePluginVm');
     }
+
+    /** @override */
+    requestPluginVmInstallerView() {
+      chrome.send('requestPluginVmInstallerView');
+    }
   }
 
   // The singleton instance_ can be replaced with a test version of this wrapper

chrome/browser/resources/settings/chromeos/plugin_vm_page/plugin_vm_page.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
-<link rel="import" href="chrome://resources/cr_elements/cr_link_row/cr_link_row.html">
+<link rel="import" href="chrome://resources/cr_elements/cr_button/cr_button.html">
+<link rel="import" href="chrome://resources/cr_elements/cr_icon_button/cr_icon_button.html">
 <link rel="import" href="chrome://resources/html/i18n_behavior.html">
 <link rel="import" href="../../i18n_setup.html">
+<link rel="import" href="../../prefs/prefs_behavior.html">
 <link rel="import" href="../os_route.html">
 <link rel="import" href="../../router.html">
 <link rel="import" href="../../settings_page/settings_animated_pages.html">
 <link rel="import" href="../../settings_page/settings_subpage.html">
 <link rel="import" href="../../settings_shared_css.html">
+<link rel="import" href="plugin_vm_browser_proxy.html">
 <link rel="import" href="plugin_vm_shared_paths.html">
 <link rel="import" href="plugin_vm_subpage.html">
 
@@ -18,11 +21,30 @@
     <settings-animated-pages id="pages" section="pluginVm"
         focus-config="[[focusConfig_]]">
       <div route-path="default">
-        <cr-link-row id="plugin-vm"
-            label="$i18n{pluginVmPageLabel}"
-            sub-label="$i18n{pluginVmPageSubtext}"
-            on-click="onSubpageClick_"
-            role-description="$i18n{subpageArrowRoleDescription}"></cr-link-row>
+        <div id="pluginVm" class="settings-box two-line first"
+            on-click="onSubpageClick_">
+          <div class="start">
+            $i18n{pluginVmPageLabel}
+            <div class="secondary" id="secondaryText">
+              $i18n{pluginVmPageSubtext}
+            </div>
+          </div>
+          <template is="dom-if" if="[[prefs.plugin_vm.image_exists.value]]">
+            <cr-icon-button class="subpage-arrow"
+                aria-label="i18n{pluginVmPageLabel}"
+                aria-describedby="secondaryText"
+                aria-roledescription="$i18n{subpageArrowRoleDescription}">
+            </cr-icon-button>
+          </template>
+          <template is="dom-if" if="[[!prefs.plugin_vm.image_exists.value]]">
+            <div class="separator"></div>
+            <cr-button id="enable"
+                on-click="onEnableClick_"
+                aria-describedby="secondaryText">
+              $i18n{pluginVmPageEnable}
+            </cr-button>
+          </template>
+        </div>
       </div>
 
       <template is="dom-if" route-path="/pluginVm/details">

chrome/browser/resources/settings/chromeos/plugin_vm_page/plugin_vm_page.js

@@ -10,6 +10,8 @@
 Polymer({
   is: 'settings-plugin-vm-page',
 
+  behaviors: [PrefsBehavior],
+
   properties: {
     /** Preferences state. */
     prefs: {
@@ -33,8 +35,24 @@ Polymer({
     },
   },
 
-  /** @private */
+  /**
+   * @param {!Event} event
+   * @private
+   */
   onSubpageClick_(event) {
-    settings.Router.getInstance().navigateTo(settings.routes.PLUGIN_VM_DETAILS);
+    if (this.getPref('plugin_vm.image_exists').value) {
+      settings.Router.getInstance().navigateTo(
+          settings.routes.PLUGIN_VM_DETAILS);
+    }
+  },
+
+  /**
+   * @param {!Event} event
+   * @private
+   */
+  onEnableClick_(event) {
+    settings.PluginVmBrowserProxyImpl.getInstance()
+        .requestPluginVmInstallerView();
+    event.stopPropagation();
   },
 });

chrome/browser/resources/settings/chromeos/plugin_vm_page/plugin_vm_subpage.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
 <link rel="import" href="chrome://resources/cr_elements/cr_icon_button/cr_icon_button.html">
+<link rel="import" href="../../prefs/prefs_behavior.html">
 <link rel="import" href="../os_route.html">
 <link rel="import" href="../../router.html">
 <link rel="import" href="plugin_vm_remove_confirmation_dialog.html">

chrome/browser/resources/settings/chromeos/plugin_vm_page/plugin_vm_subpage.js

@@ -10,6 +10,8 @@
 Polymer({
   is: 'settings-plugin-vm-subpage',
 
+  behaviors: [PrefsBehavior],
+
   properties: {
     /** Preferences state. */
     prefs: {
@@ -24,7 +26,19 @@ Polymer({
     },
   },
 
-  // TODO(juwa@google.com): Navigate back if plugin vm uninstalled.
+  observers: [
+    'onPluginVmImageExistsChanged_(prefs.plugin_vm.image_exists.value)',
+  ],
+
+  /**
+   * @param {boolean} exists
+   * @private
+   */
+  onPluginVmImageExistsChanged_(exists) {
+    if (!exists) {
+      settings.Router.getInstance().navigateTo(settings.routes.PLUGIN_VM);
+    }
+  },
 
   /** @private */
   onSharedPathsClick_() {

chrome/browser/ui/webui/settings/chromeos/os_settings_ui.cc

@@ -296,7 +296,7 @@ void OSSettingsUI::InitOSWebUIHandlers(content::WebUIDataSource* html_source) {
   web_ui()->AddMessageHandler(
       std::make_unique<chromeos::settings::WallpaperHandler>(web_ui()));
 
-  if (plugin_vm::IsPluginVmEnabled(profile)) {
+  if (plugin_vm::IsPluginVmAllowedForProfile(profile)) {
     web_ui()->AddMessageHandler(
         std::make_unique<chromeos::settings::PluginVmHandler>(profile));
   }
@@ -387,7 +387,7 @@ void OSSettingsUI::InitOSWebUIHandlers(content::WebUIDataSource* html_source) {
       "allowCrostini", crostini::CrostiniFeatures::Get()->IsUIAllowed(profile));
 
   html_source->AddBoolean("showPluginVm",
-                          plugin_vm::IsPluginVmEnabled(profile));
+                          plugin_vm::IsPluginVmAllowedForProfile(profile));
 
   html_source->AddBoolean("isDemoSession",
                           chromeos::DemoSession::IsDeviceInDemoMode());

chrome/browser/ui/webui/settings/chromeos/plugin_vm_handler.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/guest_os/guest_os_share_path.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager.h"
+#include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -37,6 +38,10 @@ void PluginVmHandler::RegisterMessages() {
       "removePluginVm",
       base::BindRepeating(&PluginVmHandler::HandleRemovePluginVm,
                           weak_ptr_factory_.GetWeakPtr()));
+  web_ui()->RegisterMessageCallback(
+      "requestPluginVmInstallerView",
+      base::BindRepeating(&PluginVmHandler::HandleRequestPluginVmInstallerView,
+                          weak_ptr_factory_.GetWeakPtr()));
 }
 
 void PluginVmHandler::HandleGetPluginVmSharedPathsDisplayText(
@@ -84,5 +89,17 @@ void PluginVmHandler::HandleRemovePluginVm(const base::ListValue* args) {
   manager->UninstallPluginVm();
 }
 
+void PluginVmHandler::HandleRequestPluginVmInstallerView(
+    const base::ListValue* args) {
+  CHECK(args->empty());
+
+  if (plugin_vm::IsPluginVmEnabled(profile_)) {
+    LOG(ERROR) << "requestPluginVmInstallerView called from a profile which "
+                  "already has Plugin VM installed.";
+    return;
+  }
+  plugin_vm::ShowPluginVmInstallerView(profile_);
+}
+
 }  // namespace settings
 }  // namespace chromeos

chrome/browser/ui/webui/settings/chromeos/plugin_vm_handler.h

@@ -34,6 +34,8 @@ class PluginVmHandler : public ::settings::SettingsPageUIHandler {
   void HandleRemovePluginVmSharedPath(const base::ListValue* args);
   // Remove Plugin VM.
   void HandleRemovePluginVm(const base::ListValue* args);
+  // Show the Plugin VM installer view if Plugin VM is not currently installed.
+  void HandleRequestPluginVmInstallerView(const base::ListValue* args);
 
   Profile* profile_;
   // weak_ptr_factory_ should always be last member.

chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc

@@ -444,6 +444,7 @@ void AddPluginVmStrings(content::WebUIDataSource* html_source,
       {"pluginVmPageTitle", IDS_SETTINGS_PLUGIN_VM_PAGE_TITLE},
       {"pluginVmPageLabel", IDS_SETTINGS_PLUGIN_VM_PAGE_LABEL},
       {"pluginVmPageSubtext", IDS_SETTINGS_PLUGIN_VM_PAGE_SUBTEXT},
+      {"pluginVmPageEnable", IDS_SETTINGS_TURN_ON},
       {"pluginVmPrinterAccess", IDS_SETTINGS_PLUGIN_VM_PRINTER_ACCESS},
       {"pluginVmSharedPaths", IDS_SETTINGS_PLUGIN_VM_SHARED_PATHS},
       {"pluginVmSharedPathsListHeading",

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

@@ -9,6 +9,7 @@ class TestPluginVmBrowserProxy extends TestBrowserProxy {
       'getPluginVmSharedPathsDisplayText',
       'removePluginVmSharedPath',
       'removePluginVm',
+      'requestPluginVmInstallerView',
     ]);
   }
 
@@ -27,11 +28,87 @@ class TestPluginVmBrowserProxy extends TestBrowserProxy {
   removePluginVm() {
     this.methodCalled('removePluginVm');
   }
+
+  /** override */
+  requestPluginVmInstallerView() {
+    this.methodCalled('requestPluginVmInstallerView');
+  }
 }
 
 /** @type {?TestPluginVmBrowserProxy} */
 let pluginVmBrowserProxy = null;
 
+suite('PluginVmPage', function() {
+  /** @type {?SettingsPluginVmElement} */
+  let page = null;
+
+  /** @type {Array<string>} */
+  let routes;
+  /** @type {!Object} */
+  let preTestSettingsRoutes;
+  /** @type {!function(string)} */
+  let preTestRouterNavigateTo;
+
+  setup(function() {
+    pluginVmBrowserProxy = new TestPluginVmBrowserProxy();
+    settings.PluginVmBrowserProxyImpl.instance_ = pluginVmBrowserProxy;
+    PolymerTest.clearBody();
+    page = document.createElement('settings-plugin-vm-page');
+
+    routes = [];
+
+    preTestRouterNavigateTo = settings.Router.getInstance().navigateTo;
+    settings.Router.getInstance().navigateTo = (route) => routes.push(route);
+
+    preTestSettingsRoutes = settings.routes;
+    settings.routes = {PLUGIN_VM_DETAILS: 'TEST_PLUGIN_VM_DETAILS_ROUTE'};
+  });
+
+  teardown(function() {
+    page.remove();
+
+    settings.Router.getInstance().navigateTo = preTestRouterNavigateTo;
+
+    settings.routes = preTestSettingsRoutes;
+  });
+
+  test('ImageExistsLink', function() {
+    page.prefs = {
+      plugin_vm: {
+        image_exists: {value: true},
+      }
+    };
+    document.body.appendChild(page);
+    Polymer.dom.flush();
+
+    const button = page.$$('cr-icon-button');
+    assertTrue(!!button);
+
+    assertDeepEquals(routes, []);
+    button.click();
+    assertDeepEquals(routes, ['TEST_PLUGIN_VM_DETAILS_ROUTE']);
+  });
+
+  test('ImageDoesntExist', function() {
+    page.prefs = {
+      plugin_vm: {
+        image_exists: {value: false},
+      }
+    };
+    document.body.appendChild(page);
+    Polymer.dom.flush();
+
+    const button = page.$$('cr-button');
+    assertTrue(!!button);
+
+    assertEquals(
+        0, pluginVmBrowserProxy.getCallCount('requestPluginVmInstallerView'));
+    button.click();
+    assertEquals(
+        1, pluginVmBrowserProxy.getCallCount('requestPluginVmInstallerView'));
+  });
+});
+
 suite('Details', function() {
   /** @type {?SettingsPluginVmSubpageElement} */
   let page = null;