Fixing memory leak in DistilledPagePrefs unittests

components/dom_distiller/core/distilled_page_prefs_unittests.cc

Index: components/dom_distiller/core/distilled_page_prefs_unittests.cc
diff --git a/components/dom_distiller/core/distilled_page_prefs_unittests.cc b/components/dom_distiller/core/distilled_page_prefs_unittests.cc
index 1c2ccf8e1729d6d63a4ba15f50cc7fe859bafeca..8497c3b9dc7a3589b866b8abca43249ac9fa6a69 100644
--- a/components/dom_distiller/core/distilled_page_prefs_unittests.cc
+++ b/components/dom_distiller/core/distilled_page_prefs_unittests.cc
@@ -32,44 +32,44 @@ class TestingObserver : public DistilledPagePrefs::Observer {
 class DistilledPagePrefsTest : public testing::Test {
  protected:
   virtual void SetUp() OVERRIDE {
-    user_prefs::TestingPrefServiceSyncable* pref_service =
-        new user_prefs::TestingPrefServiceSyncable();
-    DistilledPagePrefs::RegisterProfilePrefs(pref_service->registry());
-    distilled_page_prefs_ = new DistilledPagePrefs(pref_service);
+    pref_service_.reset(new user_prefs::TestingPrefServiceSyncable());
+    DistilledPagePrefs::RegisterProfilePrefs(pref_service_->registry());
+    distilled_page_prefs_.reset(new DistilledPagePrefs(pref_service_.get()));
   }
 
-  DistilledPagePrefs* distilled_page_prefs_;
+  scoped_ptr<DistilledPagePrefs> distilled_page_prefs_;

[nyquist, 2014/07/16 16:10:53]

I think the ordering of these two needs to be swapped, since DistilledPagePrefs
depend on TestingPrefServiceSyncable.

[smaslo, 2014/07/16 16:12:51]

Done.

+  scoped_ptr<user_prefs::TestingPrefServiceSyncable> pref_service_;
 
  private:
   base::MessageLoop message_loop_;
 };
 
 TEST_F(DistilledPagePrefsTest, TestingOnChangeThemeIsBeingCalled) {
-  TestingObserver* obs = new TestingObserver();
-  distilled_page_prefs_->AddObserver(obs);
+  TestingObserver obs;

[nyquist, 2014/07/16 16:02:57]

Remove this as an observer here and in the test below. Otherwise the lifetime of
this observer is shorter than the container for the observers.

[smaslo, 2014/07/16 16:06:50]

Done.

+  distilled_page_prefs_->AddObserver(&obs);
   distilled_page_prefs_->SetTheme(DistilledPagePrefs::SEPIA);
-  EXPECT_EQ(DistilledPagePrefs::LIGHT, obs->GetTheme());
+  EXPECT_EQ(DistilledPagePrefs::LIGHT, obs.GetTheme());
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(DistilledPagePrefs::SEPIA, obs->GetTheme());
+  EXPECT_EQ(DistilledPagePrefs::SEPIA, obs.GetTheme());
   distilled_page_prefs_->SetTheme(DistilledPagePrefs::DARK);
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(DistilledPagePrefs::DARK, obs->GetTheme());
+  EXPECT_EQ(DistilledPagePrefs::DARK, obs.GetTheme());
 }
 
 TEST_F(DistilledPagePrefsTest, TestingMultipleObservers) {
-  TestingObserver* obs = new TestingObserver();
-  distilled_page_prefs_->AddObserver(obs);
-  TestingObserver* obs2 = new TestingObserver();
-  distilled_page_prefs_->AddObserver(obs2);
+  TestingObserver obs;
+  distilled_page_prefs_->AddObserver(&obs);
+  TestingObserver obs2;
+  distilled_page_prefs_->AddObserver(&obs2);
   distilled_page_prefs_->SetTheme(DistilledPagePrefs::SEPIA);
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(DistilledPagePrefs::SEPIA, obs->GetTheme());
-  EXPECT_EQ(DistilledPagePrefs::SEPIA, obs2->GetTheme());
-  distilled_page_prefs_->RemoveObserver(obs);
+  EXPECT_EQ(DistilledPagePrefs::SEPIA, obs.GetTheme());
+  EXPECT_EQ(DistilledPagePrefs::SEPIA, obs2.GetTheme());
+  distilled_page_prefs_->RemoveObserver(&obs);
   distilled_page_prefs_->SetTheme(DistilledPagePrefs::LIGHT);
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(DistilledPagePrefs::SEPIA, obs->GetTheme());
-  EXPECT_EQ(DistilledPagePrefs::LIGHT, obs2->GetTheme());
+  EXPECT_EQ(DistilledPagePrefs::SEPIA, obs.GetTheme());
+  EXPECT_EQ(DistilledPagePrefs::LIGHT, obs2.GetTheme());
 }
 
 }  // namespace dom_distiller