Refactor ViewsDelegate and MD-ify the icon-to-text spacing for checkbox and radiobutton

chrome/browser/ui/views/toolbar/reload_button_unittest.cc

Index: chrome/browser/ui/views/toolbar/reload_button_unittest.cc
diff --git a/chrome/browser/ui/views/toolbar/reload_button_unittest.cc b/chrome/browser/ui/views/toolbar/reload_button_unittest.cc
index fe705117d469f5e57753b45a2677338f924e621c..d76dbc639be97e7c7c232af3812d2b6f4d59baa6 100644
--- a/chrome/browser/ui/views/toolbar/reload_button_unittest.cc
+++ b/chrome/browser/ui/views/toolbar/reload_button_unittest.cc
@@ -2,12 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/run_loop.h"
 #include "chrome/browser/ui/views/toolbar/reload_button.h"

[sky, 2017/02/28 20:11:52]

newline between 5/6 (see style guide for details).

[kylix_rd, 2017/03/01 17:33:27]

Interesting... this was changed by git cl format, which I would have assumed
would do the right thing.

[kylix_rd, 2017/03/01 17:33:27]

Done.

+#include "base/run_loop.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/event_utils.h"
+#include "ui/views/test/test_views_delegate.h"
 
 class ReloadButtonTest : public ChromeRenderViewHostTestHarness {
  public:
@@ -25,6 +26,9 @@ class ReloadButtonTest : public ChromeRenderViewHostTestHarness {
   }
   int reload_count() { return reload_.testing_reload_count_; }
 
+ private:

[sky, 2017/02/28 20:11:52]

Style guide says private section should be last (after protected, which is after
public).

[kylix_rd, 2017/03/01 17:33:27]

It seems that there is a chicken-and-egg problem then. Placing the
TestViewsDelegate field lexically after the ReloadButton field causes the
ReloadButton to be constructed prior to the TestViewsDelegate. This will lead to
a crash in the construction of the ReloadButton.

The solution would be to move the ReloadButton field to the private section and
then add a getter function returning a ReloadButton& to the protected section
and refactor the tests which reference the reload_ field.

[Peter Kasting, 2017/03/01 23:32:23]

I was gonna leave a comment about how this test really shouldn't have a
protected data member, but I didn't want to burden you with unnecessary code
cleanup in the midst of trying to land this.

But... yeah, that seems like the correct way to solve :)

+  views::TestViewsDelegate views_delegate_;
+
  protected:
   ReloadButton reload_;
 };

[Peter Kasting, 2017/03/01 06:32:36]

Nit: DISALLOW_COPY_AND_ASSIGN

[kylix_rd, 2017/03/01 17:33:27]

Done.