Fix always-in-roaming reset loop for cellular connections on Chromium OS.

chrome/browser/chromeos/cros/network_library_impl_cros.cc

 // Copyright (c) 2012 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/chromeos/cros/network_library_impl_cros.h"
 
 #include <dbus/dbus-glib.h>
 #include "base/command_line.h"
 #include "base/json/json_writer.h"  // for debug output only.
 #include "base/metrics/histogram.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/native_network_constants.h"
 #include "chrome/browser/chromeos/cros/native_network_parser.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 using content::BrowserThread;
 
 namespace chromeos {
 
 namespace {
 
 // List of cellular operators names that should have data roaming always enabled
 // to be able to connect to any network.
 const char* kAlwaysInRoamingOperators[] = {
-  "CUBIC"
+  "CUBIC",
+  "Cubic",
 };
 
 // List of interfaces that have portal check enabled by default.
 const char kDefaultCheckPortalList[] = "ethernet,wifi,cellular";
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////
 
 NetworkLibraryImplCros::NetworkLibraryImplCros()
@@ skipping 118 matching lines @@
           path,
           base::Bind(&NetworkLibraryImplCros::NetworkDeviceUpdate,
                      weak_ptr_factory_.GetWeakPtr()));
     }
   }
 }
 
 bool NetworkLibraryImplCros::UpdateCellularDeviceStatus(
     NetworkDevice* device, PropertyIndex index) {
   if (index == PROPERTY_INDEX_CELLULAR_ALLOW_ROAMING) {
-    if (!device->data_roaming_allowed() && IsCellularAlwaysInRoaming()) {
-      SetCellularDataRoamingAllowed(true);
+    if (IsCellularAlwaysInRoaming()) {
+      if (!device->data_roaming_allowed())
+        SetCellularDataRoamingAllowed(true);
     } else {
       bool settings_value;
       if ((CrosSettings::Get()->GetBoolean(
               kSignedDataRoamingEnabled, &settings_value)) &&
           (device->data_roaming_allowed() != settings_value)) {
         // Switch back to signed settings value.
         SetCellularDataRoamingAllowed(settings_value);
         return false;
       }
     }
@@ skipping 990 matching lines @@
     NativeNetworkDeviceParser parser;
     device = parser.CreateDeviceFromInfo(device_path, info);
     VLOG(2) << " Adding device: " << device_path;
     if (device) {
       device_map_[device_path] = device;
     }
     CHECK(device) << "Attempted to add NULL device for path: " << device_path;
   }
   VLOG(2) << "ParseNetworkDevice:" << device->name();
   if (device && device->type() == TYPE_CELLULAR) {
     if (!device->data_roaming_allowed() && IsCellularAlwaysInRoaming()) {

[petkov, 2012/08/13 09:55:13]

Should we change this logic too?

[Mattias Nissler (ping if slow), 2012/08/13 11:05:00]

Yes, good catch. I think in this case it actually wouldn't have caused a loop,
but it's incorrect here as well. There's really no good reason to have the code
twice though, so I've removed this instance entirely.

       SetCellularDataRoamingAllowed(true);
     } else {
       bool settings_value;
       if (CrosSettings::Get()->GetBoolean(
               kSignedDataRoamingEnabled, &settings_value) &&
           device->data_roaming_allowed() != settings_value) {
         // Switch back to signed settings value.
         SetCellularDataRoamingAllowed(settings_value);
       }
     }
   }
   NotifyNetworkManagerChanged(false);  // Not forced.
   AddNetworkDeviceObserver(device_path, network_device_observer_.get());
 }
 
 }  // namespace chromeos