Remove ScoreHistoryMatch::Builder

Remove ScoreHistoryMatch::Builder

ScoredHistoryMatch::Builder was introduced to allow keeping ScoredHistoryMatch
in the history component without having a dependency on autocomplete code. As
ScoredHistoryMatch has been moved to chrome/browser/autocomplete this makes
the code more complex than necessary, so remove it.

BUG=462645
Committed: https://crrev.com/c38a45a25bac176d98e91aed57d87ff9c4d14e31
Cr-Commit-Position: refs/heads/master@{#320918}

[sdefresne, 2015-03-06 10:03:30]

sdefresne@chromium.org changed reviewers:
+ mpearson@chromium.org

[sdefresne, 2015-03-06 10:03:36]

Please take a look.

[sdefresne, 2015-03-06 14:32:57]

sdefresne@chromium.org changed reviewers:
+ jcivelli@chromium.org

[sdefresne, 2015-03-06 14:32:57]

+jcivelli@chromium.org for chrome/test/base/testing_profile.h

[Mark P, 2015-03-10 00:28:12]

changelist description:
"the code more complete than necessary"?
Did you mean more complex?

--mark

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match.cc (right):

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:13: #include
"base/metrics/histogram.h"
Why do you need this?

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:17: #include
"base/strings/string_util.h"
Why do you need this?

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:20: #include
"components/bookmarks/browser/bookmark_utils.h"
Why do you need this?

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:21: #include
"components/history/core/browser/history_client.h"
why do you need this?

In general, where did you get all these from?  Their appearance makes me wonder
about the state of the rest of this file.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:28: // The number of days of
recency scores to precompute.
Did you modify any of this anonymous namespace stuff at all?

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:137:
ScoredHistoryMatch::ScoredHistoryMatch(
I assume you didn't change the content of this function in any meaningful way,
right?

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:345: // static
I assume you didn't change the content of any of these functions here and below?

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match.h (right):

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.h:43: // score.  The raw score
allows the matches to be ordered and can be/ used to
nit: extra "/"

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.h:101: private:
Did you modify anything in this large block?

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match_builder_impl.cc (right):

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match_builder_impl.cc:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
What's going on here?  I thought you were deleting this file?

And don't forget the corresponding .h file as well.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match_unittest.cc (right):

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match_unittest.cc:72: std::set<GURL>
bookmarked_urls_;
Do you need this and TearDown anymore given your new implementation?

[sdefresne, 2015-03-10 10:43:29]

PTAL

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match.cc (right):

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:13: #include
"base/metrics/histogram.h"
On 2015/03/10 00:28:12, Mark P wrote:
> Why do you need this?

Removed.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:17: #include
"base/strings/string_util.h"
On 2015/03/10 00:28:12, Mark P wrote:
> Why do you need this?

string_util.h is required for IsWhitespace.

utf_string_conversions.h is required for base::UTF8ToUTF16.

utf_offset_string_conversions.h is required for base::OffsetAdjuster.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:20: #include
"components/bookmarks/browser/bookmark_utils.h"
On 2015/03/10 00:28:12, Mark P wrote:
> Why do you need this?

bookmark_utils.h is needed for
  bookmarks::CleanUpUrlForMatching
  bookmarks::CleanUpTitleForMatching

history_client.h is not required. Removed.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:21: #include
"components/history/core/browser/history_client.h"
On 2015/03/10 00:28:12, Mark P wrote:
> why do you need this?
> 
> In general, where did you get all these from?  Their appearance makes me
wonder
> about the state of the rest of this file.

I copied them from
chrome/browser/autocomplete/scored_history_match_builder_impl.cc.
I've removed the one that were unnecessary.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:28: // The number of days of
recency scores to precompute.
On 2015/03/10 00:28:12, Mark P wrote:
> Did you modify any of this anonymous namespace stuff at all?

No.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:137:
ScoredHistoryMatch::ScoredHistoryMatch(
On 2015/03/10 00:28:12, Mark P wrote:
> I assume you didn't change the content of this function in any meaningful way,
> right?

The function was creating a local variable scored_history_match and setting its
fields. I've changed the function to instead initialise the current object, so
"scored_history_match.some_property = some_value" became "some_property =
some_value".

I also added a call to ScoredHistoryMatch::Init() to ensure that initialisation
of the static fields did happen before they were used. I also changed in the
other methods "DCHECK(initialized)" by "ScoredHistoryMatch::Init()" for the same
reason.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.cc:345: // static
On 2015/03/10 00:28:12, Mark P wrote:
> I assume you didn't change the content of any of these functions here and
below?

As mentioned in the previous comment, I did change "DCHECK(initialied)" into
"ScoredHistoryMatch::Init()".

I also changed GetTopicalityScore() to remove the last "ScoredHistoryMatch*
scored_history_match" parameters, and removing the "static" modifier so that it
would now act on "this".

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match.h (right):

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.h:43: // score.  The raw score
allows the matches to be ordered and can be/ used to
On 2015/03/10 00:28:12, Mark P wrote:
> nit: extra "/"

Done.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.h:101: private:
On 2015/03/10 00:28:12, Mark P wrote:
> Did you modify anything in this large block?

I did only replace ScoredHistoryMatchBuilderImpl by ScoredHistoryMatch.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match_builder_impl.cc (right):

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match_builder_impl.cc:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
On 2015/03/10 00:28:12, Mark P wrote:
> What's going on here?  I thought you were deleting this file?
> 
> And don't forget the corresponding .h file as well.

I locally deleted it, but forgot to do "git rm". Fixed.

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match_unittest.cc (right):

https://codereview.chromium.org/976423002/diff/40001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match_unittest.cc:72: std::set<GURL>
bookmarked_urls_;
On 2015/03/10 00:28:12, Mark P wrote:
> Do you need this and TearDown anymore given your new implementation?

No. Removed.

[Mark P, 2015-03-12 21:31:37]

lgtm
with one minor comment

https://codereview.chromium.org/976423002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match.h (right):

https://codereview.chromium.org/976423002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.h:115: // url_matches and
title_matches of |scored_history_match| in the process so
update this comment.  This variable no longer exists.

[sdefresne, 2015-03-12 21:36:21]

Thank you.

https://codereview.chromium.org/976423002/diff/60001/chrome/browser/autocompl...
File chrome/browser/autocomplete/scored_history_match.h (right):

https://codereview.chromium.org/976423002/diff/60001/chrome/browser/autocompl...
chrome/browser/autocomplete/scored_history_match.h:115: // url_matches and
title_matches of |scored_history_match| in the process so
On 2015/03/12 21:31:37, Mark P wrote:
> update this comment.  This variable no longer exists.

Done.

[sdefresne, 2015-03-17 10:03:34]

jcivelli: ping for OWNERS of
  chrome/test/base/testing_profile.cc

Can you check CQ if l-g-t-m?

[Jay Civelli, 2015-03-17 14:58:59]

The CQ bit was checked by jcivelli@chromium.org

[Jay Civelli, 2015-03-17 14:58:59]

lgtm

[Jay Civelli, 2015-03-17 14:59:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/976423002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2015-03-17 14:59:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/976423002/100001

[commit-bot: I haz the power, 2015-03-17 15:32:30]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-03-17 15:33:04]

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