Use ONC for Cellular APN and SimLock in Settings

chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc

Index: chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
diff --git a/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc b/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
index f49f63e0f40b1fd8305fa23d62d98fb513cc11ac..4e590ece58f58bf4a5d1cb7477b6d247e618ea59 100644
--- a/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
+++ b/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc
@@ -131,7 +131,6 @@ const char kShowMorePlanInfoMessage[] = "showMorePlanInfo";
 const char kTagActivate[] = "activate";
 const char kTagActivationState[] = "activationState";
 const char kTagAddConnection[] = "add";
-const char kTagApn[] = "apn";
 const char kTagCarrierSelectFlag[] = "showCarrierSelect";
 const char kTagCarrierUrl[] = "carrierUrl";
 const char kTagCellularAvailable[] = "cellularAvailable";
@@ -145,16 +144,13 @@ const char kTagDisconnect[] = "disconnect";
 const char kTagErrorMessage[] = "errorMessage";
 const char kTagForget[] = "forget";
 const char kTagLanguage[] = "language";
-const char kTagLastGoodApn[] = "lastGoodApn";
 const char kTagLocalizedName[] = "localizedName";
 const char kTagName[] = "name";
 const char kTagNameServersGoogle[] = "nameServersGoogle";
 const char kTagNameServerType[] = "nameServerType";
-const char kTagNetworkId[] = "networkId";
 const char kTagOptions[] = "options";
 const char kTagPassword[] = "password";
 const char kTagPolicy[] = "policy";
-const char kTagProviderApnList[] = "providerApnList";
 const char kTagRecommended[] = "recommended";
 const char kTagRecommendedValue[] = "recommendedValue";
 const char kTagRemembered[] = "remembered";
@@ -166,8 +162,6 @@ const char kTagCurrentCarrierIndex[] = "currentCarrierIndex";
 const char kTagShared[] = "shared";
 const char kTagShowActivateButton[] = "showActivateButton";
 const char kTagShowViewAccountButton[] = "showViewAccountButton";
-const char kTagSimCardLockEnabled[] = "simCardLockEnabled";
-const char kTagSupportUrl[] = "supportUrl";
 const char kTagTrue[] = "true";
 const char kTagUsername[] = "username";
 const char kTagValue[] = "value";
@@ -506,24 +500,7 @@ int FindCurrentCarrierIndex(const base::ListValue* carriers,
   return -1;
 }
 
