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

remoting/host/ack_or_timeout_reporter.cc

Index: remoting/host/ack_or_timeout_reporter.cc
diff --git a/remoting/host/ack_or_timeout_reporter.cc b/remoting/host/ack_or_timeout_reporter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..00ea7ce49335160eb55c7fcbb46b3f2cae3a3abe
--- /dev/null
+++ b/remoting/host/ack_or_timeout_reporter.cc
@@ -0,0 +1,100 @@
+// Copyright 2014 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.
+
+#include "remoting/host/ack_or_timeout_reporter.h"
+
+#include "base/bind.h"
+#include "base/callback.h"

[Lambros, 2014/12/05 22:19:48]

I don't think you need to repeat any #includes here that are already in
ack_or_timeout_reporter.h.

From https://code.google.com/p/include-what-you-use/
"either foo.cc or foo.h should #include a .h file that exports the declaration
of that symbol"
I interpret that as XOR.

[Łukasz Anforowicz, 2014/12/05 23:59:28]

I applied your forward-declarations comment in the .h file to both the Callback
and TimeDelta class (both only used via reference in the public declarations),
so I had to keep the include in the .cc file.

+#include "base/location.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sequence_checker.h"
+#include "base/sequenced_task_runner.h"

[Lambros, 2014/12/05 22:19:48]

Same here, can remove this.

[Łukasz Anforowicz, 2014/12/05 23:59:28]

Done.

+#include "base/time/time.h"
+
+namespace remoting {
+
+class AckOrTimeoutReporter : public base::RefCounted<AckOrTimeoutReporter> {
+ public:
+  AckOrTimeoutReporter(
+      scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner,
+      const base::Callback<void(AckOrTimeout)>& ack_or_timeout_callback);
+
+  void Execute(
+      const base::Callback<void(const base::Closure& ack)>& function_that_acks,
+      const base::TimeDelta& timeout);
+
+ private:
+  // Ref-counting helpers.

[Lambros, 2014/12/05 22:19:48]

You can remove this comment.

[Łukasz Anforowicz, 2014/12/05 23:59:27]

Done.

+  friend class base::RefCounted<AckOrTimeoutReporter>;
+  ~AckOrTimeoutReporter();
+
+  void OnAckOrTimeout(AckOrTimeout ack_or_timeout);
+
+  scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_;
+  base::Callback<void(AckOrTimeout)> ack_or_timeout_callback_;
+  base::WeakPtrFactory<AckOrTimeoutReporter> weak_ptr_factory_;
+  scoped_refptr<AckOrTimeoutReporter> self_;
+  base::SequenceChecker sequence_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(AckOrTimeoutReporter);
+};
+
+AckOrTimeoutReporter::AckOrTimeoutReporter(
+    scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner,
+    const base::Callback<void(AckOrTimeout)>& ack_or_timeout_callback)
+    : sequenced_task_runner_(sequenced_task_runner),
+      ack_or_timeout_callback_(ack_or_timeout_callback),
+      weak_ptr_factory_(this),
+      self_(this) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+}
+
+AckOrTimeoutReporter::~AckOrTimeoutReporter() {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+}
+
+void AckOrTimeoutReporter::Execute(
+    const base::Callback<void(const base::Closure& ack)>& function_that_acks,
+    const base::TimeDelta& timeout) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+
+  base::Closure timeout_callback =
+      base::Bind(&AckOrTimeoutReporter::OnAckOrTimeout,
+                 weak_ptr_factory_.GetWeakPtr(), AckOrTimeout::Timeout);
+  base::Closure ack_callback =
+      base::Bind(&AckOrTimeoutReporter::OnAckOrTimeout,
+                 weak_ptr_factory_.GetWeakPtr(), AckOrTimeout::Ack);
+
+  sequenced_task_runner_->PostDelayedTask(FROM_HERE, timeout_callback, timeout);
+  function_that_acks.Run(ack_callback);
+}
+
+void AckOrTimeoutReporter::OnAckOrTimeout(AckOrTimeout ack_or_timeout) {
+  DCHECK(sequence_checker_.CalledOnValidSequencedThread());
+
+  // Make sure |ack_callback| and |timeout_callback| created
+  // in |Execute| method become no-ops that are safe to be invoked
+  // after |this| is no longer alive.
+  // This is done as the very first thing here, to make sure
+  // |ack_callback| and |timeout_callback| won't be called
+  // again no matter what happens next (i.e. as a consequence
+  // of |ack_or_timeout_callback_|.
+  weak_ptr_factory_.InvalidateWeakPtrs();
+
+  ack_or_timeout_callback_.Run(ack_or_timeout);
+  self_ = nullptr;
+}
+
+void ReportAckOrTimeout(
+    const base::Callback<void(const base::Closure&)>& function_that_acks,
+    const base::TimeDelta& timeout,
+    scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner,
+    const base::Callback<void(AckOrTimeout)>& ack_or_timeout_callback) {
+  scoped_refptr<AckOrTimeoutReporter> reporter(
+      new AckOrTimeoutReporter(sequenced_task_runner, ack_or_timeout_callback));
+  reporter->Execute(function_that_acks, timeout);
+}
+
+}  // namespace remoting