Extensions: modernize kiosk app handler code.

To allow new WebUI to use the KioskAppsHandler code, this CL converts all
the CallJavascriptUnsafe calls to either fire a WebUI event, or resolve
a specific Javascript promise.

Bug: 764943
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I04e0f81220251cfd3eaab3100b256f13b7f34430
Reviewed-on: https://chromium-review.googlesource.com/676092
Commit-Queue: Scott Chen <scottchen@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504746}

chrome/browser/resources/extensions/chromeos/kiosk_apps.js

@@ -19,11 +19,18 @@ cr.define('extensions', function() {
      */
     clearErrorTimer_: null,
 
+    /** @private {WebUIListenerTracker} */
+    listenerTracker_: null,
+
     /**
      * Initialize the page.
      */
     initialize: function() {
-      chrome.send('initializeKioskAppSettings');
+      this.listenerTracker_ = new WebUIListenerTracker();
+
+      cr.sendWithPromise('initializeKioskAppSettings')
+          .then(KioskAppsOverlay.enableKiosk);
+
       extensions.KioskAppList.decorate($('kiosk-app-list'));
 
       var overlay = $('kiosk-apps-page');
@@ -42,7 +49,14 @@ cr.define('extensions', function() {
      * Invoked when the page is shown.
      */
     didShowPage: function() {
-      chrome.send('getKioskAppSettings');
+      this.listenerTracker_.add(
+          'kiosk-app-settings-changed', KioskAppsOverlay.setSettings);
+      this.listenerTracker_.add(
+          'kiosk-app-updated', KioskAppsOverlay.updateApp);
+      this.listenerTracker_.add('kiosk-app-error', KioskAppsOverlay.showError);
+
+      cr.sendWithPromise('getKioskAppSettings')
+          .then(KioskAppsOverlay.setSettings);
       $('kiosk-app-id-edit').focus();
     },
 
@@ -95,6 +109,7 @@ cr.define('extensions', function() {
     handleDismiss_: function() {
       this.handleAddButtonClick_();
       extensions.ExtensionSettings.showOverlay(null);
+      this.listenerTracker_.removeAll();
     }
   };
 

chrome/browser/resources/extensions/compiled_resources2.gyp

@@ -22,6 +22,9 @@
     },
     {
       'target_name': 'extensions',
+      'dependencies': [
+        '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:webui_listener_tracker',
+      ],
       'variables': {
         'script_args': ['--custom_sources'],
         'source_files': [

chrome/browser/resources/extensions/extensions.html

@@ -48,6 +48,8 @@
 </script>
 <script src="chrome://resources/js/cr/ui/list_item.js"></script>
 <script src="chrome://resources/js/cr/ui/list.js"></script>
+<script src="chrome://resources/js/promise_resolver.js"></script>
+<script src="chrome://resources/js/webui_listener_tracker.js"></script>
 </if>
 
 <script src="chrome://extensions/extensions.js"></script>

chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_browsertest.js

@@ -60,8 +60,8 @@ KioskAppSettingsWebUITest.prototype = {
          'enableKioskAutoLaunch',
          'disableKioskAutoLaunch'
         ]);
-    this.mockHandler.stubs().getKioskAppSettings().
-        will(callFunction(function() {
+    this.mockHandler.stubs().getKioskAppSettings(ANYTHING).will(
+        callFunction(function() {
           extensions.KioskAppsOverlay.setSettings(this.settings_);
         }.bind(this)));
     this.mockHandler.stubs().addKioskApp(ANYTHING);

chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_handler.cc

@@ -111,11 +111,20 @@ KioskAppsHandler::KioskAppsHandler(OwnerSettingsServiceChromeOS* service)
       is_kiosk_enabled_(false),
       is_auto_launch_enabled_(false),
       owner_settings_service_(service),
-      weak_ptr_factory_(this) {
+      weak_ptr_factory_(this) {}
+
+KioskAppsHandler::~KioskAppsHandler() {
+  // TODO(scottchen): This is needed because OnJavascriptDisallowed only called
+  // with refresh or off-page navigation, otherwise DCHECK triggered when
+  // exiting. Ask tommycli for more information.
+  kiosk_app_manager_->RemoveObserver(this);
+}
+
+void KioskAppsHandler::OnJavascriptAllowed() {
   kiosk_app_manager_->AddObserver(this);
 }
 
-KioskAppsHandler::~KioskAppsHandler() {
+void KioskAppsHandler::OnJavascriptDisallowed() {
   kiosk_app_manager_->RemoveObserver(this);
 }
 
@@ -208,6 +217,7 @@ void KioskAppsHandler::OnKioskExtensionDownloadFailed(
 }
 
 void KioskAppsHandler::OnGetConsumerKioskAutoLaunchStatus(
+    const std::string& callback_id,
     chromeos::KioskAppManager::ConsumerKioskAutoLaunchStatus status) {
   initialized_ = true;
   if (KioskAppManager::IsConsumerKioskEnabled()) {
@@ -228,23 +238,24 @@ void KioskAppsHandler::OnGetConsumerKioskAutoLaunchStatus(
     is_auto_launch_enabled_ = false;
   }
 
-  if (is_kiosk_enabled_) {
-    base::DictionaryValue kiosk_params;
-    kiosk_params.SetBoolean("kioskEnabled", is_kiosk_enabled_);
-    kiosk_params.SetBoolean("autoLaunchEnabled", is_auto_launch_enabled_);
-    web_ui()->CallJavascriptFunctionUnsafe(
-        "extensions.KioskAppsOverlay.enableKiosk", kiosk_params);
-  }
+  base::DictionaryValue kiosk_params;
+  kiosk_params.SetBoolean("kioskEnabled", is_kiosk_enabled_);
+  kiosk_params.SetBoolean("autoLaunchEnabled", is_auto_launch_enabled_);
+  ResolveJavascriptCallback(base::Value(callback_id), kiosk_params);
 }
 
 
 void KioskAppsHandler::OnKioskAppsSettingsChanged() {
-  SendKioskAppSettings();
+  FireWebUIListener("kiosk-app-settings-changed", *GetSettingsDictionary());
 }
 
-void KioskAppsHandler::SendKioskAppSettings() {
-  if (!initialized_ || !is_kiosk_enabled_)
-    return;
+std::unique_ptr<base::DictionaryValue>
+KioskAppsHandler::GetSettingsDictionary() {
+  std::unique_ptr<base::DictionaryValue> settings(new base::DictionaryValue);
+
+  if (!initialized_ || !is_kiosk_enabled_) {
+    return settings;
+  }
 
   bool enable_bailout_shortcut;
   if (!CrosSettings::Get()->GetBoolean(
@@ -253,10 +264,9 @@ void KioskAppsHandler::SendKioskAppSettings() {
     enable_bailout_shortcut = true;
   }
 
-  base::DictionaryValue settings;
-  settings.SetBoolean("disableBailout", !enable_bailout_shortcut);
-  settings.SetBoolean("hasAutoLaunchApp",
-                      !kiosk_app_manager_->GetAutoLaunchApp().empty());
+  settings->SetBoolean("disableBailout", !enable_bailout_shortcut);
+  settings->SetBoolean("hasAutoLaunchApp",
+                       !kiosk_app_manager_->GetAutoLaunchApp().empty());
 
   KioskAppManager::Apps apps;
   kiosk_app_manager_->GetApps(&apps);
@@ -269,21 +279,29 @@ void KioskAppsHandler::SendKioskAppSettings() {
     PopulateAppDict(app_data, app_info.get());
     apps_list->Append(std::move(app_info));
   }
-  settings.SetWithoutPathExpansion("apps", std::move(apps_list));
+  settings->SetWithoutPathExpansion("apps", std::move(apps_list));
 
-  web_ui()->CallJavascriptFunctionUnsafe(
-      "extensions.KioskAppsOverlay.setSettings", settings);
+  return settings;
 }
 
 void KioskAppsHandler::HandleInitializeKioskAppSettings(
     const base::ListValue* args) {
+  CHECK_EQ(1U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
+
+  AllowJavascript();
   KioskAppManager::Get()->GetConsumerKioskAutoLaunchStatus(
       base::Bind(&KioskAppsHandler::OnGetConsumerKioskAutoLaunchStatus,
-                 weak_ptr_factory_.GetWeakPtr()));
+                 weak_ptr_factory_.GetWeakPtr(), callback_id));
 }
 
 void KioskAppsHandler::HandleGetKioskAppSettings(const base::ListValue* args) {
-  SendKioskAppSettings();
+  CHECK_EQ(1U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
+
+  ResolveJavascriptCallback(base::Value(callback_id), *GetSettingsDictionary());
 }
 
 
@@ -359,15 +377,12 @@ void KioskAppsHandler::UpdateApp(const std::string& app_id) {
 
   base::DictionaryValue app_dict;
   PopulateAppDict(app_data, &app_dict);
-
-  web_ui()->CallJavascriptFunctionUnsafe(
-      "extensions.KioskAppsOverlay.updateApp", app_dict);
+  FireWebUIListener("kiosk-app-updated", app_dict);
 }
 
 void KioskAppsHandler::ShowError(const std::string& app_id) {
   base::Value app_id_value(app_id);
-  web_ui()->CallJavascriptFunctionUnsafe(
-      "extensions.KioskAppsOverlay.showError", app_id_value);
+  FireWebUIListener("kiosk-app-error", app_id_value);
 
   kiosk_app_manager_->RemoveApp(app_id, owner_settings_service_);
 }

chrome/browser/ui/webui/extensions/chromeos/kiosk_apps_handler.h

@@ -37,6 +37,8 @@ class KioskAppsHandler : public content::WebUIMessageHandler,
 
   // content::WebUIMessageHandler overrides:
   void RegisterMessages() override;
+  void OnJavascriptAllowed() override;
+  void OnJavascriptDisallowed() override;
 
   // KioskAppManagerObserver overrides:
   void OnKioskAppDataChanged(const std::string& app_id) override;
@@ -47,8 +49,8 @@ class KioskAppsHandler : public content::WebUIMessageHandler,
   void OnKioskAppsSettingsChanged() override;
 
  private:
-  // Sends all kiosk apps and settings to webui.
-  void SendKioskAppSettings();
+  // Get all kiosk apps and settings.
+  std::unique_ptr<base::DictionaryValue> GetSettingsDictionary();
 
   // JS callbacks.
   void HandleInitializeKioskAppSettings(const base::ListValue* args);
@@ -64,6 +66,7 @@ class KioskAppsHandler : public content::WebUIMessageHandler,
 
   // Callback for KioskAppManager::GetConsumerKioskModeStatus().
   void OnGetConsumerKioskAutoLaunchStatus(
+      const std::string& callback_id,
       chromeos::KioskAppManager::ConsumerKioskAutoLaunchStatus status);
 
   KioskAppManager* kiosk_app_manager_;  // not owned.