Implement progress bar spec (determinate and indeterminate).

Implement progress bar spec (determinate and indeterminate).

This takes the implementation from the notification center
(used for downloads) and moves it into views::. The color
is updated as per sgabriel's orders to Blue 500, a default
height of 5 is applied (shared in app list and notification
center) but lesser heights are also permitted. If the height
is small enough, we don't round the corners, which is exactly
what we'll want for the cros detail views.

The base views::ProgressBar class was not used on its own anywhere,
and it had a lot of code that wasn't used either. All the progress
bar classes are consolidated into a single class and unused code
is culled.

BUG=626846
Committed: https://crrev.com/a62b6b27f42e7259f7d3884184dce068de18b756
Cr-Commit-Position: refs/heads/master@{#418676}

[Evan Stade, 2016-09-10 02:09:40]

estade@chromium.org changed reviewers:
+ tdanderson@chromium.org

[Evan Stade, 2016-09-10 02:09:41]

This code is pretty much ready to go but I think we should wait for Sebastien's
design feedback regarding other places where it's used.

[Evan Stade, 2016-09-10 02:09:57]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-10 02:10:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-10 02:26:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-10 02:26:44]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tdanderson, 2016-09-12 17:11:14]

lg with a couple of comments and a question below:

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/gfx/animation...
File ui/gfx/animation/linear_animation.h (right):

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/gfx/animation...
ui/gfx/animation/linear_animation.h:28: explicit
LinearAnimation(AnimationDelegate* delegate);
Nice. Maybe also call this ctor from AutoclickRingHandler and kill off
kAutoclickRingFrameRateHz.

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/views/control...
File ui/views/controls/progress_bar.h (right):

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/views/control...
ui/views/controls/progress_bar.h:77: class VIEWS_EXPORT DeterminateProgressBar :
public ProgressBar {
Class-level descriptions would be nice to include here and above line 91.

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/views/control...
ui/views/controls/progress_bar.h:91: class VIEWS_EXPORT IndeterminateProgressBar
: public DeterminateProgressBar,
I don't understand why you have chosen to have IPB extend DPB rather than have
both extend ProgressBar.

[Evan Stade, 2016-09-13 02:04:42]

Description was changed from

==========
Implement progress bar spec (determinate and indeterminate).

This is mostly just a move/rename of the notification center's progress
bars. The appearance is also updated to match sgabriel's specs (no
rounded corners, different colors) but the motion and the rest of the
logic is left pretty much intact.

The base views::ProgressBar class was not used on its own anywhere, so
its OnPaint can just be removed. There's also a progress bar in the app
list, and it also gets the new visuals.

BUG=626846
==========

to

==========
Implement progress bar spec (determinate and indeterminate).

This takes the implementation from the notification center
(used for downloads) and moves it into views::. The color
is updated as per sgabriel's orders to Blue 500, a default
height of 5 is applied (shared in app list and notification
center) but lesser heights are also permitted. If the height
is small enough, we don't round the corners, which is exactly
what we'll want for the cros detail views.

The base views::ProgressBar class was not used on its own anywhere,
and it had a lot of code that wasn't used either. All the progress
bar classes are consolidated into a single class and unused code
is culled.

BUG=626846
==========

[Evan Stade, 2016-09-13 02:04:42]

estade@chromium.org changed reviewers:
+ sky@chromium.org

[Evan Stade, 2016-09-13 02:05:14]

+sky

lots of changes in this patch, both to streamline and to update appearance as
per sgabriel's feedback.

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/gfx/animation...
File ui/gfx/animation/linear_animation.h (right):

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/gfx/animation...
ui/gfx/animation/linear_animation.h:28: explicit
LinearAnimation(AnimationDelegate* delegate);
On 2016/09/12 17:11:13, tdanderson wrote:
> Nice. Maybe also call this ctor from AutoclickRingHandler and kill off
> kAutoclickRingFrameRateHz.

