Omnibox - Allow Matching in Port Number

Omnibox - Allow Matching in Port Number

Previously the port number was misidentified as the registry (.com, .org,
etc.).  The registry isn't used for scoring, which means the matching the
port number wouldn't be allowed.  Now they're allowed.

BUG=595524

[Mark P, 2016-09-20 19:31:36]

Description was changed from

==========
Omnibox - Allow Matching in Port Number

Previously the port number was misidentified as the registry (.com, .org,
etc.).  The registry isn't used for scoring, which means the matching the
port number wouldn't be allowed.  Now they're allowed.

BUG=595524
==========

to

==========
Omnibox - Allow Matching in Port Number

Previously the port number was misidentified as the registry (.com, .org,
etc.).  The registry isn't used for scoring, which means the matching the
port number wouldn't be allowed.  Now they're allowed.

BUG=595524
==========

[Mark P, 2016-09-20 19:31:36]

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

[Mark P, 2016-09-20 19:31:56]

Peter,

Please take a look at your leisure.

thanks,
mark

[Peter Kasting, 2016-09-20 20:44:56]

https://codereview.chromium.org/2355053003/diff/1/components/omnibox/browser/...
File components/omnibox/browser/scored_history_match.cc (right):

https://codereview.chromium.org/2355053003/diff/1/components/omnibox/browser/...
components/omnibox/browser/scored_history_match.cc:485: port_pos =
std::string::npos;
I'm really uncomfortable with all this code.

A claim (on a different bug) was that we cannot use the eTLD service because of
perf concerns.  But the code in this function (not just in this changed block)
is already really inefficient because it does a bunch of different find() calls
that each can scan the whole string.  And the chance for errors is worrisome. 
There are already multiple known errors in this that "aren't worth worrying
about" and the aforementioned open bug that you punted.  What else is
problematic, now or later?

It seems like rewriting to use the standard machinery for parsing URLs can
locate things like the hostname boundaries, port, CGI params, etc. with complete
accuracy, in noticeably less code, and I'm not convinced will lose performance. 
I think it is worth taking the time to do.

[Peter Kasting, 2017-02-11 01:58:54]

Should this be closed?