Fix DCHECK, memory leak, and refactor PasswordStore to use CancelableRequest

base/scoped_vector_unittest.cc

+// Copyright (c) 2011 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 "base/observer_list.h"
+#include "base/scoped_vector.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+// The LifeCycleObject notifies its observer upon construction & deletion.  We
+// only really need one observer, but use the standard ObserverList pattern.

[James Hawkins, 2011/03/21 01:45:06]

Why?

[Sheridan Rawlins, 2011/03/21 05:20:17]

I was exploring use of ObserverList and thought it made sense, but I suppose it
does seem like overkill.  I'll simplify with one observer_.

Done.

+class LifeCycleObject {
+ public:
+  class Observer {
+   public:

[James Hawkins, 2011/03/21 01:45:06]

protected:
 virtual ~Observer() {}

[Sheridan Rawlins, 2011/03/21 05:20:17]

You know, I was going to do that, but I saw another interface which didn't have
a destructor...  Good lesson/confirmation - will always have virtual destructors
as private as possible.

Done.

+    virtual void OnLifeCycleConstruct(LifeCycleObject* o) = 0;
+    virtual void OnLifeCycleDestroy(LifeCycleObject* o) = 0;
+  };
+
+  explicit LifeCycleObject(Observer* observer) {
+    observer_list_.AddObserver(observer);
+    FOR_EACH_OBSERVER(Observer, observer_list_, OnLifeCycleConstruct(this));
+  }
+  ~LifeCycleObject() {
+    FOR_EACH_OBSERVER(Observer, observer_list_, OnLifeCycleDestroy(this));
+  }
+
+ private:
+  ObserverList<Observer> observer_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(LifeCycleObject);
+};
+
+// The life cycle states we care about for the purposes of testing Scoped
+// Vector against objects.
+enum LifeCycleState {
+  LC_INITIAL,
+  LC_CONSTRUCTED,
+  LC_DESTROYED,
+};
+
+// Because we wish to watch the life cycle of an object being constructed and
+// destoyed, and further wish to test expectations against the state of that

[James Hawkins, 2011/03/21 01:45:06]

s/destoyed/destroyed/

[Sheridan Rawlins, 2011/03/21 05:20:17]

Done.

+// object, we cannot save state in that object itself.  Instead, we use this
+// pairing of the watcher which observes the object, which notifies of

[James Hawkins, 2011/03/21 01:45:06]

s/object, which/object then/ ?

[Sheridan Rawlins, 2011/03/21 05:20:17]

Done.

+// construction & destruction.
+// Since we also may be testing things which don't get freed, this class also
+// acts like a scoping object and deletes the |constructed_life_cycle_object_|,
+// if any when the LifeCycleWatcher is destroyed.
+// To keep this simple, the only expected state changes are
+// INITIAL -> CONSTRUCTED -> DESTROYED.
+// Anything more complicated than that should start another test.
+class LifeCycleWatcher : public LifeCycleObject::Observer {
+ public:
+  LifeCycleWatcher()
+      : life_cycle_state_(LC_INITIAL),
+        constructed_life_cycle_object_(NULL) {}
+  ~LifeCycleWatcher() {
+    if (constructed_life_cycle_object_)
+      delete constructed_life_cycle_object_;
+  }
+
+  // Assert INITIAL -> CONSTRUCTED and no LifeCycleObject associated with this
+  // LifeCycleWatcher.
+  virtual void OnLifeCycleConstruct(LifeCycleObject* object) {
+    ASSERT_EQ(LC_INITIAL, life_cycle_state_);
+    ASSERT_EQ(NULL, constructed_life_cycle_object_);
+    life_cycle_state_ = LC_CONSTRUCTED;
+    constructed_life_cycle_object_ = object;
+  }
+
+  // Assert CONSTRUCTED -> DESTROYED and the |object| being destroyed is the
+  // same one we saw constructed.
+  virtual void OnLifeCycleDestroy (LifeCycleObject* object) {

[James Hawkins, 2011/03/21 01:45:06]

Remove space before opening parenthesis.

[Sheridan Rawlins, 2011/03/21 05:20:17]

Done.

+    ASSERT_EQ(LC_CONSTRUCTED, life_cycle_state_);
+    ASSERT_EQ(constructed_life_cycle_object_, object);
+    life_cycle_state_ = LC_DESTROYED;
+    constructed_life_cycle_object_ = NULL;
+  }
+
+  LifeCycleState life_cycle_state() const { return life_cycle_state_; }
+
+  // Factory method for creating a new LifeCycleObject tied to this
+  // LifeCycleWatcher.
+  LifeCycleObject* NewLifeCycleObject() {
+    return new LifeCycleObject(this);
+  }
+
+ private:
+  LifeCycleState life_cycle_state_;
+  LifeCycleObject* constructed_life_cycle_object_;

[James Hawkins, 2011/03/21 01:45:06]

scoped_ptr

[Sheridan Rawlins, 2011/03/21 05:20:17]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(LifeCycleWatcher);
+};
+
+}  // namespace

