Merged Tether and cellular network types in System Tray.

chrome/browser/chromeos/tether/tether_service.cc

Index: chrome/browser/chromeos/tether/tether_service.cc
diff --git a/chrome/browser/chromeos/tether/tether_service.cc b/chrome/browser/chromeos/tether/tether_service.cc
index 797e978e9493975084f819dd189998e74be0db0e..da52e78549fa90bc41605955742eac600120a9db 100644
--- a/chrome/browser/chromeos/tether/tether_service.cc
+++ b/chrome/browser/chromeos/tether/tether_service.cc
@@ -177,8 +177,8 @@ void TetherService::DeviceListChanged() {
   if (is_enabled != was_pref_enabled) {
     profile_->GetPrefs()->SetBoolean(prefs::kInstantTetheringEnabled,
                                      is_enabled);
-    UpdateTetherTechnologyState();
   }
+  UpdateTetherTechnologyState();
 }
 
 void TetherService::OnPrefsChanged() {
@@ -214,7 +214,7 @@ TetherService::GetTetherTechnologyState() {
   } else if (!IsAllowedByPolicy()) {
     return chromeos::NetworkStateHandler::TechnologyState::
         TECHNOLOGY_PROHIBITED;
-  } else if (!IsBluetoothAvailable()) {
+  } else if (!IsBluetoothAvailable() || IsCellularAvailableButNotEnabled()) {

[khorimoto, 2017/05/30 22:35:16]

nit: Please add an explanation for why tether is left uninitialized when
cellular is available but not enabled.

[lesliewatkins, 2017/05/31 00:43:27]

Done.

[Kyle Horimoto, 2017/05/31 20:13:59]

Not actually done.

[lesliewatkins, 2017/05/31 22:06:28]

Done.

[Kyle Horimoto, 2017/05/31 22:18:48]

Can you move the comment into the if() block? It's currently above the code,
which makes it seem like it applies to the previous if() block.

[lesliewatkins, 2017/06/05 19:41:42]

Done.

     // TODO (hansberry): This unfortunately results in a weird UI state for

[khorimoto, 2017/05/30 22:35:16]

nit: Does this description still apply the same way now that we changed the if()
condition? Can you modify it accordingly?

[lesliewatkins, 2017/05/31 00:43:27]

Done.

[Kyle Horimoto, 2017/05/31 20:13:59]

Please explicitly refer to the code in question:

// TODO(hansberry): When !IsBluetoothAvailable(), the UI can get stuck in a
weird state...

[lesliewatkins, 2017/05/31 22:06:28]

Done.

[Kyle Horimoto, 2017/05/31 22:18:48]

nit: Please change the wording so that the "When !IsBluetoothAvailable()" is at
the beginning. Without this change, the comment is confusing because it appears
to imply that the weird UI state will always occur at this point in the code.

[lesliewatkins, 2017/06/05 19:41:42]

Done.

     // Settings where the toggle is clickable but immediately becomes disabled
     // after enabling it. Possible solution: grey out the toggle and tell the
@@ -239,6 +239,13 @@ bool TetherService::IsBluetoothAvailable() const {
   return adapter_.get() && adapter_->IsPresent() && adapter_->IsPowered();
 }
 
+bool TetherService::IsCellularAvailableButNotEnabled() const {
+  return (network_state_handler_->IsTechnologyAvailable(
+              chromeos::NetworkTypePattern::Cellular()) &&
+          !network_state_handler_->IsTechnologyEnabled(
+              chromeos::NetworkTypePattern::Cellular()));
+}
+
 bool TetherService::IsAllowedByPolicy() const {
   return profile_->GetPrefs()->GetBoolean(prefs::kInstantTetheringAllowed);
 }