Fix regression caused by CL 16142...

Fix regression caused by CL 16142

This CL fixes regression caused by CL 16142: Improve key event handling of AutocompleteEditViewGtk.

BUG=19193
: omnibox blocks ctrl-* commands
BUG=19199
: omnibox ignores history, automatically searchs
TEST=Open chrome, move cursor into omnibox then press ctrl-t to see if a new tab is opened.
TEST=Open chrome, input something in omnibox and make sure an url is matched and some text is highlighted in omnibox, then press Enter to see if the url is opened.


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

[James Su, 2009-08-13 15:10:35]

Hi,
  This is a CL to fix regression caused by my previous CL 16142: Improve key
event handling of AutocompleteEditViewGtk. Please help review.
  Sorry for my careless.

James Su

[Dean McNamee, 2009-08-13 15:34:23]

Can you give me some more background on the problems? (And write them in the CL
description).  I don't exactly understand everything you're trying to work
around and way.

This code got very complicated...

http://codereview.chromium.org/165457/diff/1/2
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/165457/diff/1/2#newcode807
Line 807: if (1 == len) {
I was really trying to avoid doing something like this.  What if you paste a
single character?

[James Su, 2009-08-13 16:52:00]

Hi,
  I updated the comment in HandleKeyPress() to explain this piece of complicated
code. Hope it can make things clear. Please help review.

Regards
James Su 

On 2009/08/13 15:34:23, Dean McNamee wrote:
> Can you give me some more background on the problems? (And write them in the
CL
> description).  I don't exactly understand everything you're trying to work
> around and way.
> 
> This code got very complicated...
> 
> http://codereview.chromium.org/165457/diff/1/2
> File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):
> 
> http://codereview.chromium.org/165457/diff/1/2#newcode807
> Line 807: if (1 == len) {
> I was really trying to avoid doing something like this.  What if you paste a
> single character?

[Dean McNamee, 2009-08-13 17:38:41]

Thanks for the additional comments, I now understand what we're doing.

As for saving and restoring the omnibox text, I am worried that is too heavy.  I
had some ideas about instead removing the selection so overwrite doesn't happen,
but I'm not sure that works...

http://codereview.chromium.org/165457/diff/8/1003
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/165457/diff/8/1003#newcode878
Line 878: }
I now understand this logic, but the code needs to be simplified.

How about doing this.  Instead of having new_line_inserted_ and tab_inserted_,
we have a single 'char last_char_inserted_'.  Then you can just always set this
without any logic, and check it in the key press handler.

[Evan Stade, 2009-08-13 18:26:17]

http://codereview.chromium.org/165457/diff/8/1003
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/165457/diff/8/1003#newcode554
Line 554: // So that if new_line_inserted_ is set to true after calling
|text_view_|'s
s/new_line_inserted_/tab_inserted_

(?)

http://codereview.chromium.org/165457/diff/8/1003#newcode557
Line 557: // perform the special behavior of Tab key safely.
If a user pastes a tab character into the omnibox, would we get a key press
event, and find a tab key had been inserted?

http://codereview.chromium.org/165457/diff/8/1003#newcode564
Line 564: // content of omnibox before sending the key evet to |text_view_|, and
then
s/evet/event

[James Su, 2009-08-13 22:29:40]

Hi,
  CL updated according to comments.
  Removing selection won't work, because the result will be wrong if the
selection is removed but IME handles the key event and generates some input.

Regards
James Su

http://codereview.chromium.org/165457/diff/8/1003
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/165457/diff/8/1003#newcode557
Line 557: // perform the special behavior of Tab key safely.
On 2009/08/13 18:26:17, Evan Stade wrote:
> If a user pastes a tab character into the omnibox, would we get a key press
> event, and find a tab key had been inserted?

copy/paste won't generate key event.

http://codereview.chromium.org/165457/diff/8/1003#newcode564
Line 564: // content of omnibox before sending the key evet to |text_view_|, and
then
On 2009/08/13 18:26:17, Evan Stade wrote:
> s/evet/event

Done.

http://codereview.chromium.org/165457/diff/8/1003#newcode878
Line 878: }
On 2009/08/13 17:38:41, Dean McNamee wrote:
> I now understand this logic, but the code needs to be simplified.
> 
> How about doing this.  Instead of having new_line_inserted_ and tab_inserted_,
> we have a single 'char last_char_inserted_'.  Then you can just always set
this
> without any logic, and check it in the key press handler.

