Strip control characters from omnibox input without losing attributes.

Strip control characters from omnibox input without losing attributes.

BUG=92521
TEST=as in bug

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

[Avi (use Gerrit), 2011-08-12 19:26:46]

For review, plz. (Scott's out?)

[Mark Mentovai, 2011-08-12 19:49:24]

LGTM

http://codereview.chromium.org/7639022/diff/1/chrome/browser/ui/cocoa/locatio...
File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
(right):

http://codereview.chromium.org/7639022/diff/1/chrome/browser/ui/cocoa/locatio...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm:287:
aString = [[aString mutableCopy] autorelease];
Technically you only need to make the copy the first time. Premature
optimization? Maybe.

[Avi (use Gerrit), 2011-08-12 19:55:15]

http://codereview.chromium.org/7639022/diff/1/chrome/browser/ui/cocoa/locatio...
File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
(right):

http://codereview.chromium.org/7639022/diff/1/chrome/browser/ui/cocoa/locatio...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm:287:
aString = [[aString mutableCopy] autorelease];
True, but...

99.99999% of the time there are no control characters, and I don't believe there
is a way to put in two control characters in one input event.

[Mark Mentovai, 2011-08-12 19:57:02]

http://codereview.chromium.org/7639022/diff/1/chrome/browser/ui/cocoa/locatio...
File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
(right):

http://codereview.chromium.org/7639022/diff/1/chrome/browser/ui/cocoa/locatio...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm:287:
aString = [[aString mutableCopy] autorelease];
Avi wrote:
> True, but...
> 
> 99.99999% of the time there are no control characters, and I don't believe
there
> is a way to put in two control characters in one input event.

Then why is there a loop at all?

I wasn’t saying “make the copy even if you don’t need it,” by the way, I meant
to do:

  bool madeCopy = false;
  while (condition) {
    if (!madeCopy)
      aString = [[aString mutableCopy] autorelease];
    // transform aString and recompute input to condition
  }

Not important either way, just clarifying.

[Mark Mentovai, 2011-08-12 19:57:15]

also setting madeCopy to true, obviously :)

[Avi (use Gerrit), 2011-08-12 19:59:59]

On 2011/08/12 19:57:15, Mark Mentovai wrote:
> also setting madeCopy to true, obviously :)

Yes, I knew that's what you meant. Note the comment saying we know this is
inefficient. :)

[Mark Mentovai, 2011-08-12 20:01:47]

Yeah, I know. Like I said it’s LGTM either way, I’m just pointing out an option
that occurred to me.