Implement material throbber and use it in one place (Autofill card

Implement material throbber and use it in one place (Autofill card
unmasking prompt).

BUG=473342
Committed: https://crrev.com/89cf95f6e0432b0bf17e5b67f2b395ae69f2ae3e
Cr-Commit-Position: refs/heads/master@{#326275}

[Evan Stade, 2015-04-21 20:06:12]

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

[Evan Stade, 2015-04-21 20:07:39]

re: performance - My unscientific testing on my local machine shows that the
normal throbber idles at about 12% CPU in the browser process and 6% in GPU. The
new Skia throbber uses about 6% in both processes.

[sky, 2015-04-21 20:17:38]

Make sure you also try this at non-1x scales to make sure it looks good.

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
File ui/views/controls/throbber.h (right):

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
ui/views/controls/throbber.h:44: const base::Time& start_time() { return
start_time_; }
It's my understanding we return times by value and not reference. Also, how come
this is a base::Time and not a TimeTicks?

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
ui/views/controls/throbber.h:47: bool running_;
Style guide says no protected members.

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
ui/views/controls/throbber.h:131: class VIEWS_EXPORT MaterialThrobber : public
CheckmarkThrobber {
Do we need both CheckmarkThrobber and MaterialThrobber? I believe autofill is
the only place using CheckmarkThrobber, so that if you convert autofill to
MaterialThrobber no one will be using CheckmarkThrobber.

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
ui/views/controls/throbber.h:136: void set_preferred_diameter(int diameter) {
preferred_diameter_ = diameter; }
It's unclear exactly what diameter in this context means, and I suspect it's
overly specific to current rendering. I think preferred_size would be better
(still an int, as you have here).

[Evan Stade, 2015-04-21 22:04:02]

looks good with --force-device-scale-factor=2

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
File ui/views/controls/throbber.h (right):

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
ui/views/controls/throbber.h:44: const base::Time& start_time() { return
start_time_; }
On 2015/04/21 20:17:38, sky wrote:
> It's my understanding we return times by value and not reference. 

Done

> Also, how come
> this is a base::Time and not a TimeTicks?

I don't know, perhaps it predates TimeTicks? I don't think it matters much which
one we use in this case. From reading the docs, it seems like the worst thing
that could happen is the user changes the system clock and active spinners jump
to a random frame.

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
ui/views/controls/throbber.h:47: bool running_;
On 2015/04/21 20:17:38, sky wrote:
> Style guide says no protected members.

fixed

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
ui/views/controls/throbber.h:131: class VIEWS_EXPORT MaterialThrobber : public
CheckmarkThrobber {
On 2015/04/21 20:17:38, sky wrote:
> Do we need both CheckmarkThrobber and MaterialThrobber? I believe autofill is
> the only place using CheckmarkThrobber, so that if you convert autofill to
> MaterialThrobber no one will be using CheckmarkThrobber.

Good point. I was going to roll all three into Throbber eventually (I can't see
why CheckmarkThrobber should exist separately), but for now we can at least
combine Checkmark and Material.

https://codereview.chromium.org/1067593006/diff/20001/ui/views/controls/throb...
ui/views/controls/throbber.h:136: void set_preferred_diameter(int diameter) {
preferred_diameter_ = diameter; }
On 2015/04/21 20:17:38, sky wrote:
> It's unclear exactly what diameter in this context means, and I suspect it's
> overly specific to current rendering. I think preferred_size would be better
> (still an int, as you have here).

But size sounds like a gfx::Size. I chose diameter as a word that indicates both
height and width.

set_preferred_diameter is specific to the current rendering but if there were
any drastic changes to the rendering we'd probably want to remove it anyway.

[sky, 2015-04-21 23:41:58]

I still like set_preferred_size(int), but ok, LGTM

[Evan Stade, 2015-04-21 23:49:32]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2015-04-21 23:50:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1067593006/40001

[commit-bot: I haz the power, 2015-04-22 00:26:26]

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

[commit-bot: I haz the power, 2015-04-22 00:26:27]

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

[Evan Stade, 2015-04-22 12:34:12]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2015-04-22 12:34:12]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1067593006/#ps60001
(title: "fix win compile?")

[commit-bot: I haz the power, 2015-04-22 12:34:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1067593006/60001

[commit-bot: I haz the power, 2015-04-22 12:38:10]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-22 12:39:01]

Patchset 4 (id:??) landed as
https://crrev.com/89cf95f6e0432b0bf17e5b67f2b395ae69f2ae3e
Cr-Commit-Position: refs/heads/master@{#326275}