Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

Properly calculate match locations for various inputs, e.g.,
"://ww" should count as a hostname match because that's where
the main word part starts.

BUG=651654
Committed: https://crrev.com/1ca732414effb6ee392b4e49a02892111783a2e3
Cr-Commit-Position: refs/heads/master@{#426252}

[Mark P, 2016-10-17 23:13:41]

Description was changed from

==========
Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

Properly calculate match locations for various inputs, e.g.,
"://ww" should count as a hostname match because that's where
the main word part starts.

BUG=651654
==========

to

==========
Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

Properly calculate match locations for various inputs, e.g.,
"://ww" should count as a hostname match because that's where
the main word part starts.

BUG=651654
==========

[Mark P, 2016-10-17 23:13:42]

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

[Mark P, 2016-10-17 23:14:44]

Peter,

This is uncomplicated.  Please take a look at your leisure.

thanks,
mark

[Peter Kasting, 2016-10-17 23:34:38]

LGTM

https://codereview.chromium.org/2421373003/diff/20001/components/omnibox/brow...
File components/omnibox/browser/scored_history_match_unittest.cc (right):

https://codereview.chromium.org/2421373003/diff/20001/components/omnibox/brow...
components/omnibox/browser/scored_history_match_unittest.cc:560:
ASCIIToUTF16("abc"), url, title);
Nit: Define a lambda here (not sure how git cl format would do this):

  auto Score = [&](char* term) {
    return GetTopicalityScoreOfTermAgainstURLAndTitle(
        ASCIIToUTF16(term), url, title);
  }

Now all these lines can just do

  const float hostname_score = Score("abc");

Better still, this syntax is now so brief you can inline some of these temps
below:

  EXPECT_EQ(hostname_score, Score("://abc"));

Or, if you don't mind duplicating some of the work:

  EXPECT_EQ(Score("abc"), Score("://abc"));

[Mark P, 2016-10-18 22:57:30]

Please take a look at your leisure.

--mark

https://codereview.chromium.org/2421373003/diff/20001/components/omnibox/brow...
File components/omnibox/browser/scored_history_match_unittest.cc (right):

https://codereview.chromium.org/2421373003/diff/20001/components/omnibox/brow...
components/omnibox/browser/scored_history_match_unittest.cc:560:
ASCIIToUTF16("abc"), url, title);
On 2016/10/17 23:34:37, Peter Kasting wrote:
> Nit: Define a lambda here (not sure how git cl format would do this):
> 
>   auto Score = [&](char* term) {
>     return GetTopicalityScoreOfTermAgainstURLAndTitle(
>         ASCIIToUTF16(term), url, title);
>   }
> Now all these lines can just do
> 
>   const float hostname_score = Score("abc");

Good idea.  I find my aging brain doesn't think of new C++ features to use when
I should.  Done.

I decided not to inline the scores in the comparisons below because I think the
names as useful descriptions.  Otherwise a casual reader of the code would have
to continue referring back to the URL as the top.  It also makes clear the
distinction between certain things, e.g., "://abc" should be a hostname match,
not a scheme match.

> 
> Better still, this syntax is now so brief you can inline some of these temps
> below:
> 
>   EXPECT_EQ(hostname_score, Score("://abc"));
> 
> Or, if you don't mind duplicating some of the work:
> 
>   EXPECT_EQ(Score("abc"), Score("://abc"));

[Peter Kasting, 2016-10-19 01:08:31]

LGTM

[Mark P, 2016-10-19 03:45:52]

The CQ bit was checked by mpearson@chromium.org

[commit-bot: I haz the power, 2016-10-19 03:46:45]

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-10-19 05:51:35]

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

[commit-bot: I haz the power, 2016-10-19 05:51:36]

Exceeded global retry quota

[Mark P, 2016-10-19 17:57:04]

The CQ bit was checked by mpearson@chromium.org

[commit-bot: I haz the power, 2016-10-19 17:57:50]

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-10-19 18:52:53]

Description was changed from

==========
Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

Properly calculate match locations for various inputs, e.g.,
"://ww" should count as a hostname match because that's where
the main word part starts.

BUG=651654
==========

to

==========
Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

Properly calculate match locations for various inputs, e.g.,
"://ww" should count as a hostname match because that's where
the main word part starts.

BUG=651654
==========

[commit-bot: I haz the power, 2016-10-19 18:52:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-21 13:10:26]

Description was changed from

==========
Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

Properly calculate match locations for various inputs, e.g.,
"://ww" should count as a hostname match because that's where
the main word part starts.

BUG=651654
==========

to

==========
Omnibox: Improve HQP Scoring for Terms that Start with Punctuation

Properly calculate match locations for various inputs, e.g.,
"://ww" should count as a hostname match because that's where
the main word part starts.

BUG=651654

Committed: https://crrev.com/1ca732414effb6ee392b4e49a02892111783a2e3
Cr-Commit-Position: refs/heads/master@{#426252}
==========

[commit-bot: I haz the power, 2016-10-21 13:10:27]

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