Use WeakPtr to simplify AutofillPopupControllerImpl memory management

chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc

Index: chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc
diff --git a/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc
index 3a43b3e0e684d512bccf366a9165e0d02c32418f..8fd110cf80d2fedfcd3ea0e74579e351ac49b66d 100644
--- a/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc
+++ b/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/test_autofill_external_delegate.h"
 #include "chrome/browser/ui/autofill/autofill_popup_controller_impl.h"
@@ -14,6 +14,7 @@
 
 using ::testing::_;
 using ::testing::AtLeast;
+using base::WeakPtr;
 using WebKit::WebAutofillClient;
 
 namespace {
@@ -94,6 +95,10 @@ class TestAutofillPopupController : public AutofillPopupControllerImpl {
     return AutofillPopupControllerImpl::GetDesiredPopupHeight();
   }
 
+  WeakPtr<AutofillPopupControllerImpl> GetWeakPtr() {
+    return AutofillPopupControllerImpl::GetWeakPtr();
+  }
+
   MOCK_METHOD1(InvalidateRow, void(size_t));
   MOCK_METHOD0(UpdateBoundsAndRedrawPopup, void());
   MOCK_METHOD0(Hide, void());
@@ -112,6 +117,7 @@ class AutofillPopupControllerUnitTest : public ::testing::Test {
     : autofill_popup_controller_(
           new testing::NiceMock<TestAutofillPopupController>(
               &external_delegate_, gfx::Rect())) {}
+
   virtual ~AutofillPopupControllerUnitTest() {
     // This will make sure the controller and the view (if any) are both
     // cleaned up.
@@ -214,14 +220,9 @@ TEST_F(AutofillPopupControllerUnitTest, RemoveLine) {
   autofill_popup_controller_->SetSelectedLine(0);
   EXPECT_TRUE(autofill_popup_controller_->RemoveSelectedLine());
 
-  // Remove the last entry. The popup should then be hidden since there are
-  // no Autofill entries left.
-  EXPECT_CALL(external_delegate_, ControllerDestroyed());
-
   autofill_popup_controller_->SetSelectedLine(0);
-  // The controller self-deletes here, don't double delete.
+  EXPECT_CALL(*autofill_popup_controller_, Hide());
   EXPECT_TRUE(autofill_popup_controller_->RemoveSelectedLine());
-  autofill_popup_controller_ = NULL;
 }
 
 TEST_F(AutofillPopupControllerUnitTest, SkipSeparator) {
@@ -247,32 +248,21 @@ TEST_F(AutofillPopupControllerUnitTest, SkipSeparator) {
 TEST_F(AutofillPopupControllerUnitTest, GetOrCreate) {
   MockAutofillExternalDelegate delegate;
 
-  AutofillPopupControllerImpl* controller =
-      AutofillPopupControllerImpl::GetOrCreate(
-          NULL,
-          &delegate,
-          NULL,
-          gfx::Rect());
+  WeakPtr<AutofillPopupControllerImpl> controller;
+  controller = AutofillPopupControllerImpl::GetOrCreate(
+      controller, &delegate, NULL, gfx::Rect());

[Ilya Sherman, 2013/02/06 05:12:52]

Rather than passing the controller as an argument here, pass
WeakPtr<AutofillPopupControllerImpl>(), since that's really what you mean.

[kaiwang, 2013/02/06 05:56:21]

Done.

   EXPECT_TRUE(controller);
 
-  // This should not inform |delegate| of its destruction.
-  EXPECT_CALL(delegate, ControllerDestroyed()).Times(0);
   controller->Hide();
 
-  controller =
-      AutofillPopupControllerImpl::GetOrCreate(
-          NULL,
-          &delegate,
-          NULL,
-          gfx::Rect());
+  controller = AutofillPopupControllerImpl::GetOrCreate(
+      WeakPtr<AutofillPopupControllerImpl>(), &delegate, NULL, gfx::Rect());
   EXPECT_TRUE(controller);
-  AutofillPopupControllerImpl* controller2 =
-      AutofillPopupControllerImpl::GetOrCreate(
-          controller,
-          &delegate,
-          NULL,
-          gfx::Rect());
-  EXPECT_EQ(controller, controller2);
+
+  WeakPtr<AutofillPopupControllerImpl> controller2 =
+      AutofillPopupControllerImpl::GetOrCreate(controller, &delegate, NULL,
+                                               gfx::Rect());
+  EXPECT_EQ(controller.get(), controller2.get());
   controller->Hide();
 
   testing::NiceMock<TestAutofillPopupController>* test_controller =
@@ -283,7 +273,7 @@ TEST_F(AutofillPopupControllerUnitTest, GetOrCreate) {
   gfx::Rect bounds(0, 0, 1, 2);
   AutofillPopupControllerImpl* controller3 =
       AutofillPopupControllerImpl::GetOrCreate(
-          test_controller,
+          test_controller->GetWeakPtr(),
           &delegate,
           NULL,
           bounds);
@@ -292,7 +282,6 @@ TEST_F(AutofillPopupControllerUnitTest, GetOrCreate) {
       static_cast<AutofillPopupController*>(controller3)->element_bounds());
   controller3->Hide();
 
-  EXPECT_CALL(delegate, ControllerDestroyed());
   delete test_controller;
 }