Fix BrowserAccessibilityManager valgrind errors.

chrome/browser/accessibility/browser_accessibility_manager_unittest.cc

Index: chrome/browser/accessibility/browser_accessibility_manager_unittest.cc
diff --git a/chrome/browser/accessibility/browser_accessibility_manager_unittest.cc b/chrome/browser/accessibility/browser_accessibility_manager_unittest.cc
index af984924ae1ae7788dedcd4cff948e0bc52f0255..babd0496d2561702328e826be227e1d34ddfd55f 100644
--- a/chrome/browser/accessibility/browser_accessibility_manager_unittest.cc
+++ b/chrome/browser/accessibility/browser_accessibility_manager_unittest.cc
@@ -625,4 +625,8 @@ TEST(BrowserAccessibilityManagerTest, TestCreateEmptyDocument) {
 
   // Verify we don't reuse objects that have changed roles.
   EXPECT_NE(acc1_2, acc2_2);
+
+  // Ensure we properly cleaned up.
+  delete manager;

[Paweł Hajdan Jr., 2011/08/12 17:10:19]

How about just making manager a scoped_ptr? Much less error-prone, and trying to
manually manage resources is #1 source of leaks in unit tests. Of course feel
free to keep fixing issue after issue.

Any ASSERT before this code will cause a leak on failure. Using scoped_ptr would
prevent that.

[David Tseng, 2011/08/12 17:30:30]

I totally agree with you in general, but in this case, it's important for us to
check the assert below because BrowserAccessibilityManager performs it's own ref
counting. If there's an error in our ref counting, a scoped_ptr would not help.
It had been my mistake to not put this here in the first place.

[Chris Guillory, 2011/08/12 18:44:06]

We had this comment in a previous code review. dmazzoni wanted to keep the
delete explicit since the test is actually checking for leaks. I suppose using a
scoped_ptr reset method could accomplish the same thing.

[Chris Guillory, 2011/08/12 18:46:08]

I think these points are valid and we should use a scoped_pointer.

+  ASSERT_EQ(0, CountedBrowserAccessibility::global_obj_count_);
 }