MD Settings: About page, porting C++ handler and adding browser proxy.

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

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2016 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.
 
-#include "chrome/browser/ui/webui/help/help_handler.h"
+#include "chrome/browser/ui/webui/settings/about_handler.h"
 
 #include <stddef.h>
 
 #include <string>
 
 #include "ash/system/chromeos/devicetype_utils.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ skipping 20 matching lines @@
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/policy/core/common/policy_namespace.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
+#include "content/public/browser/web_ui_data_source.h"
 #include "content/public/common/user_agent.h"
 #include "grit/components_chromium_strings.h"
 #include "grit/components_google_chrome_strings.h"
 #include "grit/components_strings.h"
 #include "grit/generated_resources.h"
 #include "policy/policy_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "v8/include/v8.h"
 
 #if defined(OS_CHROMEOS)
 #include "base/files/file_util_proxy.h"
 #include "base/i18n/time_formatting.h"
 #include "base/sys_info.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/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/chromeos/image_source.h"
 #include "chrome/browser/ui/webui/help/help_utils_chromeos.h"
 #include "chrome/browser/ui/webui/help/version_updater_chromeos.h"
 #include "chromeos/chromeos_switches.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager_client.h"
 #include "chromeos/system/statistics_provider.h"
-#include "components/prefs/pref_service.h"
 #include "components/user_manager/user_manager.h"
 #endif
 
 using base::ListValue;
 using content::BrowserThread;
 
 namespace {
 
 #if defined(OS_CHROMEOS)
 
@@ skipping 115 matching lines @@
   text_path = text_path.AppendASCII(kRegulatoryLabelTextFilename);
 
   std::string contents;
   if (base::ReadFileToString(text_path, &contents))
     return contents;
   return std::string();
 }
 
 #endif  // defined(OS_CHROMEOS)
 
+std::string UpdateStatusToString(VersionUpdater::Status status) {
+  std::string status_str;
+  switch (status) {
+  case VersionUpdater::CHECKING:
+    status_str = "checking";
+    break;
+  case VersionUpdater::UPDATING:
+    status_str = "updating";
+    break;
+  case VersionUpdater::NEARLY_UPDATED:
+    status_str = "nearly_updated";
+    break;
+  case VersionUpdater::UPDATED:
+    status_str = "updated";
+    break;
+  case VersionUpdater::FAILED:
+  case VersionUpdater::FAILED_OFFLINE:
+  case VersionUpdater::FAILED_CONNECTION_TYPE_DISALLOWED:
+    status_str = "failed";
+    break;
+  case VersionUpdater::DISABLED:
+    status_str = "disabled";
+    break;
+  case VersionUpdater::DISABLED_BY_ADMIN:
+    status_str = "disabled_by_admin";
+    break;
+  }
+
+  return status_str;
+}
+
 }  // namespace
 
-HelpHandler::HelpHandler()
+namespace settings {
+
+AboutHandler::AboutHandler()
     : policy_registrar_(
         g_browser_process->policy_service(),
         policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, std::string())),
       weak_factory_(this) {
 }
 
-HelpHandler::~HelpHandler() {
-}
+AboutHandler::~AboutHandler() {}
 
