Cleanup: Remove unused SearchTabHelper::GetOpenUrls

Cleanup: Remove unused SearchTabHelper::GetOpenUrls

...and all the code that was there to support it, including Browser::GetOpenUrls and chrome::GetOpenUrls, as well as TopSites::GetCanonicalURLString.
While we're here, also clean up some other unused code from TopSites.

BUG=627747
Committed: https://crrev.com/b4a9ec8b992f2f09f961efdcb4d554d2762a9c22
Cr-Commit-Position: refs/heads/master@{#440723}

[Marc Treib, 2016-12-12 16:58:18]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-12 16:58:46]

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

[Marc Treib, 2016-12-12 17:10:17]

Description was changed from

==========
Cleanup: Remove unused SearchTabHelper::GetOpenUrls

...and all the code that was there to support it.

BUG=627747
==========

to

==========
Cleanup: Remove unused SearchTabHelper::GetOpenUrls

...and all the code that was there to support it, including Browser::GetOpenUrls
and chrome::GetOpenUrls, as well as TopSites::GetCanonicalURLString.
While we're here, also clean up some other unused code from TopSites.

BUG=627747
==========

[Marc Treib, 2016-12-12 17:11:16]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-12 17:11:46]

Dry run: 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 02:00:55]

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

[commit-bot: I haz the power, 2016-12-13 02:00:56]

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

[Marc Treib, 2016-12-13 09:19:53]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 09:20:10]

Dry run: 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 10:07:04]

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

[commit-bot: I haz the power, 2016-12-13 10:07:05]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2016-12-13 10:14:26]

treib@chromium.org changed reviewers:
+ msw@chromium.org, sdefresne@chromium.org

[Marc Treib, 2016-12-13 10:14:27]

msw: Please look at c/b/ui/.

sdefresne: Please look at components/history/.

Thanks!

[msw, 2016-12-13 22:31:53]

lgtm with nits; thanks for the cleanup!

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:59: #include
"chrome/browser/history/top_sites_factory.h"
nit: remove

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:165: #include
"components/history/core/browser/top_sites.h"
nit: remove

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser.h:12: #include <set>
nit: remove

https://codereview.chromium.org/2567043002/diff/40001/components/history/core...
File components/history/core/browser/top_sites.h (right):

https://codereview.chromium.org/2567043002/diff/40001/components/history/core...
components/history/core/browser/top_sites.h:13: #include
"base/task/cancelable_task_tracker.h"
nit: remove (move to impl)

https://codereview.chromium.org/2567043002/diff/40001/components/history/core...
File components/history/core/browser/top_sites_impl.cc (right):

https://codereview.chromium.org/2567043002/diff/40001/components/history/core...
components/history/core/browser/top_sites_impl.cc:412: void
TopSitesImpl::StartQueryForMostVisited() {
nit: reorder to match new decl order

https://codereview.chromium.org/2567043002/diff/40001/components/omnibox/brow...
File components/omnibox/browser/zero_suggest_provider_unittest.cc (left):

https://codereview.chromium.org/2567043002/diff/40001/components/omnibox/brow...
components/omnibox/browser/zero_suggest_provider_unittest.cc:75: return *(new
std::string());
wow, glad we're removing this!

[Marc Treib, 2016-12-14 11:12:22]

All nits addressed, thanks!

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:59: #include
"chrome/browser/history/top_sites_factory.h"
On 2016/12/13 22:31:52, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:165: #include
"components/history/core/browser/top_sites.h"
On 2016/12/13 22:31:52, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/2567043002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser.h:12: #include <set>
On 2016/12/13 22:31:53, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/2567043002/diff/40001/components/history/core...
File components/history/core/browser/top_sites.h (right):

https://codereview.chromium.org/2567043002/diff/40001/components/history/core...
components/history/core/browser/top_sites.h:13: #include
"base/task/cancelable_task_tracker.h"
On 2016/12/13 22:31:53, msw wrote:
> nit: remove (move to impl)

Done. (impl already included it)

https://codereview.chromium.org/2567043002/diff/40001/components/history/core...
File components/history/core/browser/top_sites_impl.cc (right):

https://codereview.chromium.org/2567043002/diff/40001/components/history/core...
components/history/core/browser/top_sites_impl.cc:412: void
TopSitesImpl::StartQueryForMostVisited() {
On 2016/12/13 22:31:53, msw wrote:
> nit: reorder to match new decl order

Done. There were some other things out of order wrt the declaration order; I've
reordered those too.

[Marc Treib, 2016-12-14 11:18:22]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 11:18:35]

Dry run: 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-14 12:31:45]

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

