Fix memory leaks in extensions::SocketPermission

Fix memory leaks in extensions::SocketPermission.


BUG=143401
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152229

[Mihai Parparita -not on Chrome, 2012-08-16 22:56:58]

LGTM

https://chromiumcodereview.appspot.com/10832354/diff/1/chrome/common/extensio...
File chrome/common/extensions/permissions/socket_permission.cc (right):

https://chromiumcodereview.appspot.com/10832354/diff/1/chrome/common/extensio...
chrome/common/extensions/permissions/socket_permission.cc:98: if
(result->data_set_.empty()) {
As discussed, switching APIPermissionDetail to not be refcounted, using a
scoped_ptr here, and a linked_ptr in the APIPermissionSet map is the preferable
long-term solution.

[commit-bot: I haz the power, 2012-08-16 23:11:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/10832354/1

[commit-bot: I haz the power, 2012-08-17 00:57:18]

Try job failure for 10832354-1 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-17 00:59:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/10832354/1

[commit-bot: I haz the power, 2012-08-17 04:43:37]

Change committed as 152049

[jln (very slow on Chromium), 2012-08-17 05:22:02]

This has to be reverted.

[ RUN      ] PermissionsTest.CreateDifference
[1767:1767:0816/220639:2507265492355:FATAL:ref_counted.cc(24)] Check failed:
in_dtor_. RefCounted object deleted without calling Release()
Backtrace:
	base::debug::StackTrace::StackTrace() [0x7f231c2dd33e]
	logging::LogMessage::~LogMessage() [0x7f231c3175c2]
	base::subtle::RefCountedBase::~RefCountedBase() [0x7f231c318a62]
	base::RefCounted<>::~RefCounted() [0x19d7785]
	(anonymous namespace)::SimpleDetail::~SimpleDetail() [0x19d70fc]
	extensions::SocketPermission::~SocketPermission() [0x19e3fa1]
	extensions::SocketPermission::~SocketPermission() [0x19e3f49]
	extensions::SocketPermission::Intersect() [0x19e4d39]
	extensions::APIPermissionSet::Intersection() [0x19d83ac]
	extensions::PermissionSet::CreateIntersection() [0x19daca9]
	extensions::PermissionsTest_CreateDifference_Test::TestBody() [0x174b65c]
	testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0x28f8a93]
	testing::internal::HandleExceptionsInMethodIfSupported<>() [0x28f099e]
	testing::Test::Run() [0x28e8475]
	testing::TestInfo::Run() [0x28e8b7b]
	testing::TestCase::Run() [0x28e90e7]
	testing::internal::UnitTestImpl::RunAllTests() [0x28ed3d5]
	testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0x28f6263]
	testing::internal::HandleExceptionsInMethodIfSupported<>() [0x28f1e8e]
	testing::UnitTest::Run() [0x28ed0c4]
	base::TestSuite::Run() [0x33f1b65]
	content::UnitTestTestSuite::Run() [0x33734dd]
	main [0x297ad34]

[commit-bot: I haz the power, 2012-08-17 11:28:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/10832354/10003

[Peng, 2012-08-17 11:29:13]

On 2012/08/17 05:22:02, Julien Tinnes wrote:
> This has to be reverted.
> 
> [ RUN      ] PermissionsTest.CreateDifference
> [1767:1767:0816/220639:2507265492355:FATAL:ref_counted.cc(24)] Check failed:
> in_dtor_. RefCounted object deleted without calling Release()
> Backtrace:
> 	base::debug::StackTrace::StackTrace() [0x7f231c2dd33e]
> 	logging::LogMessage::~LogMessage() [0x7f231c3175c2]
> 	base::subtle::RefCountedBase::~RefCountedBase() [0x7f231c318a62]
> 	base::RefCounted<>::~RefCounted() [0x19d7785]
> 	(anonymous namespace)::SimpleDetail::~SimpleDetail() [0x19d70fc]
> 	extensions::SocketPermission::~SocketPermission() [0x19e3fa1]
> 	extensions::SocketPermission::~SocketPermission() [0x19e3f49]
> 	extensions::SocketPermission::Intersect() [0x19e4d39]
> 	extensions::APIPermissionSet::Intersection() [0x19d83ac]
> 	extensions::PermissionSet::CreateIntersection() [0x19daca9]
> 	extensions::PermissionsTest_CreateDifference_Test::TestBody() [0x174b65c]
> 	testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0x28f8a93]
> 	testing::internal::HandleExceptionsInMethodIfSupported<>() [0x28f099e]
> 	testing::Test::Run() [0x28e8475]
> 	testing::TestInfo::Run() [0x28e8b7b]
> 	testing::TestCase::Run() [0x28e90e7]
> 	testing::internal::UnitTestImpl::RunAllTests() [0x28ed3d5]
> 	testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0x28f6263]
> 	testing::internal::HandleExceptionsInMethodIfSupported<>() [0x28f1e8e]
> 	testing::UnitTest::Run() [0x28ed0c4]
> 	base::TestSuite::Run() [0x33f1b65]
> 	content::UnitTestTestSuite::Run() [0x33734dd]
> 	main [0x297ad34]

Fixed the issue.

[commit-bot: I haz the power, 2012-08-17 13:22:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/10832354/9004

[commit-bot: I haz the power, 2012-08-17 16:42:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/10832354/3004

[Mihai Parparita -not on Chrome, 2012-08-17 18:01:15]

https://chromiumcodereview.appspot.com/10832354/diff/3004/chrome/common/exten...
File chrome/common/extensions/permissions/socket_permission.cc (right):

https://chromiumcodereview.appspot.com/10832354/diff/3004/chrome/common/exten...
chrome/common/extensions/permissions/socket_permission.cc:99:
scoped_refptr<SocketPermission> p(result);
BTW, I think result->Release() would have been enough, that ends up calling the
destructor once the ref count goes to 0:

http://code.google.com/searchframe#OAMlx_jo-ck/src/base/memory/ref_counted.h&...

But don't worry about cleaning up the hack, just remove the refcounting
altogether.

[commit-bot: I haz the power, 2012-08-17 22:30:07]

Failed to apply patch for tools/valgrind/memcheck/suppressions.txt:
While running patch -p1 --forward --force;
patching file tools/valgrind/memcheck/suppressions.txt
Hunk #1 FAILED at 5777.
1 out of 1 hunk FAILED -- saving rejects to file
tools/valgrind/memcheck/suppressions.txt.rej

[commit-bot: I haz the power, 2012-08-18 00:49:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/10832354/13002

[commit-bot: I haz the power, 2012-08-18 03:07:07]

Change committed as 152229