Make autofill regular expressions unicode again.

Make autofill regular expressions unicode again.

Instead of compiling the source file with UTF8 directly, run it through
a python script that rewrites the UTF8 into C literals.

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

[Ilya Sherman, 2011-09-13 22:06:05]

http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
File chrome/browser/autofill/address_field.cc (right):

http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
chrome/browser/autofill/address_field.cc:28: string16 attention_ignored =
UTF8ToUTF16(autofill::kAttentionIgnoredRe);
nit: Since we're generating these strings with a script, could we skip the
UTF8ToUTF16 calls and generate UTF16 constants directly?

[tony, 2011-09-13 22:28:07]

On 2011/09/13 22:06:05, Ilya Sherman wrote:
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> File chrome/browser/autofill/address_field.cc (right):
> 
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> chrome/browser/autofill/address_field.cc:28: string16 attention_ignored =
> UTF8ToUTF16(autofill::kAttentionIgnoredRe);
> nit: Since we're generating these strings with a script, could we skip the
> UTF8ToUTF16 calls and generate UTF16 constants directly?

There's no such thing on Linux/Mac :)

I'm looking into the browser_tests failure . . .

[Ilya Sherman, 2011-09-13 23:02:23]

On 2011/09/13 22:28:07, tony wrote:
> On 2011/09/13 22:06:05, Ilya Sherman wrote:
> >
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> > File chrome/browser/autofill/address_field.cc (right):
> > 
> >
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> > chrome/browser/autofill/address_field.cc:28: string16 attention_ignored =
> > UTF8ToUTF16(autofill::kAttentionIgnoredRe);
> > nit: Since we're generating these strings with a script, could we skip the
> > UTF8ToUTF16 calls and generate UTF16 constants directly?
> 
> There's no such thing on Linux/Mac :)

Not sure what you mean...  "const char16 kMyConstantString[] = ..." should work
on all platforms.

[tony, 2011-09-13 23:08:55]

On 2011/09/13 23:02:23, Ilya Sherman wrote:
> On 2011/09/13 22:28:07, tony wrote:
> > On 2011/09/13 22:06:05, Ilya Sherman wrote:
> > >
> >
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> > > File chrome/browser/autofill/address_field.cc (right):
> > > 
> > >
> >
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> > > chrome/browser/autofill/address_field.cc:28: string16 attention_ignored =
> > > UTF8ToUTF16(autofill::kAttentionIgnoredRe);
> > > nit: Since we're generating these strings with a script, could we skip the
> > > UTF8ToUTF16 calls and generate UTF16 constants directly?
> > 
> > There's no such thing on Linux/Mac :)
> 
> Not sure what you mean...  "const char16 kMyConstantString[] = ..." should
work
> on all platforms.