-void HelpHandler::GetLocalizedValues(base::DictionaryValue* localized_strings) {
-  struct L10nResources {
-    const char* name;
-    int ids;
-  };
-
-  static L10nResources resources[] = {
-    {"aboutTitle", IDS_ABOUT_TITLE},
-#if defined(OS_CHROMEOS)
-    {"aboutProductTitle", IDS_PRODUCT_OS_NAME},
-#else
-    {"aboutProductTitle", IDS_PRODUCT_NAME},
-#endif
-    {"aboutProductDescription", IDS_ABOUT_PRODUCT_DESCRIPTION},
-    {"relaunch", IDS_RELAUNCH_BUTTON},
-#if defined(OS_CHROMEOS)
-    {"relaunchAndPowerwash", IDS_RELAUNCH_AND_POWERWASH_BUTTON},
-#endif
-    {"productName", IDS_PRODUCT_NAME},
-    {"updateCheckStarted", IDS_UPGRADE_CHECK_STARTED},
-    {"updating", IDS_UPGRADE_UPDATING},
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
-    {"updateDisabledByPolicy", IDS_UPGRADE_DISABLED_BY_POLICY},
-#endif  // defined(OS_CHROMEOS) || defined(OS_WIN)
-#if defined(OS_CHROMEOS)
-    {"updateButton", IDS_UPGRADE_BUTTON},
-    {"updatingChannelSwitch", IDS_UPGRADE_UPDATING_CHANNEL_SWITCH},
-#endif
-    {"updateAlmostDone", IDS_UPGRADE_SUCCESSFUL_RELAUNCH},
-#if defined(OS_CHROMEOS)
-    {"successfulChannelSwitch", IDS_UPGRADE_SUCCESSFUL_CHANNEL_SWITCH},
-#endif
-    {"getHelpWithChrome", IDS_GET_HELP_USING_CHROME},
-    {"reportAnIssue", IDS_REPORT_AN_ISSUE},
-#if defined(OS_CHROMEOS)
-    {"platform", IDS_PLATFORM_LABEL},
-    {"arcVersion", IDS_ARC_VERSION_LABEL},
-    {"firmware", IDS_ABOUT_PAGE_FIRMWARE},
-    {"showMoreInfo", IDS_SHOW_MORE_INFO},
-    {"hideMoreInfo", IDS_HIDE_MORE_INFO},
-    {"channel", IDS_ABOUT_PAGE_CHANNEL},
-    {"stable", IDS_ABOUT_PAGE_CHANNEL_STABLE},
-    {"beta", IDS_ABOUT_PAGE_CHANNEL_BETA},
-    {"dev", IDS_ABOUT_PAGE_CHANNEL_DEVELOPMENT},
-    {"channel-changed", IDS_ABOUT_PAGE_CHANNEL_CHANGED},
-    {"currentChannelStable", IDS_ABOUT_PAGE_CURRENT_CHANNEL_STABLE},
-    {"currentChannelBeta", IDS_ABOUT_PAGE_CURRENT_CHANNEL_BETA},
-    {"currentChannelDev", IDS_ABOUT_PAGE_CURRENT_CHANNEL_DEV},
-    {"currentChannel", IDS_ABOUT_PAGE_CURRENT_CHANNEL},
-    {"channelChangeButton", IDS_ABOUT_PAGE_CHANNEL_CHANGE_BUTTON},
-    {"channelChangeDisallowedMessage",
-     IDS_ABOUT_PAGE_CHANNEL_CHANGE_DISALLOWED_MESSAGE},
-    {"channelChangePageTitle", IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_TITLE},
-    {"channelChangePagePowerwashTitle",
-     IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_POWERWASH_TITLE},
-    {"channelChangePagePowerwashMessage",
-     IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_POWERWASH_MESSAGE},
-    {"channelChangePageDelayedChangeTitle",
-     IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_DELAYED_CHANGE_TITLE},
-    {"channelChangePageUnstableTitle",
-     IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_UNSTABLE_TITLE},
-    {"channelChangePagePowerwashButton",
-     IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_POWERWASH_BUTTON},
-    {"channelChangePageChangeButton",
-     IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_CHANGE_BUTTON},
-    {"channelChangePageCancelButton",
-     IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_CANCEL_BUTTON},
-    {"webkit", IDS_WEBKIT},
-    {"userAgent", IDS_VERSION_UI_USER_AGENT},
-    {"commandLine", IDS_VERSION_UI_COMMAND_LINE},
-    {"buildDate", IDS_VERSION_UI_BUILD_DATE},
-#endif
-#if defined(OS_MACOSX) || defined(OS_WIN)
-    {"learnMore", IDS_LEARN_MORE},
-#endif
-#if defined(OS_MACOSX)
-    {"promote", IDS_ABOUT_CHROME_PROMOTE_UPDATER},
-#endif
-  };
-
-  for (size_t i = 0; i < arraysize(resources); ++i) {
-    localized_strings->SetString(resources[i].name,
-                                 l10n_util::GetStringUTF16(resources[i].ids));
-  }
-
-#if defined(OS_CHROMEOS)
-  localized_strings->SetString("upToDate", ash::SubstituteChromeOSDeviceType(
-      IDS_UPGRADE_UP_TO_DATE));
-#else
-  localized_strings->SetString("upToDate", l10n_util::GetStringUTF16(
-      IDS_UPGRADE_UP_TO_DATE));
-#endif
-
-  localized_strings->SetString("updateObsoleteSystem",
-                               ObsoleteSystem::LocalizedObsoleteString());
-  localized_strings->SetString("updateObsoleteSystemURL",
-                               ObsoleteSystem::GetLinkURL());
-
-  localized_strings->SetString(
-      "browserVersion",
+AboutHandler* AboutHandler::Create(content::WebUIDataSource* html_source,
+                                   Profile* profile) {
+  html_source->AddString(
+      "aboutBrowserVersion",
       l10n_util::GetStringFUTF16(IDS_ABOUT_PRODUCT_VERSION,
                                  BuildBrowserVersionString()));
 
   base::Time::Exploded exploded_time;
   base::Time::Now().LocalExplode(&exploded_time);
-  localized_strings->SetString(
-      "productCopyright",
+  html_source->AddString(
+      "aboutProductCopyright",
        l10n_util::GetStringFUTF16(IDS_ABOUT_VERSION_COPYRIGHT,
                                   base::IntToString16(exploded_time.year)));
 
   base::string16 license = l10n_util::GetStringFUTF16(
       IDS_VERSION_UI_LICENSE, base::ASCIIToUTF16(chrome::kChromiumProjectURL),
       base::ASCIIToUTF16(chrome::kChromeUICreditsURL));
-  localized_strings->SetString("productLicense", license);
+  html_source->AddString("aboutProductLicense", license);
+
+  html_source->AddBoolean("aboutObsolteNowOrSoon",
+                          ObsoleteSystem::IsObsoleteNowOrSoon());
+  html_source->AddBoolean(
+      "aboutObsolteEndOfTheLine",
+      ObsoleteSystem::IsObsoleteNowOrSoon() &&
+          ObsoleteSystem::IsEndOfTheLine());
 
 #if defined(OS_CHROMEOS)
   base::string16 os_license = l10n_util::GetStringFUTF16(
       IDS_ABOUT_CROS_VERSION_LICENSE,
       base::ASCIIToUTF16(chrome::kChromeUIOSCreditsURL));
-  localized_strings->SetString("productOsLicense", os_license);
+  html_source->AddString("aboutProductOsLicense", os_license);
 
-  base::string16 product_name = l10n_util::GetStringUTF16(IDS_PRODUCT_OS_NAME);
-  localized_strings->SetString(
-      "channelChangePageDelayedChangeMessage",
-      l10n_util::GetStringFUTF16(
-          IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_DELAYED_CHANGE_MESSAGE,
-          product_name));
-  localized_strings->SetString(
-      "channelChangePageUnstableMessage",
-      l10n_util::GetStringFUTF16(
-          IDS_ABOUT_PAGE_CHANNEL_CHANGE_PAGE_UNSTABLE_MESSAGE,
-          product_name));
+  html_source->AddBoolean("aboutCanChangeChannel", CanChangeChannel(profile));
+  html_source->AddBoolean("aboutEnterpriseManaged", IsEnterpriseManaged());
 
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          chromeos::switches::kDisableNewChannelSwitcherUI)) {
-    localized_strings->SetBoolean("disableNewChannelSwitcherUI", true);
-  }
+  base::Time build_time = base::SysInfo::GetLsbReleaseTime();
+  base::string16 build_date = base::TimeFormatFriendlyDate(build_time);
+  html_source->AddString("aboutBuildDate", build_date);
 #endif
 
-  base::string16 tos = l10n_util::GetStringFUTF16(
-      IDS_ABOUT_TERMS_OF_SERVICE, base::UTF8ToUTF16(chrome::kChromeUITermsURL));
-  localized_strings->SetString("productTOS", tos);
-
-  localized_strings->SetString("webkitVersion", content::GetWebKitVersion());
-
-  localized_strings->SetString("jsEngine", "V8");
-  localized_strings->SetString("jsEngineVersion", v8::V8::GetVersion());
-
-  localized_strings->SetString("userAgentInfo", GetUserAgent());
-
-  base::CommandLine::StringType command_line =
-      base::CommandLine::ForCurrentProcess()->GetCommandLineString();
-  localized_strings->SetString("commandLineInfo", command_line);
+  return new AboutHandler();
 }
 
-void HelpHandler::RegisterMessages() {
+void AboutHandler::RegisterMessages() {
   version_updater_.reset(VersionUpdater::Create(web_ui()->GetWebContents()));
   registrar_.Add(this, chrome::NOTIFICATION_UPGRADE_RECOMMENDED,

[tommycli, 2016/05/10 20:45:40]

Can we put both registrar_ and policy_registrar_ notification registrations in
the OnJavascriptAllowed method? (And also unregister them within
OnJavascriptDisallowed)

You will need to call AllowJavascript() within the "initial" call that the page
makes to this handler.

[dpapad, 2016/05/10 22:03:02]

Done for registrar_.
policy_registrar_ does not seem to have a way to unobserve currently see
https://code.google.com/p/chromium/codesearch#chromium/src/components/policy/....
Added a TODO for adding such a method.

[tommycli, 2016/05/10 22:14:52]

Ah, yes, I've run into that problem too. Here was my solution:

See line 171 of
https://codereview.chromium.org/1961183002/diff/40001/chrome/browser/ui/webui...

[dpapad, 2016/05/10 23:49:52]

I see, that makes sense. I could follow the same solution, but the
IsJavascriptAllowed() check will be very far from the observer method (several
layers below in the call stack), which will be a bit odd. I prefer to just rely
on the check at
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...
for now, and in the meantime try to add an Unobserve method to the corresponding
registry. WDYT?

[tommycli, 2016/05/10 23:54:54]

SGTM. If you are OK with a browser crash, then relying on that CHECK would be
even better.

Maybe I should change my patch to just crash also then...

                  content::NotificationService::AllSources());
   policy_registrar_.Observe(
       policy::key::kDeviceAutoUpdateDisabled,
-      base::Bind(&HelpHandler::OnDeviceAutoUpdatePolicyChanged,
+      base::Bind(&AboutHandler::OnDeviceAutoUpdatePolicyChanged,
                  base::Unretained(this)));
 
-  web_ui()->RegisterMessageCallback("onPageLoaded",
-      base::Bind(&HelpHandler::OnPageLoaded, base::Unretained(this)));
+  web_ui()->RegisterMessageCallback("refreshUpdateStatus",
+      base::Bind(&AboutHandler::HandleRefreshUpdateStatus,
+                 base::Unretained(this)));
   web_ui()->RegisterMessageCallback("relaunchNow",
-      base::Bind(&HelpHandler::RelaunchNow, base::Unretained(this)));
+      base::Bind(&AboutHandler::HandleRelaunchNow, base::Unretained(this)));
   web_ui()->RegisterMessageCallback("openFeedbackDialog",
-      base::Bind(&HelpHandler::OpenFeedbackDialog, base::Unretained(this)));
+      base::Bind(&AboutHandler::HandleOpenFeedbackDialog,
+                 base::Unretained(this)));
   web_ui()->RegisterMessageCallback("openHelpPage",
-      base::Bind(&HelpHandler::OpenHelpPage, base::Unretained(this)));
+      base::Bind(&AboutHandler::HandleOpenHelpPage, base::Unretained(this)));
 #if defined(OS_CHROMEOS)
   web_ui()->RegisterMessageCallback("setChannel",
-      base::Bind(&HelpHandler::SetChannel, base::Unretained(this)));
-  web_ui()->RegisterMessageCallback("relaunchAndPowerwash",
-      base::Bind(&HelpHandler::RelaunchAndPowerwash, base::Unretained(this)));
+      base::Bind(&AboutHandler::SetChannel, base::Unretained(this)));
   web_ui()->RegisterMessageCallback("requestUpdate",
-      base::Bind(&HelpHandler::RequestUpdate, base::Unretained(this)));
+      base::Bind(&AboutHandler::HandleRequestUpdate, base::Unretained(this)));
+  web_ui()->RegisterMessageCallback("getOsVersion",
+      base::Bind(&AboutHandler::HandleGetOsVersion, base::Unretained(this)));
+  web_ui()->RegisterMessageCallback("getArcVersion",
+      base::Bind(&AboutHandler::HandleGetArcVersion, base::Unretained(this)));
+  web_ui()->RegisterMessageCallback("getOsFirmware",
+      base::Bind(&AboutHandler::HandleGetOsFirmware, base::Unretained(this)));
+  web_ui()->RegisterMessageCallback("getRegulatoryInfo",
+      base::Bind(&AboutHandler::HandleGetRegulatoryInfo,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback("getCurrentChannel",
+      base::Bind(&AboutHandler::HandleGetCurrentChannel,
+                 base::Unretained(this)));
+  web_ui()->RegisterMessageCallback("getTargetChannel",
+      base::Bind(&AboutHandler::HandleGetTargetChannel,
+                 base::Unretained(this)));
 #endif
 #if defined(OS_MACOSX)
   web_ui()->RegisterMessageCallback("promoteUpdater",
-      base::Bind(&HelpHandler::PromoteUpdater, base::Unretained(this)));
+      base::Bind(&AboutHandler::PromoteUpdater, base::Unretained(this)));
 #endif
 
 #if defined(OS_CHROMEOS)
   // Handler for the product label image, which will be shown if available.
   content::URLDataSource::Add(Profile::FromWebUI(web_ui()),
                               new chromeos::ImageSource());
 #endif
 }
 