[James Hawkins, 2011/03/21 01:45:06]

Wrap the rest of the file in the anonymous namespace.

[Sheridan Rawlins, 2011/03/21 05:20:17]

Man, I really copy/pasted some mediocrity from scoped_ptr.h

Done.

+
+TEST(ScopedVectorTest, LifeCycleWatcher) {
+  LifeCycleWatcher watcher;
+  EXPECT_EQ(LC_INITIAL, watcher.life_cycle_state());
+  LifeCycleObject* object = watcher.NewLifeCycleObject();
+  EXPECT_EQ(LC_CONSTRUCTED, watcher.life_cycle_state());
+  delete object;
+  EXPECT_EQ(LC_DESTROYED, watcher.life_cycle_state());
+}
+
+TEST(ScopedVectorTest, Reset) {
+  LifeCycleWatcher watcher;
+  EXPECT_EQ(LC_INITIAL, watcher.life_cycle_state());
+  ScopedVector<LifeCycleObject> scoped_vector;
+  scoped_vector.push_back(watcher.NewLifeCycleObject());
+  EXPECT_EQ(LC_CONSTRUCTED, watcher.life_cycle_state());
+  scoped_vector.reset();
+  EXPECT_EQ(LC_DESTROYED, watcher.life_cycle_state());
+}
+
+TEST(ScopedVectorTest, Scope) {
+  LifeCycleWatcher watcher;
+  EXPECT_EQ(LC_INITIAL, watcher.life_cycle_state());
+  {
+    ScopedVector<LifeCycleObject> scoped_vector;
+    scoped_vector.push_back(watcher.NewLifeCycleObject());
+    EXPECT_EQ(LC_CONSTRUCTED, watcher.life_cycle_state());
+  }
+  EXPECT_EQ(LC_DESTROYED, watcher.life_cycle_state());
+}
+
+TEST(ScopedVectorTest, InsertRange) {
+  LifeCycleWatcher watchers[5];
+
+  std::vector<LifeCycleObject*> vec;
+  for(LifeCycleWatcher* it = watchers; it != watchers + arraysize(watchers);
+      ++it) {
+    EXPECT_EQ(LC_INITIAL, it->life_cycle_state());
+    vec.push_back(it->NewLifeCycleObject());
+    EXPECT_EQ(LC_CONSTRUCTED, it->life_cycle_state());
+  }
+  // Start scope for ScopedVector.
+  {
+    ScopedVector<LifeCycleObject> scoped_vector;
+    scoped_vector.insert(scoped_vector.end(), vec.begin() + 1, vec.begin() + 3);
+    for(LifeCycleWatcher* it = watchers; it != watchers + arraysize(watchers);
+        ++it)
+      EXPECT_EQ(LC_CONSTRUCTED, it->life_cycle_state());
+  }
+  for(LifeCycleWatcher* it = watchers; it != watchers + 1; ++it)
+    EXPECT_EQ(LC_CONSTRUCTED, it->life_cycle_state());
+  for(LifeCycleWatcher* it = watchers + 1; it != watchers + 3; ++it)
+    EXPECT_EQ(LC_DESTROYED, it->life_cycle_state());
+  for(LifeCycleWatcher* it = watchers + 3; it != watchers + arraysize(watchers);
+      ++it)
+    EXPECT_EQ(LC_CONSTRUCTED, it->life_cycle_state());
+}