Reporting of policy errors via host-offline-reason: part 2b

remoting/host/remoting_me2me_host.cc

 // Copyright (c) 2012 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.
 //
 // This file implements a standalone host process for Me2Me.
 
 #include <string>
 
 #include "base/at_exit.h"
 #include "base/bind.h"
@@ skipping 124 matching lines @@
 const char kStdinConfigPath[] = "-";
 
 const char kWindowIdSwitchName[] = "window-id";
 
 }  // namespace
 
 namespace remoting {
 
 class HostProcess
     : public ConfigWatcher::Delegate,
-      public HeartbeatSender::Listener,
       public HostChangeNotificationListener::Listener,
       public IPC::Listener,
       public base::RefCountedThreadSafe<HostProcess> {
  public:
   HostProcess(scoped_ptr<ChromotingHostContext> context,
               int* exit_code_out);
 
   // ConfigWatcher::Delegate interface.
   virtual void OnConfigUpdated(const std::string& serialized_config) override;
   virtual void OnConfigWatcherError() override;
 
   // IPC::Listener implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) override;
   virtual void OnChannelError() override;
 
-  // HeartbeatSender::Listener overrides.
-  virtual void OnHeartbeatSuccessful() override;
-  virtual void OnUnknownHostIdError() override;
+  // HeartbeatSender callbacks

[Lambros, 2014/11/18 00:19:30]

nit: period.
Actually, I'm not sure you need this comment? Also, do these methods need to be
public?

[Łukasz Anforowicz, 2014/11/18 01:03:22]

Yes - good point.  These methods don't need to be public after the changes.

+  void OnHeartbeatSuccessful();
+  void OnUnknownHostIdError();
 
   // HostChangeNotificationListener::Listener overrides.
   virtual void OnHostDeleted() override;
 
   // Initializes the pairing registry on Windows.
   void OnInitializePairingRegistry(
       IPC::PlatformFileForTransit privileged_key,
       IPC::PlatformFileForTransit unprivileged_key);
 
  private:
@@ skipping 1141 matching lines @@
     frame_recorder_extension->SetMaxContentBytes(frame_recorder_buffer_size_);
     host_->AddExtension(frame_recorder_extension.Pass());
   }
 
   // TODO(simonmorris): Get the maximum session duration from a policy.
 #if defined(OS_LINUX)
   host_->SetMaximumSessionDuration(base::TimeDelta::FromHours(20));
 #endif
 
   heartbeat_sender_.reset(new HeartbeatSender(
-      this, host_id_, signal_strategy_.get(), key_pair_,
-      directory_bot_jid_));
+      base::Bind(&HostProcess::OnHeartbeatSuccessful, base::Unretained(this)),

[Lambros, 2014/11/18 00:19:30]

Why do we need base::Unretained here? This is a ref-counted object.

[Łukasz Anforowicz, 2014/11/18 01:03:22]

This temporarily follows other references to |this| from this file.  I am
changing to WeakPtr in part#3.

[Lambros, 2014/11/18 02:33:52]

I see. I'm curious to know if this still compiles without the base::Unretained?
But OK to keep it for now, since it's used several times in this file already.

[Łukasz Anforowicz, 2014/11/18 17:30:45]

It would compile without base::Unretained and would even work, but would mean
that HeartbeatSender (by holding the callbacks) is extending the lifetime of
HostProcess - this different from current code and I don't want to change it
unnecessarily (although is probably fine as HostProcess explicitly destroys
HeartbeatSender in one of Shutdown methods; but then again HeartbeatSender will
outlive HostProcess after part#3, so I rather won't muck with this
unnecessarily).

+      base::Bind(&HostProcess::OnUnknownHostIdError, base::Unretained(this)),
+      host_id_, signal_strategy_.get(), key_pair_, directory_bot_jid_));
 
   host_change_notification_listener_.reset(new HostChangeNotificationListener(
       this, host_id_, signal_strategy_.get(), directory_bot_jid_));
 
   host_status_logger_.reset(
       new HostStatusLogger(host_->AsWeakPtr(), ServerLogEntry::ME2ME,
                            signal_strategy_.get(), directory_bot_jid_));
 
   // Set up reporting the host status notifications.
 #if defined(REMOTING_MULTI_PROCESS)
@@ skipping 143 matching lines @@
   int exit_code = kSuccessExitCode;
   new HostProcess(context.Pass(), &exit_code);
 
   // Run the main (also UI) message loop until the host no longer needs it.
   message_loop.Run();
 
   return exit_code;
 }
 
 }  // namespace remoting