Profile shouldn't own PersonalDataManager

chrome/browser/autofill/personal_data_manager_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 <string>
 
 #include "base/basictypes.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill_common_test.h"
 #include "chrome/browser/autofill/autofill_profile.h"
 #include "chrome/browser/autofill/form_structure.h"
 #include "chrome/browser/autofill/personal_data_manager.h"
 #include "chrome/browser/autofill/personal_data_manager_observer.h"
 #include "chrome/browser/password_manager/encryptor.h"
 #include "chrome/common/guid.h"
@@ skipping 35 matching lines @@
     db_thread_.Start();
 
     profile_.reset(new TestingProfile);
     profile_->CreateWebDataService(false);
 
     autofill_test::DisableSystemServices(profile_.get());
     ResetPersonalDataManager();
   }
 
   virtual void TearDown() {
-    personal_data_ = NULL;
-    if (profile_.get())
-      profile_.reset(NULL);
+    personal_data_.reset(NULL);
+    profile_.reset(NULL);

[Ilya Sherman, 2011/09/23 03:56:34]

nit: Are these calls to reset() really necessary?  They seem kind of redundant
with the use of a scoped_ptr.  In fact... does profile_ even need to be a
pointer?

[dhollowa, 2011/09/23 16:30:03]

Yes, these are necessary.  Destruction order needs to be imposed specifically.

[Ilya Sherman, 2011/09/23 21:41:38]

Ok, in that case, please add a comment noting that this code is needed to impose
the destruction order.

[dhollowa, 2011/09/23 22:00:58]

Done.

[GeorgeY, 2011/09/23 22:53:27]

FYI, the order of the destruction is reverse to order of initialization, which
is *enforced* by gcc (not by MSVS though).
On 2011/09/23 22:00:58, dhollowa wrote:

 
     db_thread_.Stop();
     MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask);
     MessageLoop::current()->Run();
   }
 
   void ResetPersonalDataManager() {
-    personal_data_ = new PersonalDataManager();
+    personal_data_.reset(new PersonalDataManager);
     personal_data_->Init(profile_.get());
     personal_data_->SetObserver(&personal_data_observer_);
 
     // Verify that the web database has been updated and the notification sent.
     EXPECT_CALL(personal_data_observer_,
                 OnPersonalDataChanged()).WillOnce(QuitUIMessageLoop());
     MessageLoop::current()->Run();
   }
 
   MessageLoopForUI message_loop_;
   BrowserThread ui_thread_;
   BrowserThread db_thread_;
   scoped_ptr<TestingProfile> profile_;
-  scoped_refptr<PersonalDataManager> personal_data_;
+  scoped_ptr<PersonalDataManager> personal_data_;
   NotificationRegistrar registrar_;
   NotificationObserverMock observer_;
   PersonalDataLoadedObserverMock personal_data_observer_;
 };
 
 TEST_F(PersonalDataManagerTest, AddProfile) {
   AutofillProfile profile0;
   autofill_test::SetProfileInfo(&profile0,
       "John", "Mitchell", "Smith",
       "j@s.com", "Acme Inc.", "1 Main", "Apt A", "San Francisco", "CA",
@@ skipping 1740 matching lines @@
 
   // Modify expected to include multi-valued fields.
   std::vector<string16> values;
   expected.GetMultiInfo(PHONE_HOME_CITY_AND_NUMBER, &values);
   values.push_back(ASCIIToUTF16("214-555-1234"));
   expected.SetMultiInfo(PHONE_HOME_CITY_AND_NUMBER, values);
 
   ASSERT_EQ(1U, results2.size());
   EXPECT_EQ(0, expected.CompareMulti(*results2[0]));
 }