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

ui/views/controls/throbber.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef UI_VIEWS_CONTROLS_THROBBER_H_
 #define UI_VIEWS_CONTROLS_THROBBER_H_
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/time/time.h"
@@ skipping 23 matching lines @@
   virtual void Stop();
 
   // Set custom throbber frames. Otherwise IDR_THROBBER is loaded.
   void SetFrames(const gfx::ImageSkia* frames);
 
   // Overridden from View:
   gfx::Size GetPreferredSize() const override;
   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

 
  private:
   void Run();
 
   bool paint_while_stopped_;
   int frame_count_;  // How many frames we have.
   base::Time start_time_;  // Time when Start was called.
   const gfx::ImageSkia* frames_;  // Frames images.
   base::TimeDelta frame_time_;  // How long one frame is displayed.
   base::RepeatingTimer<Throbber> timer_;  // Used to schedule Run calls.
@@ skipping 39 matching lines @@
 };
 
 // A CheckmarkThrobber is a special variant of throbber that has three states:
 //   1. not yet completed (which paints nothing)
 //   2. working (which paints the throbber animation)
 //   3. completed (which paints a checkmark)
 //
 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.
   void SetChecked(bool checked);
 
   // Overridden from Throbber:
   void OnPaint(gfx::Canvas* canvas) override;
 
  private:
   static const int kFrameTimeMs = 30;
 
   // Whether or not we should display a checkmark.
   bool checked_;
 
   // The checkmark image.
   const gfx::ImageSkia* checkmark_;
 
   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_