Fix scoped_refptr assignment operator in the case of having it as a member.

If a class holds a reference to itself in scoped_refptr, assigning
to it a new value leads to calling Release while in destructor.

R=levin@chromium.org
TEST=RefCountedUnitTest.ScopedRefPtrToSelf

Review URL: http://codereview.chromium.org/9021020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@116480 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 33 insertions, 4 deletions

diff --git a/base/memory/ref_counted.h b/base/memory/ref_counted.h
index 439fda4..b7cb5e0 100644
--- a/base/memory/ref_counted.h
+++ b/base/memory/ref_counted.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -260,9 +260,10 @@ class scoped_refptr {
     // AddRef first so that self assignment should work
     if (p)
       p->AddRef();
-    if (ptr_ )
-      ptr_ ->Release();
+    T* old_ptr = ptr_;
     ptr_ = p;
+    if (old_ptr)
+      old_ptr->Release();
     return *this;
   }
 
diff --git a/base/memory/ref_counted_unittest.cc b/base/memory/ref_counted_unittest.cc
index dcc292f..93b93c4 100644
--- a/base/memory/ref_counted_unittest.cc
+++ b/base/memory/ref_counted_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -22,6 +22,27 @@ class CheckDerivedMemberAccess : public scoped_refptr<SelfAssign> {
   }
 };
 
+class ScopedRefPtrToSelf : public base::RefCounted<ScopedRefPtrToSelf> {
+ public:
+  ScopedRefPtrToSelf()
+      : ALLOW_THIS_IN_INITIALIZER_LIST(self_ptr_(this)) {
+  }
+  ~ScopedRefPtrToSelf() { was_destroyed_ = true; }
+
+  static bool was_destroyed() { return was_destroyed_; }
+
+  void SelfDestruct() { self_ptr_ = NULL; }
+
+ private:
+  friend class base::RefCounted<ScopedRefPtrToSelf>;
+
+  static bool was_destroyed_;
+
+  scoped_refptr<ScopedRefPtrToSelf> self_ptr_;
+};
+
+bool ScopedRefPtrToSelf::was_destroyed_ = false;
+
 }  // end namespace
 
 TEST(RefCountedUnitTest, TestSelfAssignment) {
@@ -34,3 +55,10 @@ TEST(RefCountedUnitTest, TestSelfAssignment) {
 TEST(RefCountedUnitTest, ScopedRefPtrMemberAccess) {
   CheckDerivedMemberAccess check;
 }
+
+TEST(RefCountedUnitTest, ScopedRefPtrToSelf) {
+  ScopedRefPtrToSelf* check = new ScopedRefPtrToSelf();
+  EXPECT_FALSE(ScopedRefPtrToSelf::was_destroyed());
+  check->SelfDestruct();
+  EXPECT_TRUE(ScopedRefPtrToSelf::was_destroyed());
+}