Reporting of policy errors via host-offline-reason: part 3

remoting/host/heartbeat_sender.h

Index: remoting/host/heartbeat_sender.h
diff --git a/remoting/host/heartbeat_sender.h b/remoting/host/heartbeat_sender.h
index 73080be9a6d3683cf1cb3c2869214f3477206fe9..3451e89c3ab04fb2def5c1806f9dcc6e8a3648d5 100644
--- a/remoting/host/heartbeat_sender.h
+++ b/remoting/host/heartbeat_sender.h
@@ -95,7 +95,7 @@ class HeartbeatSender : public SignalStrategy::Listener {
                   const base::Closure& on_unknown_host_id_error,
                   const std::string& host_id,
                   SignalStrategy* signal_strategy,
-                  scoped_refptr<RsaKeyPair> key_pair,
+                  const scoped_refptr<RsaKeyPair> key_pair,
                   const std::string& directory_bot_jid);
   ~HeartbeatSender() override;
 
@@ -140,7 +140,7 @@ class HeartbeatSender : public SignalStrategy::Listener {
   base::Closure on_unknown_host_id_error_;
   std::string host_id_;
   SignalStrategy* signal_strategy_;
-  scoped_refptr<RsaKeyPair> key_pair_;
+  const scoped_refptr<RsaKeyPair> key_pair_;

[Lambros, 2014/12/02 03:35:22]

Why change this?

[Łukasz Anforowicz, 2014/12/02 20:08:01]

The assumption here is that presence of "const" is good:
1) It allows the callers to pass "const" arguments.
2) If it compiles it is probably safe (although C++ doesn't give any formal
guarantees / allows to cast the constness away).

Allowing callers to pass "const" arguments is desirable, because this:
1A) enables their callers to pass "const" arguments
1B) const arguments are sometimes unavoidable (i.e. retrieved from another part
of the system that we cannot or don't want to change) and casting constness away
is yucky
1C) presence of "const" is a good code readability aid (const = I promise I
won't modify/delete this pointer).

Anyway - this is unrelated to core changes here, but this is a small, beneficial
refactoring, so I think this is ok.

[Lambros, 2014/12/03 03:20:24]

But this field is a value type (not a reference or pointer type). Callers can
still pass "const" arguments even if the field is not const - it will be
assigned by value.

Adding "const" will discourage future maintainers from making certain changes
(for example, moving the assignment of this field out of the ctor into an Init()
method), for no obvious benefit.

I suspect what you really want is scoped_refptr<const RsaKeyPair> ?

[Łukasz Anforowicz, 2014/12/03 17:54:21]

You're right.  I don't know what I was thinking.

   std::string directory_bot_jid_;
   scoped_ptr<IqSender> iq_sender_;
   scoped_ptr<IqRequest> request_;