Done.

[Dean McNamee, 2009-08-13 22:40:11]

http://codereview.chromium.org/165457/diff/12/1007
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/165457/diff/12/1007#newcode597
Line 597: tab_pressed = (tab_pressed && single_char_inserted_ == '\t');
This is confusing, now we have the same variable (tab pressed) that means two
different things.  First it means if the key was pressed.  Then it means if the
key was pressed and not handled my IME.  Maybe tab_inserted?  I dunno.

http://codereview.chromium.org/165457/diff/12/1007#newcode858
Line 858: // If len equals to 1, then the text might be generated by a key
event.
If there was only a single character... (vs equals to 1)

http://codereview.chromium.org/165457/diff/12/1007#newcode862
Line 862: if (1 == len)
we usually do (len == 1) style in Chrome.

http://codereview.chromium.org/165457/diff/12/1007#newcode863
Line 863: single_char_inserted_ = *text;
text[0]?  Same but maybe more clear.

http://codereview.chromium.org/165457/diff/12/1008
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.h (right):

http://codereview.chromium.org/165457/diff/12/1008#newcode373
Line 373: int single_char_inserted_;
char ?  last_single_ ?

[James Su, 2009-08-13 22:52:49]

CL updated.

Regards
James Su

http://codereview.chromium.org/165457/diff/12/1007
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/165457/diff/12/1007#newcode597
Line 597: tab_pressed = (tab_pressed && single_char_inserted_ == '\t');
On 2009/08/13 22:40:11, Dean McNamee wrote:
> This is confusing, now we have the same variable (tab pressed) that means two
> different things.  First it means if the key was pressed.  Then it means if
the
> key was pressed and not handled my IME.  Maybe tab_inserted?  I dunno.

Done.

http://codereview.chromium.org/165457/diff/12/1007#newcode858
Line 858: // If len equals to 1, then the text might be generated by a key
event.
On 2009/08/13 22:40:11, Dean McNamee wrote:
> If there was only a single character... (vs equals to 1)

Done.

http://codereview.chromium.org/165457/diff/12/1007#newcode862
Line 862: if (1 == len)
On 2009/08/13 22:40:11, Dean McNamee wrote:
> we usually do (len == 1) style in Chrome.

Done.

http://codereview.chromium.org/165457/diff/12/1007#newcode863
Line 863: single_char_inserted_ = *text;
On 2009/08/13 22:40:11, Dean McNamee wrote:
> text[0]?  Same but maybe more clear.

Done.

[Dean McNamee, 2009-08-13 22:58:17]

ok after fixing the type of char_inserted_.

Thanks

http://codereview.chromium.org/165457/diff/1011/16
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.h (right):

http://codereview.chromium.org/165457/diff/1011/16#newcode373
Line 373: int char_inserted_;
Should this be of type char?

[James Su, 2009-08-13 23:11:54]

Is it ok to submit this CL now?

Regards
James Su

http://codereview.chromium.org/165457/diff/1011/16
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.h (right):

http://codereview.chromium.org/165457/diff/1011/16#newcode373
Line 373: int char_inserted_;
On 2009/08/13 22:58:17, Dean McNamee wrote:
> Should this be of type char?

Done.

[Daniel Erat, 2009-08-13 23:36:38]

This is really tricky, but if it's the only way to get it working, I'm okay with
it.  I just have a bunch of picky grammar stuff about the comments.

http://codereview.chromium.org/165457/diff/2001/2002
File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):

http://codereview.chromium.org/165457/diff/2001/2002#newcode518
Line 518: // The omnibox supports several special behavior which may be
triggered by
s/behavior/behaviors/

http://codereview.chromium.org/165457/diff/2001/2002#newcode530
Line 530: // Then if the key event is one of Tab, Enter and Escape, we need
trigger
s/need trigger/need to trigger the/

http://codereview.chromium.org/165457/diff/2001/2002#newcode542
Line 542: // built-in operation if IME did not handle the key event. Because we
don't
change to "... and avoid the above built-in operation if IME did not handle the
key event, because we don't want ..."

http://codereview.chromium.org/165457/diff/2001/2002#newcode554
Line 554: // So if char_inserted_ equals to '\t' after calling |text_view_|'s
s/equals to '\t'/equals '\t'/

