Remove wstring from autocomplete....

Remove wstring from autocomplete.

BUG=23581
TEST=no visible changes; all tests pass


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72380

[Avi (use Gerrit), 2011-01-21 18:23:08]

Some fixes already made based on my own review.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
File chrome/browser/autocomplete/autocomplete.cc (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete.cc:144: if
(LowerCaseEqualsASCII(parsed_scheme, chrome::kFileScheme)) {
Yes, this change is deliberate. LowerCaseEqualsASCII below predates this later
addition to the file.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete.cc:257: if (registry_length ==
string16::npos) {
std::string::npos; fixed already.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete.cc:265: UTF16ToUTF8(host_with_tld),
false) != string16::npos)
Ditto.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/ui/toolbar/t...
File chrome/browser/ui/toolbar/toolbar_model.cc (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/ui/toolbar/t...
chrome/browser/ui/toolbar/toolbar_model.cc:56: return UTF16ToWideHack(
This file reverted.

[Avi (use Gerrit), 2011-01-24 19:06:59]

Updated; please review.

[evanm, 2011-01-24 19:14:14]

Crap, I wrote my comments before but never sent them.  Sorry if they are
obosolte now.  :(

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
File chrome/browser/autocomplete/autocomplete.cc (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete.cc:180:
ExternalProtocolHandler::GetBlockState(UTF16ToUTF8(parsed_scheme))) {
Might be easier to read if you put getting the block state into a temp variable
before the switch.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete.cc:280: &host_info));
Are these APIs really in terms of UTF-8?  (I'm wondering about i18n TLDs)

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
File chrome/browser/autocomplete/autocomplete_edit.h (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete_edit.h:97: virtual string16 GetTitle()
const = 0;
Scary!

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
File chrome/browser/autocomplete/autocomplete_unittest.cc (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete_unittest.cc:350:
AutocompleteInput::QUERY },  // Invalid IPv6 literal.
Might have been simpler to leave them wchar_t* and done the conversion in the
loop below, but I don't care too much.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/automation/t...
File chrome/browser/automation/testing_automation_provider.h (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/automation/t...
chrome/browser/automation/testing_automation_provider.h:691: static
std::map<AutoFillFieldType, std::string>
Why did we use UTF-8 here?  You can autofill an e.g. non-ASCII first name.  This
should have been string16.  :(

I know it's not your fault, but perhaps we should use string16 here and convert
to UTF-8 at the last second to avoid bad API creep.

[Avi (use Gerrit), 2011-01-24 19:34:01]

No problem, man. No real change since patchset 2, just updating to avoid bitrot.

Will go through your changes.

[Avi (use Gerrit), 2011-01-24 19:46:42]

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
File chrome/browser/autocomplete/autocomplete.cc (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete.cc:180:
ExternalProtocolHandler::GetBlockState(UTF16ToUTF8(parsed_scheme))) {
On 2011/01/24 19:14:14, evanm wrote:
> Might be easier to read if you put getting the block state into a temp
variable
> before the switch.

Done.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete.cc:280: &host_info));
On 2011/01/24 19:14:14, evanm wrote:
> Are these APIs really in terms of UTF-8?  (I'm wondering about i18n TLDs)

Yes, they are. First note that I didn't change the return type; even the wstring
version returns a string. Second, the implementation of the wstring version is
to convert to UTF-8 and call the string version...

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
File chrome/browser/autocomplete/autocomplete_edit.h (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete_edit.h:97: virtual string16 GetTitle()
const = 0;
On 2011/01/24 19:14:14, evanm wrote:
> Scary!

I know. A smaller-scoped change than all of views, though. Plus I'm protected
because if the type change doesn't match on any overrider, it's a compiler
error.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
File chrome/browser/autocomplete/autocomplete_unittest.cc (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/autocomplete...
chrome/browser/autocomplete/autocomplete_unittest.cc:350:
AutocompleteInput::QUERY },  // Invalid IPv6 literal.
On 2011/01/24 19:14:14, evanm wrote:
> Might have been simpler to leave them wchar_t* and done the conversion in the
> loop below, but I don't care too much.

True. I hate wchar_t almost as much as wstring so I've been trying to kill those
too.

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/automation/t...
File chrome/browser/automation/testing_automation_provider.h (right):

http://codereview.chromium.org/6258015/diff/18003/chrome/browser/automation/t...
chrome/browser/automation/testing_automation_provider.h:691: static
std::map<AutoFillFieldType, std::string>
On 2011/01/24 19:14:14, evanm wrote:
> Why did we use UTF-8 here?  You can autofill an e.g. non-ASCII first name.

That's not what this map is for. This map does not contain any user-provided
autofill data. It contains autofill keys. Please see the implementations of
GetAutoFillFieldToStringMap and GetCreditCardFieldToStringMap to see how they're
used.

[evanm, 2011-01-24 19:56:23]

LGTM, though I admit I skimmed