Issue 14746011: Use Persistent::Reset.

Issue 14746011: Use Persistent::Reset. (Closed)

Created:
7 years, 7 months ago by marja

Modified:
7 years, 7 months ago

Reviewers:
cbentzel, dcarney, eroman, jochen (gone - plz use gerrit)

CC:
chromium-reviews, Aaron Boodman, cbentzel+watch_chromium.org, chromium-apps-reviews_chromium.org

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

Visibility:
Public.

More Reviews

Description

Use Persistent::Reset. This gets rid of more places where Persistent handles are copied (see bug). BUG=236290 TBR=cbenzel@chromium.org TBR=eroman@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=199809

Patch Set 2 : rebased & more #

Total comments: 4

Patch Set 3 : code review (jochen) #

Created: 7 years, 7 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+3 lines, -5 lines)			Patch
	M	chrome/renderer/extensions/scoped_persistent.h	View	1 2	1 chunk	+1 line, -2 lines	0 comments	Download
	M	net/proxy/proxy_resolver_v8.cc	View	1	2 chunks	+2 lines, -3 lines	0 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

jochen (gone - plz use gerrit)

https://codereview.chromium.org/14746011/diff/2001/chrome/renderer/extensions/scoped_persistent.h File chrome/renderer/extensions/scoped_persistent.h (right): https://codereview.chromium.org/14746011/diff/2001/chrome/renderer/extensions/scoped_persistent.h#newcode30 chrome/renderer/extensions/scoped_persistent.h:30: reset(); this call is not needed anymore, right? https://codereview.chromium.org/14746011/diff/2001/chrome/renderer/extensions/scoped_persistent.h#newcode37 ...

7 years, 7 months ago (2013-05-13 14:07:40 UTC) #3

marja

Thanks for review! TBRing this to cbentzel because it's adapting proxy_resolver_v8 to API changes. https://codereview.chromium.org/14746011/diff/2001/chrome/renderer/extensions/scoped_persistent.h ...

7 years, 7 months ago (2013-05-13 14:16:04 UTC) #5

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/marja@chromium.org/14746011/10001

7 years, 7 months ago (2013-05-13 14:16:28 UTC) #6

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/marja@chromium.org/14746011/10001

7 years, 7 months ago (2013-05-13 14:21:18 UTC) #8

Message was sent while issue was closed.

Change committed as 199809

Expand Messages | Collapse Messages