-void HelpHandler::Observe(int type, const content::NotificationSource& source,
+void AboutHandler::Observe(int type, const content::NotificationSource& source,
                           const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_UPGRADE_RECOMMENDED: {
       // A version update is installed and ready to go. Refresh the UI so the
       // correct state will be shown.
-      RequestUpdate(NULL);
+      RequestUpdate();
       break;
     }
     default:
       NOTREACHED();
   }
 }
 
 // static
-base::string16 HelpHandler::BuildBrowserVersionString() {
+base::string16 AboutHandler::BuildBrowserVersionString() {
   std::string version = version_info::GetVersionNumber();
 
   std::string modifier = chrome::GetChannelString();
   if (!modifier.empty())
     version += " " + modifier;
 
 #if defined(ARCH_CPU_64_BITS)
   version += " (64-bit)";
 #endif
 
   return base::UTF8ToUTF16(version);
 }
 
-void HelpHandler::OnDeviceAutoUpdatePolicyChanged(
+void AboutHandler::OnDeviceAutoUpdatePolicyChanged(
     const base::Value* previous_policy,
     const base::Value* current_policy) {
   bool previous_auto_update_disabled = false;
   if (previous_policy)
     CHECK(previous_policy->GetAsBoolean(&previous_auto_update_disabled));
 
   bool current_auto_update_disabled = false;
   if (current_policy)
     CHECK(current_policy->GetAsBoolean(&current_auto_update_disabled));
 
   if (current_auto_update_disabled != previous_auto_update_disabled) {
     // Refresh the update status to refresh the status of the UI.
-    RefreshUpdateStatus();
+    HandleRefreshUpdateStatus(nullptr);
   }
 }
 
-void HelpHandler::RefreshUpdateStatus() {
+void AboutHandler::HandleRefreshUpdateStatus(const base::ListValue* args) {

[tommycli, 2016/05/10 20:45:39]

Optional suggestion: Consider making this an explicit Promise or adding a TODO.

It's not a hard requirement (and can be in a separate CL) since you're porting
existing code.

[dpapad, 2016/05/10 22:03:02]

I thought about this and decided not to convert this to Promise. The underlying
code depending on the current UpdateStatus might send 1-N notifications
(progress events). I at least cleaned it up so that they all come in the form of
a WebUI event, compared to the previous approach where multiple JS functions
were called by name.

Also the fact that this method has a different behavior on ChromeOS (check for
updates but don't apply them) VS non-ChromeOS (check and apply updates), adds
extra complexity.

   // On Chrome OS, do not check for an update automatically.
 #if defined(OS_CHROMEOS)
   static_cast<VersionUpdaterCros*>(version_updater_.get())->GetUpdateStatus(
-      base::Bind(&HelpHandler::SetUpdateStatus, base::Unretained(this)));
+      base::Bind(&AboutHandler::SetUpdateStatus, base::Unretained(this)));
 #else
-  RequestUpdate(NULL);
-#endif
-}
-
-void HelpHandler::OnPageLoaded(const base::ListValue* args) {
-#if defined(OS_CHROMEOS)
-  base::PostTaskAndReplyWithResult(
-      content::BrowserThread::GetBlockingPool(),
-      FROM_HERE,
-      base::Bind(&chromeos::version_loader::GetVersion,
-                 chromeos::version_loader::VERSION_FULL),
-      base::Bind(&HelpHandler::OnOSVersion,
-                 weak_factory_.GetWeakPtr()));
-  base::PostTaskAndReplyWithResult(
-      content::BrowserThread::GetBlockingPool(),
-      FROM_HERE,
-      base::Bind(&chromeos::version_loader::GetARCVersion),
-      base::Bind(&HelpHandler::OnARCVersion,
-                 weak_factory_.GetWeakPtr()));
-  base::PostTaskAndReplyWithResult(
-      content::BrowserThread::GetBlockingPool(),
-      FROM_HERE,
-      base::Bind(&chromeos::version_loader::GetFirmware),
-      base::Bind(&HelpHandler::OnOSFirmware,
-                 weak_factory_.GetWeakPtr()));
-
-  web_ui()->CallJavascriptFunction(
-      "help.HelpPage.updateEnableReleaseChannel",
-      base::FundamentalValue(CanChangeChannel(Profile::FromWebUI(web_ui()))));
-
-  base::Time build_time = base::SysInfo::GetLsbReleaseTime();
-  base::string16 build_date = base::TimeFormatFriendlyDate(build_time);
-  web_ui()->CallJavascriptFunction("help.HelpPage.setBuildDate",
-                                   base::StringValue(build_date));
-#endif  // defined(OS_CHROMEOS)
-
-  RefreshUpdateStatus();
-
-  web_ui()->CallJavascriptFunction(
-      "help.HelpPage.setObsoleteSystem",
-      base::FundamentalValue(ObsoleteSystem::IsObsoleteNowOrSoon()));
-  web_ui()->CallJavascriptFunction(
-      "help.HelpPage.setObsoleteSystemEndOfTheLine",
-      base::FundamentalValue(ObsoleteSystem::IsObsoleteNowOrSoon() &&
-                             ObsoleteSystem::IsEndOfTheLine()));
-
-#if defined(OS_CHROMEOS)
-  web_ui()->CallJavascriptFunction(
-      "help.HelpPage.updateIsEnterpriseManaged",
-      base::FundamentalValue(IsEnterpriseManaged()));
-  // First argument to GetChannel() is a flag that indicates whether
-  // current channel should be returned (if true) or target channel
-  // (otherwise).
-  version_updater_->GetChannel(true,
-      base::Bind(&HelpHandler::OnCurrentChannel, weak_factory_.GetWeakPtr()));
-  version_updater_->GetChannel(false,
-      base::Bind(&HelpHandler::OnTargetChannel, weak_factory_.GetWeakPtr()));
-
-  base::PostTaskAndReplyWithResult(
-      content::BrowserThread::GetBlockingPool(),
-      FROM_HERE,
-      base::Bind(&FindRegulatoryLabelDir),
-      base::Bind(&HelpHandler::OnRegulatoryLabelDirFound,
-                 weak_factory_.GetWeakPtr()));
+  RequestUpdate();
 #endif
 }
 
 #if defined(OS_MACOSX)
-void HelpHandler::PromoteUpdater(const base::ListValue* args) {
+void AboutHandler::PromoteUpdater(const base::ListValue* args) {
   version_updater_->PromoteUpdater();
 }
 #endif
 
-void HelpHandler::RelaunchNow(const base::ListValue* args) {
+void AboutHandler::HandleRelaunchNow(const base::ListValue* args) {
   DCHECK(args->empty());
   version_updater_->RelaunchBrowser();
 }
 
-void HelpHandler::OpenFeedbackDialog(const base::ListValue* args) {
+void AboutHandler::HandleOpenFeedbackDialog(const base::ListValue* args) {
   DCHECK(args->empty());
   Browser* browser = chrome::FindBrowserWithWebContents(
       web_ui()->GetWebContents());
   chrome::OpenFeedbackDialog(browser);
 }
 
-void HelpHandler::OpenHelpPage(const base::ListValue* args) {
+void AboutHandler::HandleOpenHelpPage(const base::ListValue* args) {
   DCHECK(args->empty());
   Browser* browser = chrome::FindBrowserWithWebContents(
       web_ui()->GetWebContents());
   chrome::ShowHelp(browser, chrome::HELP_SOURCE_WEBUI);
 }
 
 #if defined(OS_CHROMEOS)
 
-void HelpHandler::SetChannel(const base::ListValue* args) {
+void AboutHandler::SetChannel(const base::ListValue* args) {
   DCHECK(args->GetSize() == 2);
 
   if (!CanChangeChannel(Profile::FromWebUI(web_ui()))) {
     LOG(WARNING) << "Non-owner tried to change release track.";
     return;
   }
 
   base::string16 channel;
   bool is_powerwash_allowed;
   if (!args->GetString(0, &channel) ||
       !args->GetBoolean(1, &is_powerwash_allowed)) {
     LOG(ERROR) << "Can't parse SetChannel() args";
     return;
   }
 
   version_updater_->SetChannel(base::UTF16ToUTF8(channel),
                                is_powerwash_allowed);
   if (user_manager::UserManager::Get()->IsCurrentUserOwner()) {
     // Check for update after switching release channel.
-    version_updater_->CheckForUpdate(base::Bind(&HelpHandler::SetUpdateStatus,
+    version_updater_->CheckForUpdate(base::Bind(&AboutHandler::SetUpdateStatus,
                                                 base::Unretained(this)),
                                      VersionUpdater::PromoteCallback());
   }
 }
 
-void HelpHandler::RelaunchAndPowerwash(const base::ListValue* args) {
-  DCHECK(args->empty());
+void AboutHandler::HandleGetOsVersion(const base::ListValue* args) {

[tommycli, 2016/05/10 20:45:40]

Consider coalescing these blocking pool calls into one call.

[tommycli, 2016/05/10 20:45:40]

Another optional thing: Consider coalescing all these blocking pool calls into
one method instead of multiple separate calls.

Again this can be deferred until later since I can see you're porting existing
code...

[dpapad, 2016/05/10 22:03:02]

What would be the benefit of coalescing them? This seems cleaner to me, but I
don't know exactly how the code would look like if coalesced. Can you link to an
example of how to wait for multiple C++ async callbacks to return?

[tommycli, 2016/05/10 22:14:52]

I was thinking of something like:

namespace {

// Called on the blocking pool
unique_ptr<base::Value> GetChromeOsVersionInfo() {
  base::Value dictionary;
  // Multiple calls to chromeos::version_loader here..
  dictionary.add("arcVersion", ...);
  dictionary.add("version", ...);
  dictionary.add("osFirmware", ...);
  return dictionary;
}

}

void AboutHandler::HandleGetVersionInfo(base::ListValue* args) {

  base::PostTaskAndReplyWithResult(
    ...,
    base::Bind(&GetChromeOsVersionInfo),
    base::Bind(&AboutHandler::OnReceivedChromeOSVersionInfo));
}

again, not strictly necessary, just a possibility.

[dpapad, 2016/05/10 23:49:52]

Done. Thanks for the pointers.

+  CHECK_EQ(1U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
 
-  if (IsEnterpriseManaged())
-    return;
+  base::PostTaskAndReplyWithResult(
+      content::BrowserThread::GetBlockingPool(),
+      FROM_HERE,
+      base::Bind(&chromeos::version_loader::GetVersion,
+                 chromeos::version_loader::VERSION_FULL),
+      base::Bind(&AboutHandler::OnVersionReady,
+                 weak_factory_.GetWeakPtr(),
+                 callback_id));
+}
 
-  PrefService* prefs = g_browser_process->local_state();
-  prefs->SetBoolean(prefs::kFactoryResetRequested, true);
-  prefs->CommitPendingWrite();
+void AboutHandler::HandleGetArcVersion(const base::ListValue* args) {
+  CHECK_EQ(1U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
 
-  // Perform sign out. Current chrome process will then terminate, new one will
-  // be launched (as if it was a restart).
-  chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->RequestRestart();
+  base::PostTaskAndReplyWithResult(
+      content::BrowserThread::GetBlockingPool(),
+      FROM_HERE,
+      base::Bind(&chromeos::version_loader::GetARCVersion),
+      base::Bind(&AboutHandler::OnVersionReady,
+                 weak_factory_.GetWeakPtr(),
+                 callback_id));
+}
+
+void AboutHandler::HandleGetOsFirmware(const base::ListValue* args) {
+  CHECK_EQ(1U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
+
+  base::PostTaskAndReplyWithResult(
+      content::BrowserThread::GetBlockingPool(),
+      FROM_HERE,
+      base::Bind(&chromeos::version_loader::GetFirmware),
+      base::Bind(&AboutHandler::OnVersionReady,
+                 weak_factory_.GetWeakPtr(),
+                 callback_id));
+}
+
+void AboutHandler::OnVersionReady(std::string callback_id,
+                                  std::string version) {
+  ResolveJavascriptCallback(
+      base::StringValue(callback_id), base::StringValue(version));
+}
+
+void AboutHandler::HandleGetRegulatoryInfo(const base::ListValue* args) {
+  CHECK_EQ(1U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
+
+  base::PostTaskAndReplyWithResult(
+      content::BrowserThread::GetBlockingPool(),
+      FROM_HERE,
+      base::Bind(&FindRegulatoryLabelDir),
+      base::Bind(&AboutHandler::OnRegulatoryLabelDirFound,
+                 weak_factory_.GetWeakPtr(),
+                 callback_id));
+}
+
+void AboutHandler::HandleGetCurrentChannel(const base::ListValue* args) {
+  CHECK_EQ(1U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
+  // First argument to GetChannel() is a flag that indicates whether
+  // current channel should be returned (if true) or target channel
+  // (otherwise).
+  version_updater_->GetChannel(true,
+      base::Bind(&AboutHandler::OnGetChannelReady,
+                 weak_factory_.GetWeakPtr(),
+                 callback_id));
+}
+
+void AboutHandler::HandleGetTargetChannel(const base::ListValue* args) {
+  CHECK_EQ(1U, args->GetSize());
+  std::string callback_id;
+  CHECK(args->GetString(0, &callback_id));
+  version_updater_->GetChannel(false,
+      base::Bind(&AboutHandler::OnGetChannelReady,
+                 weak_factory_.GetWeakPtr(),
+                 callback_id));
+}
+
+void AboutHandler::OnGetChannelReady(std::string callback_id,
+                                     const std::string& channel) {
+  ResolveJavascriptCallback(
+      base::StringValue(callback_id), base::StringValue(channel));
+}
+
+void AboutHandler::HandleRequestUpdate(const base::ListValue* args) {
+  RequestUpdate();
 }
 
 #endif  // defined(OS_CHROMEOS)
 
-void HelpHandler::RequestUpdate(const base::ListValue* args) {
-  VersionUpdater::PromoteCallback promote_callback;
+void AboutHandler::RequestUpdate() {
   version_updater_->CheckForUpdate(
-      base::Bind(&HelpHandler::SetUpdateStatus, base::Unretained(this)),
+      base::Bind(&AboutHandler::SetUpdateStatus, base::Unretained(this)),
 #if defined(OS_MACOSX)
-      base::Bind(&HelpHandler::SetPromotionState, base::Unretained(this)));
+      base::Bind(&AboutHandler::SetPromotionState, base::Unretained(this)));
 #else
       VersionUpdater::PromoteCallback());
 #endif  // OS_MACOSX
 }
 
-void HelpHandler::SetUpdateStatus(VersionUpdater::Status status,
+void AboutHandler::SetUpdateStatus(VersionUpdater::Status status,
                                   int progress, const base::string16& message) {
   // Only UPDATING state should have progress set.
   DCHECK(status == VersionUpdater::UPDATING || progress == 0);
 
-  std::string status_str;
-  switch (status) {
-  case VersionUpdater::CHECKING:
-    status_str = "checking";
-    break;
-  case VersionUpdater::UPDATING:
-    status_str = "updating";
-    break;
-  case VersionUpdater::NEARLY_UPDATED:
-    status_str = "nearly_updated";
-    break;
-  case VersionUpdater::UPDATED:
-    status_str = "updated";
-    break;
-  case VersionUpdater::FAILED:
-  case VersionUpdater::FAILED_OFFLINE:
-  case VersionUpdater::FAILED_CONNECTION_TYPE_DISALLOWED:
-    status_str = "failed";
-    break;
-  case VersionUpdater::DISABLED:
-    status_str = "disabled";
-    break;
-  case VersionUpdater::DISABLED_BY_ADMIN:
-    status_str = "disabled_by_admin";
-    break;
-  }
-
-  web_ui()->CallJavascriptFunction("help.HelpPage.setUpdateStatus",
-                                   base::StringValue(status_str),
-                                   base::StringValue(message));
-
-  if (status == VersionUpdater::UPDATING) {
-    web_ui()->CallJavascriptFunction("help.HelpPage.setProgress",
-                                     base::FundamentalValue(progress));
-  }
+  std::unique_ptr<base::DictionaryValue> event(new base::DictionaryValue);
+  event->SetString("status", UpdateStatusToString(status));
+  event->SetString("message", message);
+  event->SetInteger("progress", progress);
 
 #if defined(OS_CHROMEOS)
   if (status == VersionUpdater::FAILED_OFFLINE ||
       status == VersionUpdater::FAILED_CONNECTION_TYPE_DISALLOWED) {
     base::string16 types_msg = GetAllowedConnectionTypesMessage();
-    if (!types_msg.empty()) {
-      web_ui()->CallJavascriptFunction(
-          "help.HelpPage.setAndShowAllowedConnectionTypesMsg",
-          base::StringValue(types_msg));
-    } else {
-      web_ui()->CallJavascriptFunction(
-          "help.HelpPage.showAllowedConnectionTypesMsg",
-          base::FundamentalValue(false));
-    }
+    if (!types_msg.empty())
+      event->SetString("connectionTypes", types_msg);
+    else
+      event->Set("connectionTypes", base::Value::CreateNullValue());
   } else {
-    web_ui()->CallJavascriptFunction(
-        "help.HelpPage.showAllowedConnectionTypesMsg",
-        base::FundamentalValue(false));
+    event->Set("connectionTypes", base::Value::CreateNullValue());
   }
 #endif  // defined(OS_CHROMEOS)
+
+  web_ui()->CallJavascriptFunction("cr.webUIListenerCallback",
+                                   base::StringValue("update-status-changed"),
+                                   *event);
 }
 
 #if defined(OS_MACOSX)
-void HelpHandler::SetPromotionState(VersionUpdater::PromotionState state) {
+void AboutHandler::SetPromotionState(VersionUpdater::PromotionState state) {
   std::string state_str;
   switch (state) {
   case VersionUpdater::PROMOTE_HIDDEN:
     state_str = "hidden";
     break;
   case VersionUpdater::PROMOTE_ENABLED:
     state_str = "enabled";
     break;
   case VersionUpdater::PROMOTE_DISABLED:
     state_str = "disabled";
     break;
   }
 
   web_ui()->CallJavascriptFunction("help.HelpPage.setPromotionState",
                                    base::StringValue(state_str));
 }
 #endif  // defined(OS_MACOSX)
 
 #if defined(OS_CHROMEOS)
-void HelpHandler::OnOSVersion(const std::string& version) {
-  web_ui()->CallJavascriptFunction("help.HelpPage.setOSVersion",
-                                   base::StringValue(version));
-}
-
-void HelpHandler::OnARCVersion(const std::string& firmware) {
-  web_ui()->CallJavascriptFunction("help.HelpPage.setARCVersion",
-                                   base::StringValue(firmware));
-}
-
-void HelpHandler::OnOSFirmware(const std::string& firmware) {
-  web_ui()->CallJavascriptFunction("help.HelpPage.setOSFirmware",
-                                   base::StringValue(firmware));
-}
-
-void HelpHandler::OnCurrentChannel(const std::string& channel) {
-  web_ui()->CallJavascriptFunction(
-      "help.HelpPage.updateCurrentChannel", base::StringValue(channel));
-}
-
-void HelpHandler::OnTargetChannel(const std::string& channel) {
-  web_ui()->CallJavascriptFunction(
-      "help.HelpPage.updateTargetChannel", base::StringValue(channel));
-}
-
-void HelpHandler::OnRegulatoryLabelDirFound(const base::FilePath& path) {
-  if (path.empty())
+void AboutHandler::OnRegulatoryLabelDirFound(std::string callback_id,
+                                             const base::FilePath& path) {
+  if (path.empty()) {
+    ResolveJavascriptCallback(base::StringValue(callback_id),

[tommycli, 2016/05/10 20:45:40]

Is this still considered a "success"? Or should it be RejectJavascript...

[dpapad, 2016/05/10 22:03:02]

Rejecting a promise is the equivalent of throwing an error in a sync function.
On the other hand, not having a regulatory label is not really an error
(ChromiumOS does not have one), so treating this as success seems more
appropriate.

+                              *base::Value::CreateNullValue());
     return;
+  }
 
   base::PostTaskAndReplyWithResult(
       content::BrowserThread::GetBlockingPool(), FROM_HERE,
       base::Bind(&ReadRegulatoryLabelText, path),
-      base::Bind(&HelpHandler::OnRegulatoryLabelTextRead,
-                 weak_factory_.GetWeakPtr()));
-
-  // Send the image path to the WebUI.
-  OnRegulatoryLabelImageFound(path.AppendASCII(kRegulatoryLabelImageFilename));
+      base::Bind(&AboutHandler::OnRegulatoryLabelTextRead,
+                 weak_factory_.GetWeakPtr(),
+                 callback_id, path));
 }
 
-void HelpHandler::OnRegulatoryLabelImageFound(const base::FilePath& path) {
+void AboutHandler::OnRegulatoryLabelTextRead(std::string callback_id,
+                                             const base::FilePath& path,
+                                             const std::string& text) {
+  std::unique_ptr<base::DictionaryValue> regulatory_info(
+      new base::DictionaryValue);
+  // Remove unnecessary whitespace.
+  regulatory_info->SetString("text", base::CollapseWhitespaceASCII(text, true));
   std::string url = std::string("chrome://") + chrome::kChromeOSAssetHost +
       "/" + path.MaybeAsASCII();
-  web_ui()->CallJavascriptFunction("help.HelpPage.setRegulatoryLabelPath",
-                                   base::StringValue(url));
-}
+  regulatory_info->SetString("url", url);
 
-void HelpHandler::OnRegulatoryLabelTextRead(const std::string& text) {
-  // Remove unnecessary whitespace.
-  web_ui()->CallJavascriptFunction(
-      "help.HelpPage.setRegulatoryLabelText",
-      base::StringValue(base::CollapseWhitespaceASCII(text, true)));
+  ResolveJavascriptCallback(
+      base::StringValue(callback_id), *regulatory_info);
 }
 
 #endif  // defined(OS_CHROMEOS)
+
+}  // namespace settings