PM: Switch RealShillprovider to use AsyncCopyVariables.
This change makes sense because shill values are now tied to
asynchronous DBus notifications. It also reduces much of the logic for
guarding attribute values, which is now handled internally by the
AsyncCopyVariable.
BUG=chromium:364997
TEST=Unit tests.
Change-Id: Ief5fffba821013f192a4029c2765c48bc2ed91c5
Reviewed-on: https://chromium-review.googlesource.com/196216
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/policy_manager/real_shill_provider.cc b/policy_manager/real_shill_provider.cc
index f6ac011..ffd851c 100644
--- a/policy_manager/real_shill_provider.cc
+++ b/policy_manager/real_shill_provider.cc
@@ -28,13 +28,6 @@
namespace chromeos_policy_manager {
-const char* RealShillProvider::kConnStatusUnavailable =
- "Connection status unavailable";
-const char* RealShillProvider::kConnTypeUnavailable =
- "Connection type unavailable";
-const char* RealShillProvider::kConnTetheringUnavailable =
- "Connection tethering mode unavailable";
-
RealShillProvider::~RealShillProvider() {
// Detach signal handler, free manager proxy.
dbus_->ProxyDisconnectSignal(manager_proxy_, shill::kMonitorPropertyChanged,
@@ -133,15 +126,14 @@
if (default_service_path_ == default_service_path_str)
return true;
- // Update the connection status, invalidate all attributes.
+ // Update the connection status.
default_service_path_ = default_service_path_str;
- is_connected_ = (default_service_path_ != "/");
- conn_last_changed_ = clock_->GetWallclockTime();
- conn_type_is_valid_ = false;
- conn_tethering_is_valid_ = false;
+ bool is_connected = (default_service_path_ != "/");
+ var_is_connected_.SetValue(is_connected);
+ var_conn_last_changed_.SetValue(clock_->GetWallclockTime());
- // If connected, update the connection attributes.
- if (is_connected_) {
+ // Update the connection attributes.
+ if (is_connected) {
DBusGProxy* service_proxy = GetProxy(default_service_path_.c_str(),
shill::kFlimflamServiceInterface);
GHashTable* hash_table = NULL;
@@ -153,9 +145,9 @@
shill::kPhysicalTechnologyProperty);
}
if (type_str) {
- conn_type_ = ParseConnectionType(type_str);
- conn_type_is_valid_ = true;
+ var_conn_type_.SetValue(ParseConnectionType(type_str));
} else {
+ var_conn_type_.UnsetValue();
LOG(ERROR) << "Could not find connection type ("
<< default_service_path_ << ")";
}
@@ -164,9 +156,9 @@
const char* tethering_str = GetStrProperty(hash_table,
shill::kTetheringProperty);
if (tethering_str) {
- conn_tethering_ = ParseConnectionTethering(tethering_str);
- conn_tethering_is_valid_ = true;
+ var_conn_tethering_.SetValue(ParseConnectionTethering(tethering_str));
} else {
+ var_conn_tethering_.UnsetValue();
LOG(ERROR) << "Could not find connection tethering mode ("
<< default_service_path_ << ")";
}
@@ -174,10 +166,11 @@
g_hash_table_unref(hash_table);
}
dbus_->ProxyUnref(service_proxy);
+ } else {
+ var_conn_type_.UnsetValue();
+ var_conn_tethering_.UnsetValue();
}
- // Mark the connection status as initialized.
- is_conn_status_init_ = true;
return true;
}
diff --git a/policy_manager/real_shill_provider.h b/policy_manager/real_shill_provider.h
index 833e147..cd5ee06 100644
--- a/policy_manager/real_shill_provider.h
+++ b/policy_manager/real_shill_provider.h
@@ -56,11 +56,6 @@
virtual bool DoInit() override;
private:
- // Default error strings for variables.
- static const char* kConnStatusUnavailable;
- static const char* kConnTypeUnavailable;
- static const char* kConnTetheringUnavailable;
-
// Return a DBus proxy for a given |path| and |interface| within shill.
DBusGProxy* GetProxy(const char* path, const char* interface);
@@ -77,23 +72,6 @@
static void HandlePropertyChangedStatic(DBusGProxy* proxy, const char* name,
GValue* value, void* data);
- // Whether the connection status has been properly initialized.
- bool is_conn_status_init_ = false;
-
- // The time when the connection type last changed.
- base::Time conn_last_changed_;
-
- // The current connection status.
- bool is_connected_;
-
- // The default connection type and whether its value is valid.
- ConnectionType conn_type_;
- bool conn_type_is_valid_ = false;
-
- // The default connection tethering mode and whether its value is valid.
- ConnectionTethering conn_tethering_;
- bool conn_tethering_is_valid_ = false;
-
// The current default service path, if connected.
std::string default_service_path_;
@@ -105,19 +83,11 @@
// A clock abstraction (mockable).
ClockInterface* const clock_;
- // The provider's variable.
- CopyVariable<bool> var_is_connected_{
- "is_connected", kVariableModePoll, is_connected_, &is_conn_status_init_,
- kConnStatusUnavailable};
- CopyVariable<ConnectionType> var_conn_type_{
- "conn_type", kVariableModePoll, conn_type_, &conn_type_is_valid_,
- kConnTypeUnavailable};
- CopyVariable<ConnectionTethering> var_conn_tethering_{
- "conn_tethering", kVariableModePoll, conn_tethering_,
- &conn_tethering_is_valid_, kConnTetheringUnavailable};
- CopyVariable<base::Time> var_conn_last_changed_{
- "conn_last_changed", kVariableModePoll, conn_last_changed_,
- &is_conn_status_init_, kConnStatusUnavailable};
+ // The provider's variables.
+ AsyncCopyVariable<bool> var_is_connected_{"is_connected"};
+ AsyncCopyVariable<ConnectionType> var_conn_type_{"conn_type"};
+ AsyncCopyVariable<ConnectionTethering> var_conn_tethering_{"conn_tethering"};
+ AsyncCopyVariable<base::Time> var_conn_last_changed_{"conn_last_changed"};
DISALLOW_COPY_AND_ASSIGN(RealShillProvider);
};