Done.

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/views/control...
File ui/views/controls/progress_bar.h (right):

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/views/control...
ui/views/controls/progress_bar.h:77: class VIEWS_EXPORT DeterminateProgressBar :
public ProgressBar {
On 2016/09/12 17:11:13, tdanderson wrote:
> Class-level descriptions would be nice to include here and above line 91.

Done.

https://chromiumcodereview.appspot.com/2329633003/diff/20001/ui/views/control...
ui/views/controls/progress_bar.h:91: class VIEWS_EXPORT IndeterminateProgressBar
: public DeterminateProgressBar,
On 2016/09/12 17:11:13, tdanderson wrote:
> I don't understand why you have chosen to have IPB extend DPB rather than have
> both extend ProgressBar.

I don't understand either. Just a brain fart.

I started deleting unused code and at some point I realized there's no need for
multiple classes (much less three of them), so I've consolidated it all down to
a single class. You get indeterminate behavior if you SetValue(-1).

[sky, 2016-09-13 02:30:08]

https://codereview.chromium.org/2329633003/diff/80001/ui/gfx/animation/linear...
File ui/gfx/animation/linear_animation.h (right):

https://codereview.chromium.org/2329633003/diff/80001/ui/gfx/animation/linear...
ui/gfx/animation/linear_animation.h:28: explicit
LinearAnimation(AnimationDelegate* delegate);
How about the following as the only constructor:

explicit LinearAnimation(AnimationDelegate* delegate,
frame_default = kDefaultFrameFrame);
?

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
File ui/views/controls/progress_bar.cc (right):

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.cc:67: SkColor ProgressBar::GetForegroundColor()
const {
definition/declaration order is off.

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.cc:114: if (adjusted_value != current_value_) {
optional: early return.

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.cc:121: SchedulePaint();
Does this need to reset indeterminate_bar_animation_?

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
File ui/views/controls/progress_bar.h (right):

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.h:28: void set_preferred_height(int
preferred_height) {
If the value changes won't this need to trigger a layout? Or at least
PreferredSizeChanged?

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.h:66: int preferred_height_;
optional: = 5 and remove comment about default.

[Evan Stade, 2016-09-13 15:45:40]

https://codereview.chromium.org/2329633003/diff/80001/ui/gfx/animation/linear...
File ui/gfx/animation/linear_animation.h (right):

https://codereview.chromium.org/2329633003/diff/80001/ui/gfx/animation/linear...
ui/gfx/animation/linear_animation.h:28: explicit
LinearAnimation(AnimationDelegate* delegate);
On 2016/09/13 02:30:08, sky wrote:
> How about the following as the only constructor:
> 
> explicit LinearAnimation(AnimationDelegate* delegate,
> frame_default = kDefaultFrameFrame);
> ?

Done.

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
File ui/views/controls/progress_bar.cc (right):

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.cc:67: SkColor ProgressBar::GetForegroundColor()
const {
On 2016/09/13 02:30:08, sky wrote:
> definition/declaration order is off.

Done.

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.cc:114: if (adjusted_value != current_value_) {
On 2016/09/13 02:30:08, sky wrote:
> optional: early return.

Done.

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.cc:121: SchedulePaint();
On 2016/09/13 02:30:08, sky wrote:
> Does this need to reset indeterminate_bar_animation_?

Done.

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
File ui/views/controls/progress_bar.h (right):

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.h:28: void set_preferred_height(int
preferred_height) {
On 2016/09/13 02:30:08, sky wrote:
> If the value changes won't this need to trigger a layout? Or at least
> PreferredSizeChanged?

Yea but I would expect that this is called by the owner, which should be aware
of the need to re-layout. It's really meant to be called before the first
layout, so I guess I can just make it an optional ctor parameter.

https://codereview.chromium.org/2329633003/diff/80001/ui/views/controls/progr...
ui/views/controls/progress_bar.h:66: int preferred_height_;
On 2016/09/13 02:30:08, sky wrote:
> optional: = 5 and remove comment about default.

default moved to ctor

[Evan Stade, 2016-09-13 15:55:48]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-13 15:56:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 16:13:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 16:13:43]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[sky, 2016-09-13 19:22:15]

LGTM

https://codereview.chromium.org/2329633003/diff/100001/ui/gfx/animation/linea...
File ui/gfx/animation/linear_animation.h (right):

https://codereview.chromium.org/2329633003/diff/100001/ui/gfx/animation/linea...
ui/gfx/animation/linear_animation.h:29: LinearAnimation(AnimationDelegate*
delegate,
As the second parameter is optional I believe you want explicit here.

https://codereview.chromium.org/2329633003/diff/100001/ui/views/controls/prog...
File ui/views/controls/progress_bar.h (right):

https://codereview.chromium.org/2329633003/diff/100001/ui/views/controls/prog...
ui/views/controls/progress_bar.h:61: int preferred_height_;
const

[tdanderson, 2016-09-13 19:28:09]

lgtm also

[Evan Stade, 2016-09-13 22:01:05]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-09-13 22:01:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdanderson@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2329633003/#ps120001
(title: "2 more minor changes")

[Evan Stade, 2016-09-13 22:01:24]

https://codereview.chromium.org/2329633003/diff/100001/ui/gfx/animation/linea...
File ui/gfx/animation/linear_animation.h (right):

https://codereview.chromium.org/2329633003/diff/100001/ui/gfx/animation/linea...
ui/gfx/animation/linear_animation.h:29: LinearAnimation(AnimationDelegate*
delegate,
On 2016/09/13 19:22:15, sky wrote:
> As the second parameter is optional I believe you want explicit here.

Done.

https://codereview.chromium.org/2329633003/diff/100001/ui/views/controls/prog...
File ui/views/controls/progress_bar.h (right):

https://codereview.chromium.org/2329633003/diff/100001/ui/views/controls/prog...
ui/views/controls/progress_bar.h:61: int preferred_height_;
On 2016/09/13 19:22:15, sky wrote:
> const

Done.

[commit-bot: I haz the power, 2016-09-13 22:02:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 22:18:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 22:18:52]

Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2016-09-13 23:59:26]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-09-13 23:59:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdanderson@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2329633003/#ps140001
(title: "fix app list test")

[commit-bot: I haz the power, 2016-09-14 00:00:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 00:22:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-14 00:22:21]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Evan Stade, 2016-09-14 19:33:41]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 19:34:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 20:48:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-14 20:48:46]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-09-14 21:09:47]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-09-14 21:09:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdanderson@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2329633003/#ps160001
(title: "attempt to unbreak mac and unit test")

[commit-bot: I haz the power, 2016-09-14 21:10:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 21:16:39]

Description was changed from

==========
Implement progress bar spec (determinate and indeterminate).

This takes the implementation from the notification center
(used for downloads) and moves it into views::. The color
is updated as per sgabriel's orders to Blue 500, a default
height of 5 is applied (shared in app list and notification
center) but lesser heights are also permitted. If the height
is small enough, we don't round the corners, which is exactly
what we'll want for the cros detail views.

The base views::ProgressBar class was not used on its own anywhere,
and it had a lot of code that wasn't used either. All the progress
bar classes are consolidated into a single class and unused code
is culled.

BUG=626846
==========

to

==========
Implement progress bar spec (determinate and indeterminate).

This takes the implementation from the notification center
(used for downloads) and moves it into views::. The color
is updated as per sgabriel's orders to Blue 500, a default
height of 5 is applied (shared in app list and notification
center) but lesser heights are also permitted. If the height
is small enough, we don't round the corners, which is exactly
what we'll want for the cros detail views.

The base views::ProgressBar class was not used on its own anywhere,
and it had a lot of code that wasn't used either. All the progress
bar classes are consolidated into a single class and unused code
is culled.

BUG=626846
==========

[commit-bot: I haz the power, 2016-09-14 21:16:41]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-09-14 21:18:33]

Description was changed from

==========
Implement progress bar spec (determinate and indeterminate).

This takes the implementation from the notification center
(used for downloads) and moves it into views::. The color
is updated as per sgabriel's orders to Blue 500, a default
height of 5 is applied (shared in app list and notification
center) but lesser heights are also permitted. If the height
is small enough, we don't round the corners, which is exactly
what we'll want for the cros detail views.

The base views::ProgressBar class was not used on its own anywhere,
and it had a lot of code that wasn't used either. All the progress
bar classes are consolidated into a single class and unused code
is culled.

BUG=626846
==========

to

==========
Implement progress bar spec (determinate and indeterminate).

This takes the implementation from the notification center
(used for downloads) and moves it into views::. The color
is updated as per sgabriel's orders to Blue 500, a default
height of 5 is applied (shared in app list and notification
center) but lesser heights are also permitted. If the height
is small enough, we don't round the corners, which is exactly
what we'll want for the cros detail views.

The base views::ProgressBar class was not used on its own anywhere,
and it had a lot of code that wasn't used either. All the progress
bar classes are consolidated into a single class and unused code
is culled.

BUG=626846

Committed: https://crrev.com/a62b6b27f42e7259f7d3884184dce068de18b756
Cr-Commit-Position: refs/heads/master@{#418676}
==========

[commit-bot: I haz the power, 2016-09-14 21:18:34]

Patchset 9 (id:??) landed as
https://crrev.com/a62b6b27f42e7259f7d3884184dce068de18b756
Cr-Commit-Position: refs/heads/master@{#418676}