fix leak in RTree::Remove()

fix leak in RTree::Remove()

Caused in the common case when RTree::RemoveNode would remove a non-record-node
child but would have more than min_children_ remaining nodes.

BUG=370779
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269154

[piman, 2014-05-07 20:29:09]

Could you add a unit test that exercises the problematic path? that way it'll be
run via ASAN/LSAN and can confirm that the leak is fixed.

https://codereview.chromium.org/270373002/diff/1/ui/gfx/geometry/r_tree.cc
File ui/gfx/geometry/r_tree.cc (right):

https://codereview.chromium.org/270373002/diff/1/ui/gfx/geometry/r_tree.cc#ne...
ui/gfx/geometry/r_tree.cc:630:
root_.reset(root_->RemoveAndReturnLastChild().release());
nit: root_ = root_->RemoveAndReturnLastChild(); should work

[luken, 2014-05-07 20:36:58]

So when I ran LSAN locally almost all of the unit tests reported leaks in this
code path, until it was fixed. Not sure why the first RTRee cl was allowed to
land, we must not be running LSAN on unit tests.

https://codereview.chromium.org/270373002/diff/1/ui/gfx/geometry/r_tree.cc
File ui/gfx/geometry/r_tree.cc (right):

https://codereview.chromium.org/270373002/diff/1/ui/gfx/geometry/r_tree.cc#ne...
ui/gfx/geometry/r_tree.cc:630:
root_.reset(root_->RemoveAndReturnLastChild().release());
On 2014/05/07 20:29:09, piman wrote:
> nit: root_ = root_->RemoveAndReturnLastChild(); should work

Done.

[piman, 2014-05-07 20:55:02]

lgtm

[scottmg, 2014-05-07 20:58:57]

The CQ bit was checked by scottmg@chromium.org

[luken, 2014-05-07 20:59:16]

The CQ bit was checked by luken@chromium.org

[luken, 2014-05-07 20:59:18]

The CQ bit was unchecked by luken@chromium.org

[luken, 2014-05-07 20:59:46]

The CQ bit was checked by luken@chromium.org

[commit-bot: I haz the power, 2014-05-07 21:00:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/luken@chromium.org/270373002/20001

[luken, 2014-05-07 21:10:51]

The CQ bit was unchecked by luken@chromium.org

[luken, 2014-05-07 21:11:07]

LGTM

[luken, 2014-05-07 21:11:20]

The CQ bit was checked by luken@chromium.org

[commit-bot: I haz the power, 2014-05-07 21:14:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/luken@chromium.org/270373002/20001

[luken, 2014-05-07 21:25:27]

The CQ bit was unchecked by luken@chromium.org

[sky, 2014-05-07 21:31:50]

Rubber stamp LGTM

[luken, 2014-05-07 21:33:14]

The CQ bit was checked by luken@chromium.org

[commit-bot: I haz the power, 2014-05-07 21:39:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/luken@chromium.org/270373002/20001

[commit-bot: I haz the power, 2014-05-08 01:21:44]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium

[commit-bot: I haz the power, 2014-05-08 03:07:21]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium

[commit-bot: I haz the power, 2014-05-08 03:41:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-08 03:41:10]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium

[luken, 2014-05-08 16:15:33]

The CQ bit was checked by luken@chromium.org

[commit-bot: I haz the power, 2014-05-08 16:17:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/luken@chromium.org/270373002/20001

[commit-bot: I haz the power, 2014-05-09 02:24:36]

Change committed as 269154