Clean up SSL false start blacklist code. Numerous changes, including:

Clean up SSL false start blacklist code.  Numerous changes, including:
  * Handle trailing dots in LastTwoLabels() as in http://codereview.chromium.org/7518035/ .  Rename this function to LastTwoComponents() to match the terminology used in the RegistryControlledDomainService and elsewhere in Chrome.
  * Since callers are using std::string anyway, make the functions in the header take const std::string& instead of char*.  This also allows doing string operations on them.
  * Use string operations (like find_last_of()) in place of hand-written algorithms, for brevity, clarity, and safety.
  * Avoid "unsigned", which the style guide forbids, and use allowed types like size_t, uint32, or int (depending on the situation).
  * Avoid #define and "using".
  * Use standard algorithms for similar reasons as using string ops.
  * Use file_util functions to significantly abbreviate file reading/writing code.
  * Use wmain() (on Windows) in combination with FilePath to avoid issues if the provided pathname has extended characters that don't flatten losslessly to the default codepage (thanks Darin for pointing out this issue).
  * Avoid casting where possible.  Avoid some casts for printf()-style calls by using a string stream, which also allows for slightly less boilerplate.
  * Convert non-error uses of stderr to the chrome-standard VLOG(1).
  * Correctly handle hostnames with trailing dots in the input file.
  * In general, shorten code where possible.

Because this adds a dependency on base, and ssl_false_start_blacklist_process has the "#host" specifier in net.gyp, bradnelson tells me that base and its dependencies need an explicit "host, target" toolchain list for the Linux builds to work correctly.  It would be nice if we could avoid this but I guess gyp would have to be smarter or something.

BUG=none
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95907

[Peter Kasting, 2011-08-02 21:12:09]

I got a bit carried away with this, but the end result was so much shorter and
seemed more style-compliant and (IMO) readable that I figured it was worth
sending.

Generally it's easier to just read the new code than to try to read the diffs
between new and old, since the diffs are so large.

[agl, 2011-08-02 21:39:16]

LGTM. The original code was designed to avoid depending on Chromium code (and to
be a C-like as possible) in order that debugging via try-server wasn't as
painful.

I agree that the code is nicer with the extra dependencies. I hope you can get
it to compile everywhere! With a bit of luck, all this should be gone by 2012.

http://codereview.chromium.org/7550002/diff/7001/net/base/ssl_false_start_bla...
File net/base/ssl_false_start_blacklist_process.cc (right):

http://codereview.chromium.org/7550002/diff/7001/net/base/ssl_false_start_bla...
net/base/ssl_false_start_blacklist_process.cc:64: for (Hosts::const_iterator
i(first_dupe); i != hosts->end(); ++i)
The SGI STL spec says that the iterators after |first_dupe| are undefined. Since
printing the duplicates isn't very useful at this stage, I think you can just
remove this loop.

http://codereview.chromium.org/7550002/diff/7001/net/base/ssl_false_start_bla...
net/base/ssl_false_start_blacklist_process.cc:105:
hosts->erase(std::remove_if(hosts->begin(), hosts->end(),
ParentInSet(hosts_set)),
(nit) 80 chars

http://codereview.chromium.org/7550002/diff/7001/net/base/ssl_false_start_bla...
net/base/ssl_false_start_blacklist_process.cc:134:
buckets[net::SSLFalseStartBlacklist::Hash(
I think that this is too much for one line. Maybe:

  size_t hash = net::SSLFalseStartBlacklist::Hash(
      net::SSLFalseStartBlacklist::LastTwoComponents(*i));
  hash &= net::SSLFalseStartBlacklist::kBuckets - 1;
  buckets[hash].push_back(*i);

http://codereview.chromium.org/7550002/diff/7001/net/net.gyp
File net/net.gyp (right):

http://codereview.chromium.org/7550002/diff/7001/net/net.gyp#newcode1340
net/net.gyp:1340:
'../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
I'll take your word on this.

[agl, 2011-08-02 21:44:25]

On Tue, Aug 2, 2011 at 5:39 PM,  <agl@chromium.org> wrote:
> I agree that the code is nicer with the extra dependencies. I hope you can
> get
> it to compile everywhere! With a bit of luck, all this should be gone by
> 2012.

And, by the way, thank you!


AGL

[Peter Kasting, 2011-08-02 23:37:03]

I'll upload a new patch once bradnelson and I have nailed down exactly how to
make the ARM cross-compile build work correctly.

http://codereview.chromium.org/7550002/diff/7001/net/base/ssl_false_start_bla...
File net/base/ssl_false_start_blacklist_process.cc (right):

http://codereview.chromium.org/7550002/diff/7001/net/base/ssl_false_start_bla...
net/base/ssl_false_start_blacklist_process.cc:64: for (Hosts::const_iterator
i(first_dupe); i != hosts->end(); ++i)
On 2011/08/02 21:39:16, agl wrote:
> The SGI STL spec says that the iterators after |first_dupe| are undefined.
Since
> printing the duplicates isn't very useful at this stage, I think you can just
> remove this loop.

Done.

http://codereview.chromium.org/7550002/diff/7001/net/base/ssl_false_start_bla...
net/base/ssl_false_start_blacklist_process.cc:105:
hosts->erase(std::remove_if(hosts->begin(), hosts->end(),
ParentInSet(hosts_set)),
On 2011/08/02 21:39:16, agl wrote:
> (nit) 80 chars

Fixed.

http://codereview.chromium.org/7550002/diff/7001/net/base/ssl_false_start_bla...
net/base/ssl_false_start_blacklist_process.cc:134:
buckets[net::SSLFalseStartBlacklist::Hash(
On 2011/08/02 21:39:16, agl wrote:
> I think that this is too much for one line.

Yeah, it's a big chunk.  Split into two pieces.

[Peter Kasting, 2011-08-08 21:13:04]

Brad, it looks as if this latest set of changes compiles on Linux.  Can you
review the .gyp and .gypi diffs here?

[bradn, 2011-08-08 22:24:41]

LGTM, sigh

[Peter Kasting, 2011-08-08 23:09:39]

+willchan: Can you OWNERS r+ for base and net?

[willchan no longer on Chromium, 2011-08-08 23:14:09]

On 2011/08/08 23:09:39, Peter Kasting wrote:
> +willchan: Can you OWNERS r+ for base and net?

LGTM