Show proper message in about Chrome OS page

chrome/browser/ui/webui/help/version_updater_chromeos.cc

Index: chrome/browser/ui/webui/help/version_updater_chromeos.cc
diff --git a/chrome/browser/ui/webui/help/version_updater_chromeos.cc b/chrome/browser/ui/webui/help/version_updater_chromeos.cc
index 00d657bafea0f5d0c6b952ff0883571aba62ab27..63450c1346d3e6582d3de592672aca105262a56d 100644
--- a/chrome/browser/ui/webui/help/version_updater_chromeos.cc
+++ b/chrome/browser/ui/webui/help/version_updater_chromeos.cc
@@ -37,9 +37,9 @@ namespace {
 
 // Network status in the context of device update.
 enum NetworkStatus {
-  // It's allowed in device policy to use current network for update.
+  // It's allowed to use current network for update.
   NETWORK_STATUS_ALLOWED = 0,
-  // It's disallowed in device policy to use current network for update.
+  // It's disallowed to use current network for update.
   NETWORK_STATUS_DISALLOWED,
   // Device is in offline state.
   NETWORK_STATUS_OFFLINE
@@ -47,16 +47,21 @@ enum NetworkStatus {
 
 const bool kDefaultAutoUpdateDisabled = false;
 
-NetworkStatus GetNetworkStatus(const chromeos::NetworkState* network) {
+NetworkStatus GetNetworkStatus(bool interactive,
+                               const chromeos::NetworkState* network) {
   if (!network || !network->IsConnectedState())  // Offline state.
     return NETWORK_STATUS_OFFLINE;
-  // The connection type checking strategy must be the same as the one
-  // used in update engine.
+  // The connection type checking strategy for non-cellular is the same as the
+  // one used in update engine.

[stevenjb, 2017/05/22 22:57:32]

The comment doesn't align very well with the logic here. Maybe just:
// Update Engine prohibits updates over Bluetooth

(That said, we don't actually support Bluetooth network connections and probably
never will, so we could just remove this line.)

[weidongg, 2017/05/23 00:17:01]

Ok, removed.

   if (network->type() == shill::kTypeBluetooth)
     return NETWORK_STATUS_DISALLOWED;
-  // Allow updates over cellular by default in chrome, as update engine still
-  // needs to check device policy and user preferences when checking for
-  // updates to decide whether to proceed to downloading.
+  // We leave the decision as to whether to allow updates over cellular up to
+  // update engine if the user is actively checking for updates and device
+  // policy is not set.

[stevenjb, 2017/05/22 22:57:32]

I think this comment just confuses things; we should rely on the comment for
IsUpdateOverCellularAllowed() instead.

[weidongg, 2017/05/23 00:17:00]

Ok, removed

+  if (network->type() == shill::kTypeCellular &&
+      !help_utils_chromeos::IsUpdateOverCellularAllowed(interactive)) {
+    return NETWORK_STATUS_DISALLOWED;
+  }
   return NETWORK_STATUS_ALLOWED;
 }
 
@@ -74,8 +79,10 @@ bool IsAutoUpdateDisabled() {
 }
 
 // Returns whether an update is allowed. If not, it calls the callback with
-// the appropriate status.
-bool EnsureCanUpdate(const VersionUpdater::StatusCallback& callback) {
+// the appropriate status. |interactive| indicates whether the user is actively
+// checking for updates.
+bool EnsureCanUpdate(bool interactive,
+                     const VersionUpdater::StatusCallback& callback) {
   if (IsAutoUpdateDisabled()) {
     callback.Run(VersionUpdater::DISABLED_BY_ADMIN, 0, std::string(), 0,
                  l10n_util::GetStringUTF16(IDS_UPGRADE_DISABLED_BY_POLICY));
@@ -89,7 +96,7 @@ bool EnsureCanUpdate(const VersionUpdater::StatusCallback& callback) {
 
   // Don't allow an update if we're currently offline or connected
   // to a network for which updates are disallowed.
-  NetworkStatus status = GetNetworkStatus(network);
+  NetworkStatus status = GetNetworkStatus(interactive, network);
   if (status == NETWORK_STATUS_OFFLINE) {
     callback.Run(VersionUpdater::FAILED_OFFLINE, 0, std::string(), 0,
                  l10n_util::GetStringUTF16(IDS_UPGRADE_OFFLINE));
@@ -116,7 +123,8 @@ VersionUpdater* VersionUpdater::Create(content::WebContents* web_contents) {
 void VersionUpdaterCros::GetUpdateStatus(const StatusCallback& callback) {
   callback_ = callback;
 
-  if (!EnsureCanUpdate(callback))
+  // User is not actively checking for updates.
+  if (!EnsureCanUpdate(false /* interactive */, callback))
     return;
 
   UpdateEngineClient* update_engine_client =
@@ -132,7 +140,8 @@ void VersionUpdaterCros::CheckForUpdate(const StatusCallback& callback,
                                         const PromoteCallback&) {
   callback_ = callback;
 
-  if (!EnsureCanUpdate(callback))
+  // User is actively checking for updates.
+  if (!EnsureCanUpdate(true /* interactive */, callback))
     return;
 
   UpdateEngineClient* update_engine_client =