New issue
Advanced search Search tips

Issue 820198 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

net/http/http_util.h and StringTokenizer aren't StringPiece-friendly

Project Member Reported by davidben@chromium.org, Mar 8 2018

Issue description

net/http/http_util.h has a lot of random const_iterator functions, which makes it difficult to, e.g., use a NameValuePairsIterator on a StringPiece. (While one can go from std::string::const_iterator to StringPiece, the converse does not work.)

This is rather tied up everywhere, all the way down to base::StringTokenizer not being StringPiece-friendly. There is base::CStringTokenizer, which works, though it relies on StringPiece's iterators being pointers, which isn't true of std::string_view. I'll try to chew through that as well, though I suspect I'll end up switching the HTTP parsers CStringTokenizer rather than finishing merging CStringTokenizer.

Filing this bug to track all the random StringPiece CLs.
 
Are there specific perf issues you're concerned about, or is this just more general code health concerns?  I'm fine with this, either way - StringPiece is awesome, and we should be using it more in net/, I'm just curious.
Mostly code health. Also I wanted to switch MerkleIntegritySourceStream's header parser to one of the iterators, and I feel silly having just switched it to StringPiece (so the substr call didn't copy) and switching it right back. :-)
Project Member

Comment 4 by bugdroid1@chromium.org, May 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/001b02f1ef8393cff0015c13940405e7d6abc340

commit 001b02f1ef8393cff0015c13940405e7d6abc340
Author: David Benjamin <davidben@chromium.org>
Date: Tue May 08 02:13:51 2018

Switch an easy token_begin/token_end use to token_piece.

Bug: 820198
Change-Id: I8e1992390ac2bbb6f7b2e5a4409cc739ddb5ad60
Reviewed-on: https://chromium-review.googlesource.com/955729
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556650}
[modify] https://crrev.com/001b02f1ef8393cff0015c13940405e7d6abc340/base/sys_info_chromeos.cc

Components: Internals>Network
Components: -Internals>Network>HTTP

Sign in to add a comment