[gtk] Add fade-commit to instant suggestion for predictive instant.

[gtk] Add fade-commit to instant suggestion for predictive instant.

BUG=65399
TEST=manual

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

[James Su, 2010-12-04 03:43:37]

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

http://codereview.chromium.org/5588004/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc:64: const int
kFadeInTimeMS = 300;
Is it possible or necessary to share these constants with other ports?

http://codereview.chromium.org/5588004/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc:814: GdkColor c;
It's not used in views port.

http://codereview.chromium.org/5588004/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc:854: void
AutocompleteEditViewGtk::UpdateInstantViewColors() {
Please fix compilation failure for views port.

[sky, 2010-12-06 17:38:12]

http://codereview.chromium.org/5588004/diff/2001/app/multi_animation.cc
File app/multi_animation.cc (right):

http://codereview.chromium.org/5588004/diff/2001/app/multi_animation.cc#newco...
app/multi_animation.cc:36: current_value_ = 0;
current_value and current_parent_index should be set in SetStartTime.
SetStartTime should probably be renamed to indicate its the place for this sort
of resetting. Don't feel you need to rename SetStartTime as part of this patch.

http://codereview.chromium.org/5588004/diff/2001/app/multi_animation.h
File app/multi_animation.h (right):

http://codereview.chromium.org/5588004/diff/2001/app/multi_animation.h#newcode51
app/multi_animation.h:51: void Reset();
If we want this, it should be in Animation.

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

http://codereview.chromium.org/5588004/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc:64: const int
kFadeInTimeMS = 300;
On 2010/12/04 03:43:38, James Su wrote:
> Is it possible or necessary to share these constants with other ports?

Maybe move them to browser/defaults?

[Evan Stade, 2010-12-06 22:15:47]

http://codereview.chromium.org/5588004/diff/2001/app/multi_animation.cc
File app/multi_animation.cc (right):

http://codereview.chromium.org/5588004/diff/2001/app/multi_animation.cc#newco...
app/multi_animation.cc:36: current_value_ = 0;
On 2010/12/06 17:38:12, sky wrote:
> current_value and current_parent_index should be set in SetStartTime.
> SetStartTime should probably be renamed to indicate its the place for this
sort
> of resetting. Don't feel you need to rename SetStartTime as part of this
patch.

I think I've done what you intended

http://codereview.chromium.org/5588004/diff/2001/app/multi_animation.h
File app/multi_animation.h (right):

http://codereview.chromium.org/5588004/diff/2001/app/multi_animation.h#newcode51
app/multi_animation.h:51: void Reset();
On 2010/12/06 17:38:12, sky wrote:
> If we want this, it should be in Animation.

removed

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

http://codereview.chromium.org/5588004/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc:64: const int
kFadeInTimeMS = 300;
On 2010/12/06 17:38:12, sky wrote:
> On 2010/12/04 03:43:38, James Su wrote:
> > Is it possible or necessary to share these constants with other ports?
> 
> Maybe move them to browser/defaults?

moved to instant_controller. defaults.h doesn't seem to be a place for stuffing
generic constants.

http://codereview.chromium.org/5588004/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc:814: GdkColor c;
On 2010/12/04 03:43:38, James Su wrote:
> It's not used in views port.

Done.

http://codereview.chromium.org/5588004/diff/2001/chrome/browser/autocomplete/...
chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc:854: void
AutocompleteEditViewGtk::UpdateInstantViewColors() {
On 2010/12/04 03:43:38, James Su wrote:
> Please fix compilation failure for views port.

Done.

[sky, 2010-12-06 22:23:15]

LGTM

[James Su, 2010-12-06 22:42:06]

LGTM.