Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(530)

Issue 2881063003: cc: Change RTree::Search to return results, add comments, more tests. (Closed)

Created:
3 years, 7 months ago by vmpstr
Modified:
3 years, 7 months ago
Reviewers:
danakj
CC:
chromium-reviews, cc-bugs_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

cc: Change RTree::Search to return results, add comments, more tests. This patch does three things: - Changes RTree::Search signature to return the resulting vector, instead of taking it as a pointer. - Adds function comments to document the behavior. - Adds tests to ensure that the queries we make return sorted results. R=danakj@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2881063003 Cr-Commit-Position: refs/heads/master@{#472148} Committed: https://chromium.googlesource.com/chromium/src/+/c524579fa4b01df8f55cb766f59a166a93c02624

Patch Set 1 #

Total comments: 5

Patch Set 2 : update #

Patch Set 3 : fixup #

Patch Set 4 : update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -28 lines) Patch
M cc/base/rtree.h View 2 chunks +14 lines, -6 lines 0 comments Download
M cc/base/rtree.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M cc/base/rtree_perftest.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M cc/base/rtree_unittest.cc View 1 2 chunks +36 lines, -10 lines 0 comments Download
M cc/paint/discardable_image_map.cc View 1 2 1 chunk +1 line, -3 lines 0 comments Download
M cc/paint/discardable_image_map_unittest.cc View 1 1 chunk +1 line, -3 lines 0 comments Download
M cc/paint/display_item_list.cc View 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 19 (10 generated)
vmpstr
Please take a look.
3 years, 7 months ago (2017-05-15 20:02:28 UTC) #2
danakj
LGTM https://codereview.chromium.org/2881063003/diff/1/cc/paint/discardable_image_map.cc File cc/paint/discardable_image_map.cc (right): https://codereview.chromium.org/2881063003/diff/1/cc/paint/discardable_image_map.cc#newcode41 cc/paint/discardable_image_map.cc:41: for (size_t index : indices) { you could ...
3 years, 7 months ago (2017-05-15 20:35:34 UTC) #3
vmpstr
https://codereview.chromium.org/2881063003/diff/1/cc/base/rtree_unittest.cc File cc/base/rtree_unittest.cc (right): https://codereview.chromium.org/2881063003/diff/1/cc/base/rtree_unittest.cc#newcode104 cc/base/rtree_unittest.cc:104: VerifySorted(results); Done here as well. https://codereview.chromium.org/2881063003/diff/1/cc/paint/discardable_image_map.cc File cc/paint/discardable_image_map.cc (right): ...
3 years, 7 months ago (2017-05-15 20:46:34 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2881063003/40001
3 years, 7 months ago (2017-05-15 20:47:48 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm64_dbg_recipe/builds/268410) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
3 years, 7 months ago (2017-05-15 21:27:50 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2881063003/60001
3 years, 7 months ago (2017-05-15 21:36:35 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/454967)
3 years, 7 months ago (2017-05-15 22:40:16 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2881063003/60001
3 years, 7 months ago (2017-05-16 15:04:40 UTC) #16
commit-bot: I haz the power
3 years, 7 months ago (2017-05-16 17:43:37 UTC) #19
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/c524579fa4b01df8f55cb766f59a...

Powered by Google App Engine
This is Rietveld 408576698