Reduce candidate window flickering.

Reduce candidate window flickering.

Candidate window is shown upper side of composition string if there is no space to be shown.
However it become too often.
According to CL, candidate window can be switched the side to be shown only one time.


BUG=129119
TEST=manually done on Alex

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

[Hiro Komatsu, 2012-06-08 05:22:42]

http://codereview.chromium.org/10542051/diff/1/chrome/browser/chromeos/input_...
File chrome/browser/chromeos/input_method/candidate_window.cc (right):

http://codereview.chromium.org/10542051/diff/1/chrome/browser/chromeos/input_...
chrome/browser/chromeos/input_method/candidate_window.cc:779:
did_window_avoid_display_edge_(false),
How about keep_position_ or should_keep_position_?

[Seigo Nonaka, 2012-06-08 05:42:14]

http://codereview.chromium.org/10542051/diff/1/chrome/browser/chromeos/input_...
File chrome/browser/chromeos/input_method/candidate_window.cc (right):

http://codereview.chromium.org/10542051/diff/1/chrome/browser/chromeos/input_...
chrome/browser/chromeos/input_method/candidate_window.cc:779:
did_window_avoid_display_edge_(false),
position doesn't mean bit clear.
changed to |should_show_upper_side_|
On 2012/06/08 05:22:42, komatsu wrote:
> How about keep_position_ or should_keep_position_?

[Hiro Komatsu, 2012-06-08 05:52:32]

lgtm

[Seigo Nonaka, 2012-06-08 05:55:07]

Thank you for your quick review.

+yusukes
Could you take a look this CL?

Thanks.

[Yusuke Sato, 2012-06-08 07:42:26]

lgtm with nits:

I think the first line of the CL description should only explain what problem is
fixed with the change or what the change does, and the line should be short
enough (~100 characters). Please do not write miscellaneous background of the
problem, performance of the patch, safety of the patch, ... there (please put
them at the bottom of the description if needed). I guess it would make it
harder for e.g. release managers to quickly understand what this patch is for.

In this case, "Candidate window is shown upper side of composition string if
there is no space to be shown. " is the current behavior of Chrome and does not
explain what your patch does, I think.

http://codereview.chromium.org/10542051/diff/10001/chrome/browser/chromeos/in...
File chrome/browser/chromeos/input_method/candidate_window.cc (right):

http://codereview.chromium.org/10542051/diff/10001/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/candidate_window.cc:1256: 
I think you should do 

if (should_show_upper_side && top_overflow) {
  frame_bounds.set_y(...);
  should_show_upper_side_ = false;
}

here since the user might scroll the text area during composition. For now,
please add 'TODO(nona): check top_overflow here' or something similar.

[Seigo Nonaka, 2012-06-08 07:51:00]

Okay, revised description.

http://codereview.chromium.org/10542051/diff/10001/chrome/browser/chromeos/in...
File chrome/browser/chromeos/input_method/candidate_window.cc (right):

http://codereview.chromium.org/10542051/diff/10001/chrome/browser/chromeos/in...
chrome/browser/chromeos/input_method/candidate_window.cc:1256: 
On 2012/06/08 07:42:26, Yusuke Sato wrote:
> I think you should do 
> 
> if (should_show_upper_side && top_overflow) {
>   frame_bounds.set_y(...);
>   should_show_upper_side_ = false;
> }
> 
> here since the user might scroll the text area during composition. For now,
> please add 'TODO(nona): check top_overflow here' or something similar.

Done.

[commit-bot: I haz the power, 2012-06-08 09:33:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nona@chromium.org/10542051/11003

[commit-bot: I haz the power, 2012-06-08 11:28:48]

Change committed as 141196