Change scoped_ptr::reset()'s behaviour to match unique_ptr.

Change scoped_ptr::reset()'s behaviour to match unique_ptr.

unique_ptr::reset() is defined to update its pointer before deleting the
old pointer. An interim change was introduced 2 years ago to try and
catch cases which relied on the pointer being set after deletion.

BUG=176091
Committed: https://crrev.com/0d4ef2583a6f19c3e61be04d36eb1a60b133832c
Cr-Commit-Position: refs/heads/master@{#350422}

[Anand Mistry (off Chromium), 2015-09-23 02:56:59]

amistry@chromium.org changed reviewers:
+ danakj@chromium.org, pkasting@chromium.org

[Anand Mistry (off Chromium), 2015-09-23 02:58:17]

cc dcheng, owner of crbug.com/176091

[Peter Kasting, 2015-09-23 03:09:45]

LGTM!

https://codereview.chromium.org/1358373002/diff/1/base/memory/scoped_ptr_unit...
File base/memory/scoped_ptr_unittest.cc (right):

https://codereview.chromium.org/1358373002/diff/1/base/memory/scoped_ptr_unit...
base/memory/scoped_ptr_unittest.cc:712: // Break the cycle by calling |reset()|.
Nit: No || around function names

I might add to the end: "This will cause a->b to be deleted before the call to
reset() returns.  This tests that the implementation of scoped_ptr::reset()
doesn't access |this| after it deletes the underlying pointer."

[Anand Mistry (off Chromium), 2015-09-23 04:04:21]

I'll wait for danakj and/or dcheng to take a look, and won't submit this till
tomorrow at the earliest to give anyone else a chance to drive-by comment.

https://codereview.chromium.org/1358373002/diff/1/base/memory/scoped_ptr_unit...
File base/memory/scoped_ptr_unittest.cc (right):

https://codereview.chromium.org/1358373002/diff/1/base/memory/scoped_ptr_unit...
base/memory/scoped_ptr_unittest.cc:712: // Break the cycle by calling |reset()|.
On 2015/09/23 03:09:44, Peter Kasting wrote:
> Nit: No || around function names
> 
> I might add to the end: "This will cause a->b to be deleted before the call to
> reset() returns.  This tests that the implementation of scoped_ptr::reset()
> doesn't access |this| after it deletes the underlying pointer."

Done and done.

[dcheng, 2015-09-23 04:39:25]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2015-09-23 04:39:25]

lgtm (though I'm not a //base OWNER)

Nit: remove the ||'s around {unique,scoped}_ptr::reset in the issue description
as well.

[Anand Mistry (off Chromium), 2015-09-23 04:43:45]

On 2015/09/23 04:39:25, dcheng wrote:
> lgtm (though I'm not a //base OWNER)
> 
> Nit: remove the ||'s around {unique,scoped}_ptr::reset in the issue
description
> as well.

Done, but added ()'s to be consistent with the in-code comments.

[danakj, 2015-09-23 18:51:13]

LGTM

[Anand Mistry (off Chromium), 2015-09-23 22:44:06]

The CQ bit was checked by amistry@chromium.org

[Anand Mistry (off Chromium), 2015-09-23 22:44:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1358373002/#ps20001
(title: "Expand comment.")

[commit-bot: I haz the power, 2015-09-23 22:45:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1358373002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1358373002/20001

[commit-bot: I haz the power, 2015-09-24 00:51:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-24 00:52:36]

Patchset 2 (id:??) landed as
https://crrev.com/0d4ef2583a6f19c3e61be04d36eb1a60b133832c
Cr-Commit-Position: refs/heads/master@{#350422}