Refactor AddScrollListItem() in system menu detailed views

ash/system/tray/tray_details_view.h

Index: ash/system/tray/tray_details_view.h
diff --git a/ash/system/tray/tray_details_view.h b/ash/system/tray/tray_details_view.h
index f7f30f4976e6f26fc105b06850097a305aa572e1..d4ae963a6e1bab647d633a7445920c156560b665 100644
--- a/ash/system/tray/tray_details_view.h
+++ b/ash/system/tray/tray_details_view.h
@@ -15,6 +15,10 @@
 #include "ui/views/controls/button/button.h"
 #include "ui/views/view.h"
 
+namespace gfx {
+struct VectorIcon;
+}  // namespace gfx
+
 namespace views {
 class BoxLayout;
 class CustomButton;
@@ -27,6 +31,7 @@ namespace test {
 class TrayDetailsViewTest;
 }  // namespace test
 
+class HoverHighlightView;
 class ScrollBorder;
 class SystemTrayItem;
 class TriView;
@@ -47,8 +52,6 @@ class ASH_EXPORT TrayDetailsView : public views::View,
   void ButtonPressed(views::Button* sender, const ui::Event& event) final;
 
   SystemTrayItem* owner() { return owner_; }
-  views::ScrollView* scroller() { return scroller_; }
-  views::View* scroll_content() { return scroll_content_; }
 
  protected:
   // views::View:
@@ -67,6 +70,20 @@ class ASH_EXPORT TrayDetailsView : public views::View,
   // any other view between the list and the footer row at the bottom.
   void CreateScrollableList();
 
+  // Adds an item to the scroll list. |gfx::kNoneIcon| can be used if there is

[tdanderson, 2017/04/24 15:43:39]

nit: consider a rewording of first sentence to give a bit more information such
as "Creates a targetable row containing |icon| and |text| and adds it as a child
view of |scroll_content_|" or similar. Ditto for the other docs here.

[mohsen, 2017/04/25 05:10:01]

Done.

+  // no icon.
+  HoverHighlightView* AddScrollListItem(const gfx::VectorIcon& icon,
+                                        const base::string16& text);
+  // Adds an item to the scroll list with a checkbox. |gfx::kNoneIcon| can be
+  // used if there is no icon.
+  HoverHighlightView* AddScrollListCheckableItem(const gfx::VectorIcon& icon,
+                                                 const base::string16& text,
+                                                 bool checked);

[tdanderson, 2017/04/24 15:43:39]

nit: newline

[mohsen, 2017/04/25 05:10:01]

Done.

+  // Adds a sticky sub header to the scroll list. |gfx::kNoneIcon| can be used
+  // if there is no icon.
+  TriView* AddScrollListSubHeader(const gfx::VectorIcon& icon,

[tdanderson, 2017/04/24 15:43:39]

To simplify the call sites (not requiring kNoneIcon to be passed in for certain
cases), consider having both AddScrollListSubHeader(icon) and
AddScrollListSubHeader(icon,text) protected methods here. Ditto for
AddScrollListItem().

[mohsen, 2017/04/25 05:10:01]

Done.

+                                  const base::string16& text);
+
   // Removes (and destroys) all child views.
   void Reset();
 
@@ -82,6 +99,8 @@ class ASH_EXPORT TrayDetailsView : public views::View,
   views::CustomButton* CreateHelpButton();
 
   TriView* tri_view() { return tri_view_; }
+  views::ScrollView* scroller() const { return scroller_; }
+  views::View* scroll_content() const { return scroll_content_; }
 
  private:
   friend class test::TrayDetailsViewTest;