[commit-bot: I haz the power, 2016-12-14 12:31:46]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2016-12-19 09:14:25]

Ping sdefresne: Please look at components/history/

[sdefresne, 2016-12-19 13:09:59]

On 2016/12/19 09:14:25, Marc Treib (OOO until Jan 12) wrote:
> Ping sdefresne: Please look at components/history/

I'll try to get to it today, but I may not have time. If this is urgent, please
see if you can find another owners. Otherwise, I'll probably have a look
tomorrow if I can't do it today.

[Marc Treib, 2016-12-19 13:12:51]

On 2016/12/19 13:09:59, sdefresne wrote:
> On 2016/12/19 09:14:25, Marc Treib (OOO until Jan 12) wrote:
> > Ping sdefresne: Please look at components/history/
> 
> I'll try to get to it today, but I may not have time. If this is urgent,
please
> see if you can find another owners. Otherwise, I'll probably have a look
> tomorrow if I can't do it today.

Not urgent, it's just a code cleanup. Tomorrow is fine, thanks!

[sdefresne, 2016-12-21 12:57:09]

lgtm

https://codereview.chromium.org/2567043002/diff/60001/components/history/core...
File components/history/core/browser/top_sites_impl.cc (right):

https://codereview.chromium.org/2567043002/diff/60001/components/history/core...
components/history/core/browser/top_sites_impl.cc:334: bool
TopSitesImpl::IsKnownURL(const GURL& url) {
I assume you are just reordering function to match header file here, correct?

[Marc Treib, 2016-12-26 19:56:51]

https://codereview.chromium.org/2567043002/diff/60001/components/history/core...
File components/history/core/browser/top_sites_impl.cc (right):

https://codereview.chromium.org/2567043002/diff/60001/components/history/core...
components/history/core/browser/top_sites_impl.cc:334: bool
TopSitesImpl::IsKnownURL(const GURL& url) {
On 2016/12/21 12:57:09, sdefresne ooo till 2nd Jan 17 wrote:
> I assume you are just reordering function to match header file here, correct?

Yup, exactly. The only change apart from the reordering is that
StartQueryForMostVisited now returns void (previously it returned a
CancelableTaskTracker which was never used).

[Marc Treib, 2016-12-26 19:57:52]

The CQ bit was checked by treib@chromium.org

[Marc Treib, 2016-12-26 19:57:52]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2567043002/#ps60001
(title: "msw nits")

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

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-26 21:16:48]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1482782272066980,
"parent_rev": "fc116e7c978f2076fd42e5173f1891ccc752eb96", "commit_rev":
"6ad98a7ea0babc9e86273117c71258ebf6994ac9"}

[commit-bot: I haz the power, 2016-12-26 21:17:15]

Description was changed from

==========
Cleanup: Remove unused SearchTabHelper::GetOpenUrls

...and all the code that was there to support it, including Browser::GetOpenUrls
and chrome::GetOpenUrls, as well as TopSites::GetCanonicalURLString.
While we're here, also clean up some other unused code from TopSites.

BUG=627747
==========

to

==========
Cleanup: Remove unused SearchTabHelper::GetOpenUrls

...and all the code that was there to support it, including Browser::GetOpenUrls
and chrome::GetOpenUrls, as well as TopSites::GetCanonicalURLString.
While we're here, also clean up some other unused code from TopSites.

BUG=627747

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

[commit-bot: I haz the power, 2016-12-26 21:17:16]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2017-01-02 15:45:15]

Description was changed from

==========
Cleanup: Remove unused SearchTabHelper::GetOpenUrls

...and all the code that was there to support it, including Browser::GetOpenUrls
and chrome::GetOpenUrls, as well as TopSites::GetCanonicalURLString.
While we're here, also clean up some other unused code from TopSites.

BUG=627747

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

to

==========
Cleanup: Remove unused SearchTabHelper::GetOpenUrls

...and all the code that was there to support it, including Browser::GetOpenUrls
and chrome::GetOpenUrls, as well as TopSites::GetCanonicalURLString.
While we're here, also clean up some other unused code from TopSites.

BUG=627747

Committed: https://crrev.com/b4a9ec8b992f2f09f961efdcb4d554d2762a9c22
Cr-Commit-Position: refs/heads/master@{#440723}
==========

[commit-bot: I haz the power, 2017-01-02 15:45:17]

Patchset 4 (id:??) landed as
https://crrev.com/b4a9ec8b992f2f09f961efdcb4d554d2762a9c22
Cr-Commit-Position: refs/heads/master@{#440723}