Support auto-reload on errors.

chrome/renderer/net/mockable_one_shot_timer_unittest.cc

+// Copyright 2013 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 "chrome/renderer/net/mockable_one_shot_timer.h"
+
+#include "base/message_loop/message_loop.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+void BumpCounter(int *counter) {
+  (*counter)++;
+}
+
+}  // namespace

[mmenke, 2014/03/11 20:07:54]

Can't this entire file be put in the anonymous namespace?

[Elly Fong-Jones, 2014/03/12 15:08:58]

Done.

+
+TEST(MockableOneShotTimerTest, TimerRuns) {
+  base::MessageLoop loop;
+  int calls = 0;
+  MockableOneShotTimer timer;
+  timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(0),
+              base::Bind(&BumpCounter,
+                         base::Unretained(&calls)));
+  base::MessageLoop::current()->RunUntilIdle();
+  EXPECT_EQ(1, calls);
+}
+
+class Incrementer {

[mmenke, 2014/03/11 20:07:54]

I think this is very hard to follow.  How about something like:

class HasWeakPtr : public SupportsWeakPtr<HasWeakPtr> {
  // Do standard weak ptr setup here.
}

void RestartTimer(HasWeakPtr* has_weak_ptr, ..., MockableOneShotTimer* timer) {
  base::WeakPtr<HasWeakPtr> weak_ptr(has_weak_ptr->AsWeakPtr());
  timer->Start(...);
  // If starting the timer did not destroy the callback calling this function,
  // |has_weak_ptr| will not have been deleted.
  ASSERT_TRUE(weak_ptr.get());
}

And use base::Owned(has_weak_ptr) to call the function.  Can grab another weak
ptr for has_weak_ptr to make sure it's destroyed, though not sure that's needed
(Tests are run with ASAN)

[mmenke, 2014/03/12 13:19:42]

Could actually get away with just using a WeakFactory<int>, instead of a new
class, though a new class may be clearer.

[Elly Fong-Jones, 2014/03/12 15:08:58]

Done.

[Elly Fong-Jones, 2014/03/12 15:08:58]

Done.

+ public:
+  Incrementer(int* counter) : counter_(counter) {}

[mmenke, 2014/03/11 20:07:54]

explicit

[Elly Fong-Jones, 2014/03/12 15:08:58]

Deleted.

+  ~Incrementer() { (*counter_)++; }
+  void set_counter(int* counter) { counter_ = counter; }
+ private:

[mmenke, 2014/03/11 20:07:54]

Blank line before private.

[Elly Fong-Jones, 2014/03/12 15:08:58]

Deleted.

+  int* counter_;

[mmenke, 2014/03/11 20:07:54]

DISALLOW_COPY_AND_ASSIGN

[Elly Fong-Jones, 2014/03/12 15:08:58]

Deleted.

+};
+
+namespace {
+
+void AllDone(int* counter) {

[mmenke, 2014/03/11 20:07:54]

Do we really need both this and BumpCounter?

[Elly Fong-Jones, 2014/03/12 15:08:58]

Deleted.

+  (*counter)++;
+}
+
+void RestartTimer(int* counter, int* new_counter, Incrementer* incrementer,
+                  MockableOneShotTimer* timer) {
+  (*counter)++;
+  timer->Start(FROM_HERE, base::TimeDelta::FromMilliseconds(0),
+               base::Bind(&AllDone,
+                          base::Unretained(counter)));
+  incrementer->set_counter(new_counter);
+};
+
+}
+
+// Test that MockableOneShotTimer does not destroy the callback while the
+// callback is still being run. Create a closure that owns an Incrementer (see
+// above) and keep track of whether the Incrementer is destroyed "early" while
+// the callback is running or not. Also, ensure that the timer executes both
+// RestartTimer() and AllDone().
+TEST(MockableOneShotTimerTest, NoDestructionAtStart) {
+  base::MessageLoop loop;
+  int early_destructions = 0;
+  int destructions = 0;
+  int calls = 0;
+  MockableOneShotTimer timer;
+  // If |incrementer| is destroyed, it will increment |early_destructions|;
+  // |RestartTimer| is responsible for swapping its destruction counter from
+  // |early_destructions| to |destructions|.
+  Incrementer* incrementer = new Incrementer(&early_destructions);
+  timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(0),
+              base::Bind(&RestartTimer,
+                         base::Unretained(&calls),
+                         base::Unretained(&destructions),
+                         base::Owned(incrementer),
+                         base::Unretained(&timer)));
+  base::MessageLoop::current()->RunUntilIdle();
+  // Both |AllDone| and |RestartTimer| must have run.
+  EXPECT_EQ(2, calls);
+  EXPECT_EQ(0, early_destructions);
+  EXPECT_EQ(1, destructions);
+}