-void CreateDictionaryFromCellularApn(const base::DictionaryValue* apn,
-                                     base::DictionaryValue* dictionary) {
-  CopyStringFromDictionary(*apn, shill::kApnProperty, kTagApn, dictionary);
-  CopyStringFromDictionary(
-      *apn, shill::kApnNetworkIdProperty, kTagNetworkId, dictionary);
-  CopyStringFromDictionary(
-      *apn, shill::kApnUsernameProperty, kTagUsername, dictionary);
-  CopyStringFromDictionary(
-      *apn, shill::kApnPasswordProperty, kTagPassword, dictionary);
-  CopyStringFromDictionary(*apn, shill::kApnNameProperty, kTagName, dictionary);
-  CopyStringFromDictionary(
-      *apn, shill::kApnLocalizedNameProperty, kTagLocalizedName, dictionary);
-  CopyStringFromDictionary(
-      *apn, shill::kApnLanguageProperty, kTagLanguage, dictionary);
-}
-
 void PopulateCellularDetails(const NetworkState* cellular,
-                             const base::DictionaryValue& shill_properties,
                              base::DictionaryValue* dictionary) {
   dictionary->SetBoolean(kTagCarrierSelectFlag,
                          CommandLine::ForCurrentProcess()->HasSwitch(
@@ -539,29 +516,6 @@ void PopulateCellularDetails(const NetworkState* cellular,
                         internet_options_strings::RestrictedStateString(
                             cellular->connection_state()));
 
-  const base::DictionaryValue* olp = NULL;
-  if (shill_properties.GetDictionaryWithoutPathExpansion(
-          shill::kPaymentPortalProperty, &olp)) {
-    std::string url;
-    olp->GetStringWithoutPathExpansion(shill::kPaymentPortalURL, &url);
-    dictionary->SetString(kTagSupportUrl, url);

[stevenjb, 2014/08/21 23:06:58]

This is unused in the JS code.

-  }
-
-  base::DictionaryValue* apn = new base::DictionaryValue;
-  const base::DictionaryValue* source_apn = NULL;
-  if (shill_properties.GetDictionaryWithoutPathExpansion(
-          shill::kCellularApnProperty, &source_apn)) {
-    CreateDictionaryFromCellularApn(source_apn, apn);
-  }
-  dictionary->Set(kTagApn, apn);
-
-  base::DictionaryValue* last_good_apn = new base::DictionaryValue;
-  if (shill_properties.GetDictionaryWithoutPathExpansion(
-          shill::kCellularLastGoodApnProperty, &source_apn)) {
-    CreateDictionaryFromCellularApn(source_apn, last_good_apn);
-  }
-  dictionary->Set(kTagLastGoodApn, last_good_apn);
-
   // These default to empty and are only set if device != NULL.
   std::string carrier_id;
   std::string mdn;
@@ -572,15 +526,6 @@ void PopulateCellularDetails(const NetworkState* cellular,
           cellular->device_path());
   if (device) {
     const base::DictionaryValue& device_properties = device->properties();
-    ::onc::ONCSource onc_source;
-    NetworkHandler::Get()->managed_network_configuration_handler()->
-        FindPolicyByGUID(LoginState::Get()->primary_user_hash(),
-                         cellular->guid(), &onc_source);
-    const NetworkPropertyUIData cellular_property_ui_data(onc_source);
-    SetValueDictionary(kTagSimCardLockEnabled,
-                       new base::FundamentalValue(device->sim_lock_enabled()),
-                       cellular_property_ui_data,
-                       dictionary);
 
     carrier_id = device->home_provider_id();
     device_properties.GetStringWithoutPathExpansion(shill::kMdnProperty, &mdn);
@@ -592,25 +537,6 @@ void PopulateCellularDetails(const NetworkState* cellular,
         dictionary->SetString(kTagCarrierUrl, carrier->top_up_url());
     }
 
-    base::ListValue* apn_list_value = new base::ListValue();
-    const base::ListValue* apn_list;
-    if (device_properties.GetListWithoutPathExpansion(
-            shill::kCellularApnListProperty, &apn_list)) {
-      for (base::ListValue::const_iterator iter = apn_list->begin();
-           iter != apn_list->end();
-           ++iter) {
-        const base::DictionaryValue* dict;
-        if ((*iter)->GetAsDictionary(&dict)) {
-          base::DictionaryValue* apn = new base::DictionaryValue;
-          CreateDictionaryFromCellularApn(dict, apn);
-          apn_list_value->Append(apn);
-        }
-      }
-    }
-    SetValueDictionary(kTagProviderApnList,
-                       apn_list_value,
-                       cellular_property_ui_data,
-                       dictionary);
     const base::ListValue* supported_carriers;
     if (device_properties.GetListWithoutPathExpansion(
             shill::kSupportedCarriersProperty, &supported_carriers)) {
@@ -626,12 +552,9 @@ void PopulateCellularDetails(const NetworkState* cellular,
   }
 
   // Don't show any account management related buttons if the activation
-  // state is unknown or no payment portal URL is available.
-  std::string support_url;
-  if (cellular->activation_state() == shill::kActivationStateUnknown ||
-      !dictionary->GetString(kTagSupportUrl, &support_url) ||
-      support_url.empty()) {

[stevenjb, 2014/08/21 23:06:58]

Since the support URL is unused, it doesn't seem to make any sense to test for
it when determining whether or not to display the show portal button below.

[armansito, 2014/08/22 00:56:45]

Even though the settings JS isn't using it, I think we still need the check
here. As far as I can remember, this is how the code distinguishes between
activation flows that don't use the mobile setup dialog vs those that do.

thieule@, can you comment on this?

[stevenjb, 2014/08/22 17:08:44]

According to service_constants.h, kPaymentPortalProperty is deprecated.

showMorePlanInfo controls a button that calls ShowMorePlanInfoCallback which
calls ash::network_connect::ShowMobileSetup(), which checks activation state and
calls MobileSetupDialog::Show().

So, it seems that relying on
MobileConfig::GetInstance()->GetCarrier(carrier_id)->show_portal_button() should
be sufficient, i.e. any logic to determine whether or not the button should be
visible should be done there.

[thieule, 2014/08/22 19:13:04]

We should be using the new kActivationType property to determine the type of
activation flow.


On 2014/08/22 00:56:45, armansito wrote:

[stevenjb, 2014/08/25 20:57:27]

So... is there an additional check that we should be doing here or not? 

[armansito, 2014/08/25 21:48:50]

Yeah...I wouldn't trust that. Then again I think the actual payment portal URL
is the Cellular.OLP property?
Yeah, I guess this should suffice then. I think we don't really provide that
support URL thing on any of the newer versions (or carriers) so I think this
should be OK. I don't know if there will be any regressions (perhaps some older
plans) but might as well commit this.

[stevenjb, 2014/08/25 22:32:34]

So, I looked at the code again and realized that mobile_setup_ui.cc is
independent of mobile_confg_ui.cc. I think we need to do some cleanup of that
code at some point, but for now I am going to try to preserve the existing
logic.

-    VLOG(2) << "No support URL is available. Don't display buttons.";
+  // state is unknown.
+  if (cellular->activation_state() == shill::kActivationStateUnknown) {
+    VLOG(2) << "Activation state unknown. Don't display buttons.";
     return;
   }
 
@@ -641,16 +564,8 @@ void PopulateCellularDetails(const NetworkState* cellular,
   } else {
     bool may_show_portal_button = false;
 
-    // If an online payment URL was provided by shill, then this means that the
-    // "View Account" button should be shown for the current carrier.
-    if (olp) {
-      std::string url;
-      olp->GetStringWithoutPathExpansion(shill::kPaymentPortalURL, &url);
-      may_show_portal_button = !url.empty();
-    }
-    // If no online payment URL was provided by shill, fall back to
-    // MobileConfig to determine if the "View Account" should be shown.
-    if (!may_show_portal_button && MobileConfig::GetInstance()->IsReady()) {
+    // Use MobileConfig to determine if the "View Account" should be shown.
+    if (MobileConfig::GetInstance()->IsReady()) {
       const MobileConfig::Carrier* carrier =
           MobileConfig::GetInstance()->GetCarrier(carrier_id);
       may_show_portal_button = carrier && carrier->show_portal_button();
@@ -721,7 +636,7 @@ scoped_ptr<base::DictionaryValue> PopulateConnectionDetails(
   dictionary->SetBoolean(kTagDeviceConnected, connected_network != NULL);
 
   if (type == shill::kTypeCellular)
-    PopulateCellularDetails(network, shill_properties, dictionary.get());
+    PopulateCellularDetails(network, dictionary.get());
   else if (type == shill::kTypeVPN)
     PopulateVPNDetails(network, shill_properties, dictionary.get());