Skip to content

Commit

Permalink
Additional debugging for chromium-os:7864
Browse files Browse the repository at this point in the history 
Added additional checking to ensure that libcros calls are made from the UI thread.

I have been unable to establish any obvious cause for 7864. However, it is possible
that chromeos::NetworkLibrary calls are being made outside the UI thread. This will
catch any such calls, crashing where the call is made instead of at some later point.

I also audited network_library.cc to ensure that we check CrosLibrary::EnsureLoaded() around all cros calls.

BUG=http://code.google.com/p/chromium-os/issues/detail?id=7864
TEST=run Chrome on ChromeOS, make some changes to the networking menu, and make sure Chrome doesn't crash.

Review URL: http://codereview.chromium.org/4002001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63390 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
stevenjb@chromium.org committed Oct 21, 2010
1 parent d9ffd55 commit 3dd5198
Showing 1 changed file with 36 additions and 22 deletions.
58 changes: 36 additions & 22 deletions chrome/browser/chromeos/cros/network_library.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,18 @@ static std::string SafeString(const char* s) {
return s ? std::string(s) : std::string();
}

static bool EnsureCrosLoaded() {
if (!CrosLibrary::Get()->EnsureLoaded()) {
return false;
} else {
if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
LOG(ERROR) << "chromeos_library calls made from non UI thread!";
NOTREACHED();
}
return true;
}
}

////////////////////////////////////////////////////////////////////////////////
// Network

Expand All @@ -86,7 +98,7 @@ void Network::ConfigureFromService(const ServiceInfo& service) {
device_path_ = SafeString(service.device_path);
ip_address_.clear();
// If connected, get ip config.
if (connected() && service.device_path) {
if (EnsureCrosLoaded() && connected() && service.device_path) {
IPConfigStatus* ipconfig_status = ListIPConfigs(service.device_path);
if (ipconfig_status) {
for (int i = 0; i < ipconfig_status->size; i++) {
Expand Down Expand Up @@ -199,7 +211,7 @@ CellularNetwork::CellularNetwork()
}

bool CellularNetwork::StartActivation() const {
if (!CrosLibrary::Get()->EnsureLoaded())
if (!EnsureCrosLoaded())
return false;
return ActivateCellularModem(service_path_.c_str(), NULL);
}
Expand Down Expand Up @@ -451,19 +463,21 @@ class NetworkLibraryImpl : public NetworkLibrary {
enabled_devices_(0),
connected_devices_(0),
offline_mode_(false) {
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
Init();
} else {
InitTestData();
}
}

~NetworkLibraryImpl() {
if (network_status_connection_) {
DisconnectMonitorNetwork(network_status_connection_);
}
if (data_plan_monitor_) {
DisconnectDataPlanUpdateMonitor(data_plan_monitor_);
if (EnsureCrosLoaded()) {
if (network_status_connection_) {
DisconnectMonitorNetwork(network_status_connection_);
}
if (data_plan_monitor_) {
DisconnectDataPlanUpdateMonitor(data_plan_monitor_);
}
}
}

Expand Down Expand Up @@ -549,13 +563,13 @@ class NetworkLibraryImpl : public NetworkLibrary {
}

virtual void RequestWifiScan() {
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
RequestScan(TYPE_WIFI);
}
}

virtual bool GetWifiAccessPoints(WifiAccessPointVector* result) {
if (!CrosLibrary::Get()->EnsureLoaded())
if (!EnsureCrosLoaded())
return false;
DeviceNetworkList* network_list = GetDeviceNetworkList();
if (network_list == NULL)
Expand Down Expand Up @@ -583,7 +597,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
const std::string& password,
const std::string& identity,
const std::string& certpath) {
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
if (ConnectToNetworkWithCertInfo(network.service_path().c_str(),
password.empty() ? NULL : password.c_str(),
identity.empty() ? NULL : identity.c_str(),
Expand All @@ -608,7 +622,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
const std::string& identity,
const std::string& certpath,
bool auto_connect) {
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
// First create a service from hidden network.
ServiceInfo* service = GetWifiService(ssid.c_str(),
SECURITY_UNKNOWN);
Expand All @@ -631,7 +645,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
}

virtual void ConnectToCellularNetwork(CellularNetwork network) {
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
if (ConnectToNetwork(network.service_path().c_str(), NULL)) {
// Update local cache and notify listeners.
CellularNetwork* cellular = GetWirelessNetworkByPath(
Expand All @@ -646,13 +660,13 @@ class NetworkLibraryImpl : public NetworkLibrary {
}

virtual void RefreshCellularDataPlans(const CellularNetwork& network) {
if (!CrosLibrary::Get()->EnsureLoaded())
if (!EnsureCrosLoaded())
return;
RequestCellularDataPlanUpdate(network.service_path().c_str());
}

virtual void DisconnectFromWirelessNetwork(const WirelessNetwork& network) {
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
if (DisconnectFromNetwork(network.service_path().c_str())) {
// Update local cache and notify listeners.
if (network.type() == TYPE_WIFI) {
Expand Down Expand Up @@ -683,7 +697,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
*cellular = network;

// Update the cellular network with libcros.
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
SetAutoConnect(network.service_path().c_str(), network.auto_connect());
}
}
Expand All @@ -696,7 +710,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
*wifi = network;

// Update the wifi network with libcros.
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
SetPassphrase(
network.service_path().c_str(), network.passphrase().c_str());
SetIdentity(network.service_path().c_str(), network.identity().c_str());
Expand All @@ -706,7 +720,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
}

virtual void ForgetWirelessNetwork(const std::string& service_path) {
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
if (DeleteRememberedService(service_path.c_str())) {
// Update local cache and notify listeners.
remembered_wifi_networks_.erase(
Expand Down Expand Up @@ -759,7 +773,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
}

virtual void EnableOfflineMode(bool enable) {
if (!CrosLibrary::Get()->EnsureLoaded())
if (!EnsureCrosLoaded())
return;

// If network device is already enabled/disabled, then don't do anything.
Expand All @@ -782,7 +796,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
std::string* hardware_address) {
hardware_address->clear();
NetworkIPConfigVector ipconfig_vector;
if (!device_path.empty()) {
if (EnsureCrosLoaded() && !device_path.empty()) {
IPConfigStatus* ipconfig_status = ListIPConfigs(device_path.c_str());
if (ipconfig_status) {
for (int i = 0; i < ipconfig_status->size; i++) {
Expand Down Expand Up @@ -1038,7 +1052,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
}

void UpdateSystemInfo() {
if (CrosLibrary::Get()->EnsureLoaded()) {
if (EnsureCrosLoaded()) {
UpdateNetworkStatus();
}
}
Expand Down Expand Up @@ -1070,7 +1084,7 @@ class NetworkLibraryImpl : public NetworkLibrary {
}

void EnableNetworkDeviceType(ConnectionType device, bool enable) {
if (!CrosLibrary::Get()->EnsureLoaded())
if (!EnsureCrosLoaded())
return;

// If network device is already enabled/disabled, then don't do anything.
Expand Down

0 comments on commit 3dd5198

Please sign in to comment.