arc: Add InstanceHelper::GetInstanceForMethod()

components/arc/net/arc_net_host_impl.cc

Index: components/arc/net/arc_net_host_impl.cc
diff --git a/components/arc/net/arc_net_host_impl.cc b/components/arc/net/arc_net_host_impl.cc
index 09caeb9c66c59b912175ddb08bfb8731a2972c43..576e08b09c8fd2b8afbcac45a3bad33f2a31feaf 100644
--- a/components/arc/net/arc_net_host_impl.cc
+++ b/components/arc/net/arc_net_host_impl.cc
@@ -27,7 +27,10 @@
 
 namespace {
 
-const int kGetNetworksListLimit = 100;
+constexpr int kGetNetworksListLimit = 100;
+constexpr uint32_t kDefaultMinInstanceVersion = 0;

[Yusuke Sato, 2016/09/16 23:58:49]

same, remove?

[Luis Héctor Chávez, 2016/09/17 00:30:53]

Acknowledged.

+constexpr uint32_t kDefaultNetworkChangedMinInstanceVersion = 2;
+constexpr uint32_t kDeviceListChangedMinInstanceVersion = 3;

[Yusuke Sato, 2016/09/16 23:58:49]

I think this should be renamed to kWifiEnabledStateChanged.

[Luis Héctor Chávez, 2016/09/17 00:30:53]

Done.

 
 chromeos::NetworkStateHandler* GetStateHandler() {
   return chromeos::NetworkHandler::Get()->network_state_handler();
@@ -572,16 +575,12 @@ void ArcNetHostImpl::StartScan() {
 }
 
 void ArcNetHostImpl::ScanCompleted(const chromeos::DeviceState* /*unused*/) {
-  if (!arc_bridge_service()->net()->instance()) {
-    VLOG(2) << "NetInstance not ready yet";
+  auto* net_instance = arc_bridge_service()->net()->GetInstanceForVersion(
+      kDefaultMinInstanceVersion, "ScanCompleted");
+  if (!net_instance)
     return;
-  }
-  if (arc_bridge_service()->net()->version() < 1) {
-    VLOG(1) << "NetInstance does not support ScanCompleted.";
-    return;
-  }
 
-  arc_bridge_service()->net()->instance()->ScanCompleted();
+  net_instance->ScanCompleted();
 }
 
 void ArcNetHostImpl::GetDefaultNetwork(
@@ -605,26 +604,25 @@ void ArcNetHostImpl::GetDefaultNetwork(
 void ArcNetHostImpl::DefaultNetworkSuccessCallback(
     const std::string& service_path,
     const base::DictionaryValue& dictionary) {
-  if (!arc_bridge_service()->net()->instance()) {
-    VLOG(2) << "NetInstance is null.";
+  auto* net_instance = arc_bridge_service()->net()->GetInstanceForVersion(
+      kDefaultMinInstanceVersion, "GetDefaultNetwork");

[Yusuke Sato, 2016/09/16 23:58:49]

* Not sure why kDefaultMinInstanceVersion is used here. I believe this should be
kDefaultNetworkChangedMinInstanceVersion.
* Also, the second param should be "DefaultNetworkChanged"?

[Luis Héctor Chávez, 2016/09/17 00:30:53]

The thing is that there are two "optional" parameters to this function: the
version (default to 0), and the method name (default to nullptr).  I didn't want
to create 4 very similar functions, so adding the version=0 was a compromise.

I can also move the version to a template parameter (with a default of 0 so it
can be omitted), but i'm not sure if that's more or less readable.

+  if (!net_instance)
     return;
-  }
-  arc_bridge_service()->net()->instance()->DefaultNetworkChanged(
-      TranslateONCConfiguration(&dictionary),
-      TranslateONCConfiguration(&dictionary));
+
+  net_instance->DefaultNetworkChanged(TranslateONCConfiguration(&dictionary),
+                                      TranslateONCConfiguration(&dictionary));
 }
 
 void ArcNetHostImpl::DefaultNetworkChanged(
     const chromeos::NetworkState* network) {
-  if (arc_bridge_service()->net()->version() < 2) {
-    VLOG(1) << "ArcBridgeService does not support DefaultNetworkChanged.";
+  auto* net_instance = arc_bridge_service()->net()->GetInstanceForVersion(

[Yusuke Sato, 2016/09/16 23:58:49]

I think you can move this to line 624.

[Luis Héctor Chávez, 2016/09/17 00:30:54]

Done.

+      kDefaultNetworkChangedMinInstanceVersion, "DefaultNetworkChanged");
+  if (!net_instance)
     return;
-  }
 
   if (!network) {
     VLOG(1) << "No default network";
-    arc_bridge_service()->net()->instance()->DefaultNetworkChanged(nullptr,
-                                                                   nullptr);
+    net_instance->DefaultNetworkChanged(nullptr, nullptr);
     return;
   }
 
@@ -638,14 +636,14 @@ void ArcNetHostImpl::DefaultNetworkChanged(
 }
 
 void ArcNetHostImpl::DeviceListChanged() {
-  if (arc_bridge_service()->net()->version() < 3) {
-    VLOG(1) << "ArcBridgeService does not support DeviceListChanged.";
+  auto* net_instance = arc_bridge_service()->net()->GetInstanceForVersion(
+      kDeviceListChangedMinInstanceVersion, "DeviceListChanged");

[Yusuke Sato, 2016/09/16 23:58:49]

      kWifiEnabledStateChanged, "WifiEnabledStateChanged");

[Luis Héctor Chávez, 2016/09/17 00:30:53]

Done.

+  if (!net_instance)
     return;
-  }
 
   bool is_enabled = GetStateHandler()->IsTechnologyEnabled(
       chromeos::NetworkTypePattern::WiFi());
-  arc_bridge_service()->net()->instance()->WifiEnabledStateChanged(is_enabled);
+  net_instance->WifiEnabledStateChanged(is_enabled);
 }
 
 void ArcNetHostImpl::OnShuttingDown() {