Factor out bookmark-specific sorting logic from BookmarkIndex

Factor out bookmark-specific sorting logic from BookmarkIndex

BookmarkIndex contains logic for sorting bookmarks by the number of times
the bookmark URL was typed into the omnibox. This change moves the logic
out of BookmarkIndex. Instead, if a TitledUrlNodeSorter is supplied when
the index is constructed then it will be used to sort the match results.

BUG=630769
Committed: https://crrev.com/191b88720ff188f2aa3aac77265a1c3573e95f0e
Cr-Commit-Position: refs/heads/master@{#438250}

[mattreynolds, 2016-12-12 19:32:54]

mattreynolds@chromium.org changed reviewers:
+ sky@chromium.org

[mattreynolds, 2016-12-12 19:32:54]

Hi Scott, PTAL

[sky, 2016-12-12 20:46:11]

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
File components/bookmarks/browser/titled_url_node_sorter.h (right):

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/titled_url_node_sorter.h:20: // Constructs
|sorted_nodes| by sorting the nodes in |matches|.
Constructs? Maybe 'Sorts |matches| in an implementation specific way, placing
the result in |sorted_nodes|'.

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/titled_url_node_sorter.h:23: };
Add public virtual destructor.

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
File components/bookmarks/browser/typed_count_sorter.h (right):

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/typed_count_sorter.h:17:
TypedCountSorter(BookmarkClient* client);
explicit

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/typed_count_sorter.h:18: ~TypedCountSorter();
override

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/typed_count_sorter.h:23: void SortMatches(const
TitledUrlNodeSet& matches,
Chrome style is to not document this and instead prefix with the class you are
overriding from. See chromium style guide for more details. I would place the
description you have here as a class description (with a bit of cleanup).

[mattreynolds, 2016-12-12 22:09:14]

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
File components/bookmarks/browser/titled_url_node_sorter.h (right):

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/titled_url_node_sorter.h:20: // Constructs
|sorted_nodes| by sorting the nodes in |matches|.
On 2016/12/12 20:46:10, sky wrote:
> Constructs? Maybe 'Sorts |matches| in an implementation specific way, placing
> the result in |sorted_nodes|'.

Done.

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/titled_url_node_sorter.h:23: };
On 2016/12/12 20:46:10, sky wrote:
> Add public virtual destructor.

Done.

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
File components/bookmarks/browser/typed_count_sorter.h (right):

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/typed_count_sorter.h:17:
TypedCountSorter(BookmarkClient* client);
On 2016/12/12 20:46:10, sky wrote:
> explicit

Done.

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/typed_count_sorter.h:18: ~TypedCountSorter();
On 2016/12/12 20:46:10, sky wrote:
> override

Done.

https://codereview.chromium.org/2559633003/diff/20001/components/bookmarks/br...
components/bookmarks/browser/typed_count_sorter.h:23: void SortMatches(const
TitledUrlNodeSet& matches,
On 2016/12/12 20:46:10, sky wrote:
> Chrome style is to not document this and instead prefix with the class you are
> overriding from. See chromium style guide for more details. I would place the
> description you have here as a class description (with a bit of cleanup).

Done.

[sky, 2016-12-12 23:22:20]

LGTM

[mattreynolds, 2016-12-12 23:48:45]

The CQ bit was checked by mattreynolds@chromium.org

[commit-bot: I haz the power, 2016-12-12 23:49:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 01:45:16]

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

[commit-bot: I haz the power, 2016-12-13 01:45:17]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mattreynolds, 2016-12-13 18:22:49]

The CQ bit was checked by mattreynolds@chromium.org

[commit-bot: I haz the power, 2016-12-13 18:23:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 19:25:43]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481653369429930,
"parent_rev": "846757848bf16de810c4bc88a03ead77b5fe917f", "commit_rev":
"51f8e42eb347e82f0687ca42fb95b41c7dfc102a"}

[commit-bot: I haz the power, 2016-12-13 19:26:13]

Description was changed from

==========
Factor out bookmark-specific sorting logic from BookmarkIndex

BookmarkIndex contains logic for sorting bookmarks by the number of times
the bookmark URL was typed into the omnibox. This change moves the logic
out of BookmarkIndex. Instead, if a TitledUrlNodeSorter is supplied when
the index is constructed then it will be used to sort the match results.

BUG=630769
==========

to

==========
Factor out bookmark-specific sorting logic from BookmarkIndex

BookmarkIndex contains logic for sorting bookmarks by the number of times
the bookmark URL was typed into the omnibox. This change moves the logic
out of BookmarkIndex. Instead, if a TitledUrlNodeSorter is supplied when
the index is constructed then it will be used to sort the match results.

BUG=630769

Review-Url: https://codereview.chromium.org/2559633003
==========

[commit-bot: I haz the power, 2016-12-13 19:26:14]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-13 19:29:24]

Description was changed from

==========
Factor out bookmark-specific sorting logic from BookmarkIndex

BookmarkIndex contains logic for sorting bookmarks by the number of times
the bookmark URL was typed into the omnibox. This change moves the logic
out of BookmarkIndex. Instead, if a TitledUrlNodeSorter is supplied when
the index is constructed then it will be used to sort the match results.

BUG=630769

Review-Url: https://codereview.chromium.org/2559633003
==========

to

==========
Factor out bookmark-specific sorting logic from BookmarkIndex

BookmarkIndex contains logic for sorting bookmarks by the number of times
the bookmark URL was typed into the omnibox. This change moves the logic
out of BookmarkIndex. Instead, if a TitledUrlNodeSorter is supplied when
the index is constructed then it will be used to sort the match results.

BUG=630769

Committed: https://crrev.com/191b88720ff188f2aa3aac77265a1c3573e95f0e
Cr-Commit-Position: refs/heads/master@{#438250}
==========

[commit-bot: I haz the power, 2016-12-13 19:29:25]

Patchset 3 (id:??) landed as
https://crrev.com/191b88720ff188f2aa3aac77265a1c3573e95f0e
Cr-Commit-Position: refs/heads/master@{#438250}