arc: Add InstanceHelper::GetInstanceForMethod()

components/arc/instance_holder.h

Index: components/arc/instance_holder.h
diff --git a/components/arc/instance_holder.h b/components/arc/instance_holder.h
index 4fb5b37023eac07e6303f0c9928c0e8752c8f547..4df2302593b571ac7c6edcbab826bdbc505a3dc2 100644
--- a/components/arc/instance_holder.h
+++ b/components/arc/instance_holder.h
@@ -42,6 +42,38 @@ class InstanceHolder {
   T* instance() const { return instance_; }
   uint32_t version() const { return version_; }

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

Can we remove this now? If not, what's preventing us from doing so?

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

GetIntentHelperInstanceWithErrorCode() still needs it to disambiguate.

 
+  // Gets the Mojo interface for the instance, but only if its reported version
+  // is at least |min_version|. Returns nullptr if the instance is either not
+  // ready or does not have the requested version, and logs appropriately.
+  T* GetInstanceForVersion(uint32_t min_version) {
+    if (!instance_) {
+      VLOG(2) << __func__ << " Instance not available.";
+      return nullptr;
+    }
+    if (version_ < min_version) {
+      VLOG(2) << __func__ << " Instance version mismatch. Expected "
+              << min_version << " got " << version_;
+      return nullptr;
+    }
+    return instance_;
+  }
+
+  // Same as the above, but with an explicit method name for better logging.
+  T* GetInstanceForVersion(uint32_t min_version, const char* method_name) {

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

Passing the method_name every time only for logging seems a bit tedious. I
already saw several typos during the review too. Can't we automagically query a
method name from |min_version| (or can't we modify the mojo C++ code generator
so that we can do it?) It'll be 1:N mapping and the logging will be a bit
verbose, though.

Or, querying a min version from a function pointer
(&mojom::...Instance::DoSomething) would be even better if at all possible. This
way, the mapping will be N:1.

wdyt?

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

We can't query the method name from |min_version| unfortunately (see the
bluetooth service, several methods were added in the same version). There's
nothing generated right now that we can use :( I can try some stuff out, but I'd
like to do that in a separate commit (if possible).

(There's also typeid, but I didn't pursue it since it seems to be unused in
Chromium).

[Yusuke Sato, 2016/09/17 01:38:56]

Acknowledged. I guess typeid() won't always work as Chromium is built with
-fno-rtti.

+    if (!instance_) {
+      VLOG(2) << __func__ << " Instance not available to call " << method_name
+              << ".";
+      return nullptr;
+    }
+    if (version_ < min_version) {
+      VLOG(2) << __func__ << " Instance version mismatch to call "
+              << method_name << ". Expected " << min_version << " got "
+              << version_;
+      return nullptr;
+    }
+    return instance_;
+  }
+
   // Adds or removes observers. This can only be called on the thread that this
   // class was created on. RemoveObserver does nothing if |observer| is not in
   // the list.