[cros] Add network tray item for mobile network setup.

chrome/browser/chromeos/system/ash_system_tray_delegate.cc

Index: chrome/browser/chromeos/system/ash_system_tray_delegate.cc
diff --git a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
index 1922b4c8e0693a01c290b52703d57877849bf0c5..a0c0da1c412e6e503da9929762dcd8a68971f3e4 100644
--- a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
+++ b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
@@ -595,7 +595,9 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
   }
 
   virtual bool GetCellularCarrierInfo(std::string* carrier_id,
-                                      std::string* topup_url) OVERRIDE {
+                                      std::string* topup_url,
+                                      std::string* setup_url) OVERRIDE {
+    bool result = false;
     NetworkLibrary* crosnet = CrosLibrary::Get()->GetNetworkLibrary();
     const NetworkDevice* cellular = crosnet->FindCellularDevice();
     if (cellular) {

[stevenjb, 2012/04/25 01:29:49]

if (!cellular) return; would simplify this a bit.

[Nikita (slow), 2012/04/25 14:01:38]

Done.

@@ -605,15 +607,24 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
         const MobileConfig::Carrier* carrier = config->GetCarrier(*carrier_id);
         if (carrier) {
           *topup_url = carrier->top_up_url();
-          return true;
+          result = true;
+        }
+        const MobileConfig::LocaleConfig* locale_config =
+            config->GetLocaleConfig();
+        if (locale_config) {
+          // Only link to setup URL if SIM card is not inserted.
+          if (cellular->is_sim_absent()) {
+            *setup_url = locale_config->setup_url();
+            result = true;

[stevenjb, 2012/04/25 01:29:49]

The return logic seems flawed now. We can return true now even if we fail to get
the topup url. Shouldn't we return false if !carrier above?

[Nikita (slow), 2012/04/25 14:01:38]

LocaleConfig might be available even if current carrier is not mapped. In fact
that would be exactly the case as we only return setup_url if SIM card is absent
i.e. carrier is not matched.

So GetCellularCarrierInfo returns true if one of these carrier_id/topup_url or
setup_url was initialized.

Code in tray_network.cc handles that in a way so that if carrier information is
defined, it creates top up UI entry. If setup URL is defined, it creates related
UI entry.

Updated comment in header.

Do you think that this should be further split into 2 methods?
bool GetCellularCarrierInfo(std::string* carrier_id, std::string* topup_url); 
bool GetCellularSetupInfo(std::string* setup_url);

[stevenjb, 2012/04/25 16:47:55]

This seems fine with the updated header. I think it would be better as two
functions, but eventually this should all get moved to ash/ and chromeos/, so I
wouldn't worry about splitting it up as long as the header comment is clear.

+          }
         }
       }
     }
-    return false;
+    return result;
   }
 
-  virtual void ShowCellularTopupURL(const std::string& topup_url) OVERRIDE {
-    GetAppropriateBrowser()->ShowSingletonTab(GURL(topup_url));
+  virtual void ShowCellularURL(const std::string& url) OVERRIDE {
+    GetAppropriateBrowser()->ShowSingletonTab(GURL(url));
   }
 
   virtual void ChangeProxySettings() OVERRIDE {