Always use arc::InstanceHolder<T>::GetInstanceForMethod

components/arc/instance_holder.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_ARC_INSTANCE_HOLDER_H_
 #define COMPONENTS_ARC_INSTANCE_HOLDER_H_
 
+#include <string>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/threading/thread_checker.h"
 
 namespace arc {
 
 // Holds a Mojo instance+version pair. This also allows for listening for state
@@ skipping 10 matching lines @@
 
     // Called when the connection to the instance is closed.
     virtual void OnInstanceClosed() {}
 
    protected:
     virtual ~Observer() = default;
   };
 
   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
               << " not available.";
       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);
   }
 
   // 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.
   void AddObserver(Observer* observer) {
     DCHECK(thread_checker_.CalledOnValidThread());
     observer_list_.AddObserver(observer);
 
-    if (instance())
+    if (instance_)
       observer->OnInstanceReady();
   }
 
   void RemoveObserver(Observer* observer) {
     DCHECK(thread_checker_.CalledOnValidThread());
     observer_list_.RemoveObserver(observer);
   }
 
   // Sets |instance| with |version|.
   // This can be called in both case; on ready, and on closed.
@@ skipping 24 matching lines @@
 
   base::ThreadChecker thread_checker_;
   base::ObserverList<Observer> observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(InstanceHolder<T>);
 };
 
 }  // namespace arc
 
 #endif  // COMPONENTS_ARC_INSTANCE_HOLDER_H_