Issue 9997007: Objects that derive from RefCounted/RefCountedThreadSafe should not have public dtors.

Issue 9997007: Objects that derive from RefCounted/RefCountedThreadSafe should not have public dtors. (Closed)

Created:
8 years, 8 months ago by Ryan Sleevi

Modified:
8 years, 8 months ago

Reviewers:
michaeln, jar (doing other things), Jamie

CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, sadrul, hclam+watch_chromium.org, wez+watch_chromium.org, erikwright (departed), amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, darin-cc_chromium.org, simonmorris+watch_chromium.org, brettw-cc_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Objects that derive from RefCounted/RefCountedThreadSafe should not have public dtors. BUG=none TEST=compiles Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=131088

Patch Set 1 #

Total comments: 2

Patch Set 2 : One more base/ change from a different CL #

Total comments: 1

Patch Set 3 : virtual & DISALLOW #

Created: 8 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+66 lines, -19 lines)			Patch
M	base/message_loop_proxy_impl.h	View		3 chunks	+7 lines, -7 lines	0 comments	Download
M	base/message_pump.h	View		2 chunks	+4 lines, -1 line	0 comments	Download
M	base/message_pump_default.h	View		2 chunks	+3 lines, -1 line	0 comments	Download
M	base/message_pump_libevent.h	View		2 chunks	+3 lines, -1 line	0 comments	Download
M	base/message_pump_mac.h	View		6 chunks	+13 lines, -2 lines	0 comments	Download
M	base/message_pump_mac.mm	View		2 chunks	+4 lines, -0 lines	0 comments	Download
M	base/sequenced_task_runner.h	View		1 chunk	+4 lines, -1 line	0 comments	Download
M	base/single_thread_task_runner.h	View		1 chunk	+4 lines, -1 line	0 comments	Download
M	base/synchronization/waitable_event.h	View		2 chunks	+4 lines, -1 line	0 comments	Download
M	base/synchronization/waitable_event_watcher_posix.cc	View	1 2	1 chunk	+5 lines, -0 lines	0 comments	Download
M	base/threading/worker_pool_posix.h	View		2 chunks	+3 lines, -1 line	0 comments	Download
M	remoting/base/plugin_message_loop_proxy.h	View		2 chunks	+3 lines, -1 line	0 comments	Download
M	webkit/dom_storage/dom_storage_task_runner.h	View		3 chunks	+9 lines, -2 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

Ryan Sleevi

jar: base/ miachaeln: webkit/dom_storage jamiewalch: remoting/ These were picked up with some WIP changes to ...

8 years, 8 months ago (2012-04-05 03:27:12 UTC) #1

michaeln

webkit/dom_storage lgtm http://codereview.chromium.org/9997007/diff/1/webkit/dom_storage/dom_storage_task_runner.h File webkit/dom_storage/dom_storage_task_runner.h (left): http://codereview.chromium.org/9997007/diff/1/webkit/dom_storage/dom_storage_task_runner.h#oldcode82 webkit/dom_storage/dom_storage_task_runner.h:82: virtual ~DomStorageWorkerPoolTaskRunner(); why the change from private ...

8 years, 8 months ago (2012-04-05 18:42:09 UTC) #3

Ryan Sleevi

http://codereview.chromium.org/9997007/diff/1/webkit/dom_storage/dom_storage_task_runner.h File webkit/dom_storage/dom_storage_task_runner.h (left): http://codereview.chromium.org/9997007/diff/1/webkit/dom_storage/dom_storage_task_runner.h#oldcode82 webkit/dom_storage/dom_storage_task_runner.h:82: virtual ~DomStorageWorkerPoolTaskRunner(); On 2012/04/05 18:42:10, michaeln wrote: > why ...

8 years, 8 months ago (2012-04-05 19:18:12 UTC) #4

jar (doing other things)

LGTM with one nit below. http://codereview.chromium.org/9997007/diff/3002/base/synchronization/waitable_event_watcher_posix.cc File base/synchronization/waitable_event_watcher_posix.cc (right): http://codereview.chromium.org/9997007/diff/3002/base/synchronization/waitable_event_watcher_posix.cc#newcode47 base/synchronization/waitable_event_watcher_posix.cc:47: virtual ~Flag() {} Why ...

8 years, 8 months ago (2012-04-05 23:04:59 UTC) #5

Ryan Sleevi

On 2012/04/05 23:04:59, jar wrote: > LGTM with one nit below. > > http://codereview.chromium.org/9997007/diff/3002/base/synchronization/waitable_event_watcher_posix.cc > ...

8 years, 8 months ago (2012-04-06 00:54:33 UTC) #6

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsleevi@chromium.org/9997007/8001

8 years, 8 months ago (2012-04-06 00:56:15 UTC) #7

Change committed as 131088

Expand Messages | Collapse Messages