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

ui/views/controls/throbber.h

Index: ui/views/controls/throbber.h
diff --git a/ui/views/controls/throbber.h b/ui/views/controls/throbber.h
index 0ee428d0dfa4aad51debed2dc825d772aab06f80..071653b521674ff1aa57928c5a81f0d2108f19c2 100644
--- a/ui/views/controls/throbber.h
+++ b/ui/views/controls/throbber.h
@@ -41,6 +41,8 @@ class VIEWS_EXPORT Throbber : public View {
   void OnPaint(gfx::Canvas* canvas) override;
 
  protected:
+  const base::Time& start_time() { return start_time_; }

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

It's my understanding we return times by value and not reference. Also, how come
this is a base::Time and not a TimeTicks?

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

Done
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.

+
   // Specifies whether the throbber is currently animating or not
   bool running_;

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

Style guide says no protected members.

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

fixed

 
@@ -102,6 +104,7 @@ class VIEWS_EXPORT SmoothedThrobber : public Throbber {
 class VIEWS_EXPORT CheckmarkThrobber : public Throbber {
  public:
   CheckmarkThrobber();
+  ~CheckmarkThrobber() override;
 
   // If checked is true, the throbber stops spinning and displays a checkmark.
   // If checked is false, the throbber stops spinning and displays nothing.
@@ -122,6 +125,30 @@ class VIEWS_EXPORT CheckmarkThrobber : public Throbber {
   DISALLOW_COPY_AND_ASSIGN(CheckmarkThrobber);
 };
 
+// A throbber that follows material design guidelines. This is a specialization
+// of CheckmarkThrobber for now, but should probably replace Throbber
+// eventually.
+class VIEWS_EXPORT MaterialThrobber : public CheckmarkThrobber {

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

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.

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

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.

+ public:
+  MaterialThrobber();
+  ~MaterialThrobber() override;
+
+  void set_preferred_diameter(int diameter) { preferred_diameter_ = diameter; }

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

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]

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.

+
+  // View implementation.
+  gfx::Size GetPreferredSize() const override;
+  int GetHeightForWidth(int w) const override;
+  void OnPaint(gfx::Canvas* canvas) override;
+
+ private:
+  // The preferred width and height for this view. Zero indicates the size is
+  // set by the parent class (i.e. matches the size of the pre-material
+  // sprites).
+  int preferred_diameter_;
+
+  DISALLOW_COPY_AND_ASSIGN(MaterialThrobber);
+};
+
 }  // namespace views
 
 #endif  // UI_VIEWS_CONTROLS_THROBBER_H_