http://codereview.chromium.org/165457/diff/2001/2002#newcode555
Line 555: // default signal handler against a Tab key press event, then we can
sure this
switch to "then we know that the Tab key press event was handled by GtkTextView
rather than IME, and can perform the special behavior for Tab safely."

http://codereview.chromium.org/165457/diff/2001/2002#newcode563
Line 563: // it, we use backup and restore trick: If Tab or Enter is pressed,
backup the
s/we use backup/we use a backup/

http://codereview.chromium.org/165457/diff/2001/2002#newcode568
Line 568: bool tab_pressed = ((event->keyval == GDK_Tab ||
insert a blank line before this line?  it looks strange pressed up against the
klass definition when there's a blank line between it and enter_pressed

http://codereview.chromium.org/165457/diff/2001/2002#newcode587
Line 587: // Reset these variable, which may be set in "insert-text" signal
handler.
change to "Reset |char_inserted_|, which may be set in the "insert-text" signal
handler, so that we'll know if a Tab or Enter key event was handled by IME."

(side note: just "IME" or "an IME" / "the IME"?)

http://codereview.chromium.org/165457/diff/2001/2002#newcode629
Line 629: // We can handle Escape key if |text_view_| did not handle it.
s/can handle Escape/can handle the Escape/

http://codereview.chromium.org/165457/diff/2001/2002#newcode630
Line 630: // If it's not handled by us, then we need propagate it upto parent
change to "... then we need to propagate it up to the parent ..."

http://codereview.chromium.org/165457/diff/2001/2002#newcode635
Line 635: // If the key event is not handled by |text_view_| and us, then we
need
s/and us/or us/

s/need/need to/

http://codereview.chromium.org/165457/diff/2001/2002#newcode637
Line 637: // In this case we need stop the signal emission explicitly to prevent
the
s/need stop/need to stop/

[James Su, 2009-08-13 23:41:19]

Sorry that I just submitted this CL. Will create a new CL to make these
changes.

Regards
James Su

2009/8/14 <derat@chromium.org>

> This is really tricky, but if it's the only way to get it working, I'm
> okay with it.  I just have a bunch of picky grammar stuff about the
> comments.
>
>
> http://codereview.chromium.org/165457/diff/2001/2002
> File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode518
> Line 518: // The omnibox supports several special behavior which may be
> triggered by
> s/behavior/behaviors/
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode530
> Line 530: // Then if the key event is one of Tab, Enter and Escape, we
> need trigger
> s/need trigger/need to trigger the/
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode542
> Line 542: // built-in operation if IME did not handle the key event.
> Because we don't
> change to "... and avoid the above built-in operation if IME did not
> handle the key event, because we don't want ..."
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode554
> Line 554: // So if char_inserted_ equals to '\t' after calling
> |text_view_|'s
> s/equals to '\t'/equals '\t'/
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode555
> Line 555: // default signal handler against a Tab key press event, then
> we can sure this
> switch to "then we know that the Tab key press event was handled by
> GtkTextView rather than IME, and can perform the special behavior for
> Tab safely."
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode563
> Line 563: // it, we use backup and restore trick: If Tab or Enter is
> pressed, backup the
> s/we use backup/we use a backup/
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode568
> Line 568: bool tab_pressed = ((event->keyval == GDK_Tab ||
> insert a blank line before this line?  it looks strange pressed up
> against the klass definition when there's a blank line between it and
> enter_pressed
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode587
> Line 587: // Reset these variable, which may be set in "insert-text"
> signal handler.
> change to "Reset |char_inserted_|, which may be set in the "insert-text"
> signal handler, so that we'll know if a Tab or Enter key event was
> handled by IME."
>
> (side note: just "IME" or "an IME" / "the IME"?)
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode629
> Line 629: // We can handle Escape key if |text_view_| did not handle it.
> s/can handle Escape/can handle the Escape/
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode630
> Line 630: // If it's not handled by us, then we need propagate it upto
> parent
> change to "... then we need to propagate it up to the parent ..."
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode635
> Line 635: // If the key event is not handled by |text_view_| and us,
> then we need
> s/and us/or us/
>
> s/need/need to/
>
> http://codereview.chromium.org/165457/diff/2001/2002#newcode637
> Line 637: // In this case we need stop the signal emission explicitly to
> prevent the
> s/need stop/need to stop/
>
>
> http://codereview.chromium.org/165457
>

[use derat at chromium.org, 2009-08-13 23:43:21]

No worries... If you just want to leave it as-is, I'm okay with making
them myself the next time I edit the file too, if that's easier. :-)

On Thu, Aug 13, 2009 at 4:40 PM, James Su<suzhe@chromium.org> wrote:
> Sorry that I just submitted this CL. Will create a new CL to make these
> changes.
>
> Regards
> James Su
>
> 2009/8/14 <derat@chromium.org>
>>
>> This is really tricky, but if it's the only way to get it working, I'm
>> okay with it. =A0I just have a bunch of picky grammar stuff about the
>> comments.
>>
>>
>> http://codereview.chromium.org/165457/diff/2001/2002
>> File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode518
>> Line 518: // The omnibox supports several special behavior which may be
>> triggered by
>> s/behavior/behaviors/
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode530
>> Line 530: // Then if the key event is one of Tab, Enter and Escape, we
>> need trigger
>> s/need trigger/need to trigger the/
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode542
>> Line 542: // built-in operation if IME did not handle the key event.
>> Because we don't
>> change to "... and avoid the above built-in operation if IME did not
>> handle the key event, because we don't want ..."
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode554
>> Line 554: // So if char_inserted_ equals to '\t' after calling
>> |text_view_|'s
>> s/equals to '\t'/equals '\t'/
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode555
>> Line 555: // default signal handler against a Tab key press event, then
>> we can sure this
>> switch to "then we know that the Tab key press event was handled by
>> GtkTextView rather than IME, and can perform the special behavior for
>> Tab safely."
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode563
>> Line 563: // it, we use backup and restore trick: If Tab or Enter is
>> pressed, backup the
>> s/we use backup/we use a backup/
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode568
>> Line 568: bool tab_pressed =3D ((event->keyval =3D=3D GDK_Tab ||
>> insert a blank line before this line? =A0it looks strange pressed up
>> against the klass definition when there's a blank line between it and
>> enter_pressed
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode587
>> Line 587: // Reset these variable, which may be set in "insert-text"
>> signal handler.
>> change to "Reset |char_inserted_|, which may be set in the "insert-text"
>> signal handler, so that we'll know if a Tab or Enter key event was
>> handled by IME."
>>
>> (side note: just "IME" or "an IME" / "the IME"?)
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode629
>> Line 629: // We can handle Escape key if |text_view_| did not handle it.
>> s/can handle Escape/can handle the Escape/
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode630
>> Line 630: // If it's not handled by us, then we need propagate it upto
>> parent
>> change to "... then we need to propagate it up to the parent ..."
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode635
>> Line 635: // If the key event is not handled by |text_view_| and us,
>> then we need
>> s/and us/or us/
>>
>> s/need/need to/
>>
>> http://codereview.chromium.org/165457/diff/2001/2002#newcode637
>> Line 637: // In this case we need stop the signal emission explicitly to
>> prevent the
>> s/need stop/need to stop/
>>
>> http://codereview.chromium.org/165457
>
>

[James Su, 2009-08-13 23:47:16]

Thanks a lot. It's ok to leave them to me, as I'm going to implement those
unimplemented methods in that file.

Regards
James Su

2009/8/14 Daniel Erat <derat@google.com>

> No worries... If you just want to leave it as-is, I'm okay with making
> them myself the next time I edit the file too, if that's easier. :-)
>
> On Thu, Aug 13, 2009 at 4:40 PM, James Su<suzhe@chromium.org> wrote:
> > Sorry that I just submitted this CL. Will create a new CL to make these
> > changes.
> >
> > Regards
> > James Su
> >
> > 2009/8/14 <derat@chromium.org>
> >>
> >> This is really tricky, but if it's the only way to get it working, I'm
> >> okay with it.  I just have a bunch of picky grammar stuff about the
> >> comments.
> >>
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002
> >> File chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc (right):
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode518
> >> Line 518: // The omnibox supports several special behavior which may be
> >> triggered by
> >> s/behavior/behaviors/
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode530
> >> Line 530: // Then if the key event is one of Tab, Enter and Escape, we
> >> need trigger
> >> s/need trigger/need to trigger the/
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode542
> >> Line 542: // built-in operation if IME did not handle the key event.
> >> Because we don't
> >> change to "... and avoid the above built-in operation if IME did not
> >> handle the key event, because we don't want ..."
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode554
> >> Line 554: // So if char_inserted_ equals to '\t' after calling
> >> |text_view_|'s
> >> s/equals to '\t'/equals '\t'/
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode555
> >> Line 555: // default signal handler against a Tab key press event, then
> >> we can sure this
> >> switch to "then we know that the Tab key press event was handled by
> >> GtkTextView rather than IME, and can perform the special behavior for
> >> Tab safely."
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode563
> >> Line 563: // it, we use backup and restore trick: If Tab or Enter is
> >> pressed, backup the
> >> s/we use backup/we use a backup/
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode568
> >> Line 568: bool tab_pressed = ((event->keyval == GDK_Tab ||
> >> insert a blank line before this line?  it looks strange pressed up
> >> against the klass definition when there's a blank line between it and
> >> enter_pressed
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode587
> >> Line 587: // Reset these variable, which may be set in "insert-text"
> >> signal handler.
> >> change to "Reset |char_inserted_|, which may be set in the "insert-text"
> >> signal handler, so that we'll know if a Tab or Enter key event was
> >> handled by IME."
> >>
> >> (side note: just "IME" or "an IME" / "the IME"?)
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode629
> >> Line 629: // We can handle Escape key if |text_view_| did not handle it.
> >> s/can handle Escape/can handle the Escape/
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode630
> >> Line 630: // If it's not handled by us, then we need propagate it upto
> >> parent
> >> change to "... then we need to propagate it up to the parent ..."
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode635
> >> Line 635: // If the key event is not handled by |text_view_| and us,
> >> then we need
> >> s/and us/or us/
> >>
> >> s/need/need to/
> >>
> >> http://codereview.chromium.org/165457/diff/2001/2002#newcode637
> >> Line 637: // In this case we need stop the signal emission explicitly to
> >> prevent the
> >> s/need stop/need to stop/
> >>
> >> http://codereview.chromium.org/165457
> >
> >
>

[Hironori Bono, 2009-08-14 01:21:36]

Personally, I'm very wondering why we need to make this file so complicated to
solve a regression. (The code of Firefox is far simpler than this.) To modify a
phrase of Darin, "it is far easier to keep simple code simple than to make
complicated code simple." (Sorry Darin for modifying your phrase without your
permissions.)
For what it's worth, I'm afraid we may be moving on a wrong way and it may be
better to roll back whole this code before Dean added a call of
gtk_im_context_filter_keypress(), which caused the original IME-compatibility
problem fixed by James and find another way.

Regards,

Hironori Bono

[Dean McNamee, 2009-08-14 01:25:56]

I completely agree this code is too complicated.  Any advice on how to
solve that would be fantastic.

Thanks

On Thu, Aug 13, 2009 at 6:21 PM, <hbono@chromium.org> wrote:
> Personally, I'm very wondering why we need to make this file so
> complicated to solve a regression. (The code of Firefox is far simpler
> than this.) To modify a phrase of Darin, "it is far easier to keep
> simple code simple than to make complicated code simple." (Sorry Darin
> for modifying your phrase without your permissions.)
> For what it's worth, I'm afraid we may be moving on a wrong way and it
> may be better to roll back whole this code before Dean added a call of
> gtk_im_context_filter_keypress(), which caused the original
> IME-compatibility problem fixed by James and find another way.
>
> Regards,
>
> Hironori Bono
>
> http://codereview.chromium.org/165457
>

[James Su, 2009-08-14 09:38:24]

How about we write our own edit view widget to get rid off GtkTextView
completely? Though the code would be longer, it won't be so tricky
like current code.

Regards
James SU

2009/8/14, Dean McNamee <deanm@chromium.org>:
> I completely agree this code is too complicated.  Any advice on how to
> solve that would be fantastic.
>
> Thanks
>
> On Thu, Aug 13, 2009 at 6:21 PM, <hbono@chromium.org> wrote:
>> Personally, I'm very wondering why we need to make this file so
>> complicated to solve a regression. (The code of Firefox is far simpler
>> than this.) To modify a phrase of Darin, "it is far easier to keep
>> simple code simple than to make complicated code simple." (Sorry Darin
>> for modifying your phrase without your permissions.)
>> For what it's worth, I'm afraid we may be moving on a wrong way and it
>> may be better to roll back whole this code before Dean added a call of
>> gtk_im_context_filter_keypress(), which caused the original
>> IME-compatibility problem fixed by James and find another way.
>>
>> Regards,
>>
>> Hironori Bono
>>
>> http://codereview.chromium.org/165457
>>
>