Always use arc::InstanceHolder<T>::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 06e581a8140d594edfcac129e6ddd38cbbef6ab6..9787ca730ea43226bc26a48ea9efc0d0f799d49d 100644
--- a/components/arc/instance_holder.h
+++ b/components/arc/instance_holder.h
@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_ARC_INSTANCE_HOLDER_H_
 #define COMPONENTS_ARC_INSTANCE_HOLDER_H_
 
+#include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ -35,18 +36,20 @@ class InstanceHolder {
 
   InstanceHolder() = default;
 
-  // Gets the Mojo interface for all the instance services. This will return
-  // nullptr if that particular service is not ready yet. Use an Observer if you
-  // want to be notified when this is ready. This can only be called on the
-  // thread that this class was created on.
-  T* instance() const { return instance_; }
-  uint32_t version() const { return version_; }
+  // Returns true if the Mojo interface is ready at least for its version 0
+  // interface. Use an Observer if you want to be notified when this is ready.
+  // This can only be called on the thread that this class was created on.
+  bool HasInstance() const { return instance_; }

[Luis Héctor Chávez, 2016/09/23 22:51:04]

nit: has_instance()

[Yusuke Sato, 2016/09/24 00:15:22]

Done.

+
+  // Gets the version of the instance. Use this only for logging purposes. To
+  // get an instance pointer with a version check, use GetInstanceForMethod().
+  uint32_t GetVersionForLogging() const { return version_; }

[Luis Héctor Chávez, 2016/09/23 22:51:04]

This one is fine since we want to discourage its use.

[Luis Héctor Chávez, 2016/09/23 23:22:45]

Oh wait, it's not being used anywhere! Let's get rid of it :) That way nobody
will even have the temptation.

[Yusuke Sato, 2016/09/24 00:15:22]

Ok, removed.

[Yusuke Sato, 2016/09/24 00:15:22]

Acknowledged.

 
   // Gets the Mojo interface that's intended to call for
   // |method_name_for_logging|, 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* GetInstanceForMethod(const char* method_name_for_logging,
+  T* GetInstanceForMethod(const std::string& method_name_for_logging,
                           uint32_t min_version) {
     if (!instance_) {
       VLOG(1) << "Instance for " << T::Name_ << "::" << method_name_for_logging
@@ -54,16 +57,17 @@ class InstanceHolder {
       return nullptr;
     }
     if (version_ < min_version) {
-      VLOG(1) << "Instance for " << T::Name_ << "::" << method_name_for_logging
-              << " version mismatch. Expected " << min_version << " got "
-              << version_;
+      LOG(ERROR) << "Instance for " << T::Name_
+                 << "::" << method_name_for_logging
+                 << " version mismatch. Expected " << min_version << " got "
+                 << version_;
       return nullptr;
     }
     return instance_;
   }
 
   // Same as the above, but for the version zero.
-  T* GetInstanceForMethod(const char* method_name_for_logging) {
+  T* GetInstanceForMethod(const std::string& method_name_for_logging) {
     return GetInstanceForMethod(method_name_for_logging, 0u);
   }
 
@@ -74,7 +78,7 @@ class InstanceHolder {
     DCHECK(thread_checker_.CalledOnValidThread());
     observer_list_.AddObserver(observer);
 
-    if (instance())
+    if (instance_)
       observer->OnInstanceReady();
   }