What would you put on the right side for char16?  On Windows, we can do L"blah
blah..", but that will generate 4-byte characters on Linux/Mac.  I guess we
could do {0x####, 0x####, ...}, but that seems worse to me.  Maybe it doesn't
matter because it's generated code?

[dhollowa, 2011-09-13 23:26:26]

I'd actually prefer to keep the intermediate output as escaped utf8 because it
is a bit more readable and compatible with text editors.

On 2011/09/13 23:08:55, tony wrote:
> On 2011/09/13 23:02:23, Ilya Sherman wrote:
> > On 2011/09/13 22:28:07, tony wrote:
> > > On 2011/09/13 22:06:05, Ilya Sherman wrote:
> > > >
> > >
> >
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> > > > File chrome/browser/autofill/address_field.cc (right):
> > > > 
> > > >
> > >
> >
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> > > > chrome/browser/autofill/address_field.cc:28: string16 attention_ignored
=
> > > > UTF8ToUTF16(autofill::kAttentionIgnoredRe);
> > > > nit: Since we're generating these strings with a script, could we skip
the
> > > > UTF8ToUTF16 calls and generate UTF16 constants directly?
> > > 
> > > There's no such thing on Linux/Mac :)
> > 
> > Not sure what you mean...  "const char16 kMyConstantString[] = ..." should
> work
> > on all platforms.
> 
> What would you put on the right side for char16?  On Windows, we can do L"blah
> blah..", but that will generate 4-byte characters on Linux/Mac.  I guess we
> could do {0x####, 0x####, ...}, but that seems worse to me.  Maybe it doesn't
> matter because it's generated code?

[Ilya Sherman, 2011-09-13 23:36:32]

On 2011/09/13 23:26:26, dhollowa wrote:
> I'd actually prefer to keep the intermediate output as escaped utf8 because it
> is a bit more readable and compatible with text editors.

What do you mean by "compatible with text editors"?

> 
> On 2011/09/13 23:08:55, tony wrote:
> > On 2011/09/13 23:02:23, Ilya Sherman wrote:
> > > On 2011/09/13 22:28:07, tony wrote:
> > > > On 2011/09/13 22:06:05, Ilya Sherman wrote:
> > > > >
> > > >
> > >
> >
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> > > > > File chrome/browser/autofill/address_field.cc (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
http://codereview.chromium.org/7891020/diff/12/chrome/browser/autofill/addres...
> > > > > chrome/browser/autofill/address_field.cc:28: string16
attention_ignored
> =
> > > > > UTF8ToUTF16(autofill::kAttentionIgnoredRe);
> > > > > nit: Since we're generating these strings with a script, could we skip
> the
> > > > > UTF8ToUTF16 calls and generate UTF16 constants directly?
> > > > 
> > > > There's no such thing on Linux/Mac :)
> > > 
> > > Not sure what you mean...  "const char16 kMyConstantString[] = ..." should
> > work
> > > on all platforms.
> > 
> > What would you put on the right side for char16?  On Windows, we can do
L"blah
> > blah..", but that will generate 4-byte characters on Linux/Mac.  I guess we
> > could do {0x####, 0x####, ...}, but that seems worse to me.  Maybe it
doesn't
> > matter because it's generated code?

Yeah, I was basically thinking of something like {'p', 'a', 0x####, 's', 0} for
"país", for example.  That doesn't seem much more obfuscated than escaped UTF8
to me... but I'll defer to your and David's judgement.

[dhollowa, 2011-09-13 23:48:03]

> What do you mean by "compatible with text editors"?

Escaped UTF-16 would be largely unreadable to humans.

[dhollowa, 2011-09-14 00:27:05]

On 2011/09/13 23:48:03, dhollowa wrote:
> > What do you mean by "compatible with text editors"?
> 
> Escaped UTF-16 would be largely unreadable to humans.

Granted, this is generated file isn't going to be read much.  And removing the
utf8->utf16 conversion is a nice win both for performance and readability in the
main code.  So maybe it is better...

[tony, 2011-09-14 16:44:18]

On 2011/09/14 00:27:05, dhollowa wrote:
> On 2011/09/13 23:48:03, dhollowa wrote:
> > > What do you mean by "compatible with text editors"?
> > 
> > Escaped UTF-16 would be largely unreadable to humans.
> 
> Granted, this is generated file isn't going to be read much.  And removing the
> utf8->utf16 conversion is a nice win both for performance and readability in
the
> main code.  So maybe it is better...

I looked into doing this.  It'll make escape_unicode.py a bit more complicated
since the transform is a bit more complicated.  For example, converting
const char16 foo[] =
  "aaaa"
  "bbbb";
to
const char16 foo[] = {
  'a', 'a', 'a', 'a',
  'b', 'b', 'b', 'b'};
Requires some more logic to place the open and closing braces.  Converting to
\x## is easier since I don't have to track the open and close double quotes or
worry about line continuations.

If \x## is ok, I think this change is ready for review.

[dhollowa, 2011-09-14 17:12:16]

Yes, I'm fine with \x##.  I don't see the new changes however.

On 2011/09/14 16:44:18, tony wrote:
> On 2011/09/14 00:27:05, dhollowa wrote:
> > On 2011/09/13 23:48:03, dhollowa wrote:
> > > > What do you mean by "compatible with text editors"?
> > > 
> > > Escaped UTF-16 would be largely unreadable to humans.
> > 
> > Granted, this is generated file isn't going to be read much.  And removing
the
> > utf8->utf16 conversion is a nice win both for performance and readability in
> the
> > main code.  So maybe it is better...
> 
> I looked into doing this.  It'll make escape_unicode.py a bit more complicated
> since the transform is a bit more complicated.  For example, converting
> const char16 foo[] =
>   "aaaa"
>   "bbbb";
> to
> const char16 foo[] = {
>   'a', 'a', 'a', 'a',
>   'b', 'b', 'b', 'b'};
> Requires some more logic to place the open and closing braces.  Converting to
> \x## is easier since I don't have to track the open and close double quotes or
> worry about line continuations.
> 
> If \x## is ok, I think this change is ready for review.

[tony, 2011-09-14 17:33:24]

On 2011/09/14 17:12:16, dhollowa wrote:
> Yes, I'm fine with \x##.  I don't see the new changes however.

What changes are you looking for?  The generated file isn't in the patch, but if
you want to see it, I've posted it here: http://pastebin.com/hxqhyYQ7

[dhollowa, 2011-09-14 18:05:45]

On 2011/09/14 17:33:24, tony wrote:
> On 2011/09/14 17:12:16, dhollowa wrote:
> > Yes, I'm fine with \x##.  I don't see the new changes however.
> 
> What changes are you looking for?  The generated file isn't in the patch, but
if
> you want to see it, I've posted it here: http://pastebin.com/hxqhyYQ7

I was expecting to see: const char16 foo[] = ...
But I'm seeing: const char foo[] = ...

[tony, 2011-09-14 18:13:06]

On 2011/09/14 18:05:45, dhollowa wrote:
> On 2011/09/14 17:33:24, tony wrote:
> > On 2011/09/14 17:12:16, dhollowa wrote:
> > > Yes, I'm fine with \x##.  I don't see the new changes however.
> > 
> > What changes are you looking for?  The generated file isn't in the patch,
but
> if
> > you want to see it, I've posted it here: http://pastebin.com/hxqhyYQ7
> 
> I was expecting to see: const char16 foo[] = ...
> But I'm seeing: const char foo[] = ...

Oh, I think there's a misunderstanding.  If it's char16, we have to use {
0x####, ...} syntax because char16 is just uint16 on Linux/Mac.  I don't know of
a way to convert "\x##" into a char16 array aside from using UTF8ToUTF16.

[dhollowa, 2011-09-14 18:30:18]

On 2011/09/14 18:13:06, tony wrote:
> On 2011/09/14 18:05:45, dhollowa wrote:
> > On 2011/09/14 17:33:24, tony wrote:
> > > On 2011/09/14 17:12:16, dhollowa wrote:
> > > > Yes, I'm fine with \x##.  I don't see the new changes however.
> > > 
> > > What changes are you looking for?  The generated file isn't in the patch,
> but
> > if
> > > you want to see it, I've posted it here: http://pastebin.com/hxqhyYQ7
> > 
> > I was expecting to see: const char16 foo[] = ...
> > But I'm seeing: const char foo[] = ...
> 
> Oh, I think there's a misunderstanding.  If it's char16, we have to use {
> 0x####, ...} syntax because char16 is just uint16 on Linux/Mac.  I don't know
of
> a way to convert "\x##" into a char16 array aside from using UTF8ToUTF16.

Right, ok.  I'm fine with this then.  LGTM.

[commit-bot: I haz the power, 2011-09-15 03:35:55]

Change committed as 101236