Make GCMDriver::GetInstanceIDHandler protected, to avoid misuse

components/gcm_driver/instance_id/instance_id.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_GCM_DRIVER_INSTANCE_ID_INSTANCE_ID_H_
 #define COMPONENTS_GCM_DRIVER_INSTANCE_ID_INSTANCE_ID_H_
 
 #include <map>
 #include <string>
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
 
 namespace gcm {
-class GCMDriver;
+class InstanceIDHandler;
 }  // namespace gcm
 
 namespace instance_id {
 
 // Encapsulates Instance ID functionalities that need to be implemented for
 // different platform. One instance is created per application. Life of
 // Instance ID is managed by the InstanceIdDriver.
 class InstanceID {
  public:
   enum Result {
@@ skipping 21 matching lines @@
       GetCreationTimeCallback;
   typedef base::Callback<void(const std::string& token,
                               Result result)> GetTokenCallback;
   typedef base::Callback<void(Result result)> DeleteTokenCallback;
   typedef base::Callback<void(Result result)> DeleteIDCallback;
 
   static const int kInstanceIDByteLength = 8;
 
   // Creator.
   // |app_id|: identifies the application that uses the Instance ID.
   // |gcm_driver|: driver to access the GCM functionalities needed to support

[Peter Beverloo, 2016/03/11 16:10:19]

Please update this comment.

[johnme, 2016/03/11 17:53:23]

Done.

-  //               Instance ID.
+  //               Instance ID. Its InstanceIDHandler must outlive this class.
   static scoped_ptr<InstanceID> Create(const std::string& app_id,
-                                       gcm::GCMDriver* gcm_driver);
+                                       gcm::InstanceIDHandler* handler);
 
   virtual ~InstanceID();
 
   // Sets the callback that will be invoked when the token refresh event needs
   // to be triggered.
   void SetTokenRefreshCallback(const TokenRefreshCallback& callback);
 
   // Returns the Instance ID.
   virtual void GetID(const GetIDCallback& callback) = 0;
 
@@ skipping 26 matching lines @@
                            const DeleteTokenCallback& callback) = 0;
 
   // Resets the app instance identifier and revokes all tokens associated with
   // it.
   // |callback|: to be called once the asynchronous operation is done.
   virtual void DeleteID(const DeleteIDCallback& callback) = 0;
 
   std::string app_id() const { return app_id_; }
 
  protected:
-  explicit InstanceID(const std::string& app_id);
+  InstanceID(const std::string& app_id, gcm::InstanceIDHandler* handler);
 
   void NotifyTokenRefresh(bool update_id);
 
+  gcm::InstanceIDHandler* handler() { return handler_; }

[Peter Beverloo, 2016/03/11 16:10:19]

Can this be const? (Like the method you're replacing.)

[johnme, 2016/03/11 17:53:22]

Done.

+
  private:
+  gcm::InstanceIDHandler* handler_;  // Not owned.

[Peter Beverloo, 2016/03/11 16:10:19]

Personally I would prefer documenting *why* it's safe to store it here. The fact
that it's not owned is implied by it not being a scoped_(ref)ptr.

[johnme, 2016/03/11 17:53:23]

Done: "
// Owned by GCMProfileServiceFactory, which is a dependency of
// InstanceIDProfileServiceFactory, which owns this."

ditto in instance_id_driver.h

   std::string app_id_;
   TokenRefreshCallback token_refresh_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(InstanceID);
 };
 
 }  // namespace instance_id
 
 #endif  // COMPONENTS_GCM_DRIVER_INSTANCE_ID_INSTANCE_ID_H_