Refactor AddScrollListItem() in system menu detailed views

ash/system/tray/tray_popup_utils.h

Index: ash/system/tray/tray_popup_utils.h
diff --git a/ash/system/tray/tray_popup_utils.h b/ash/system/tray/tray_popup_utils.h
index d674034c8b8e9384d5e47dcfcc3440aa56f77191..a632044acb07f8ebb2fdb2978262e49512e18ec5 100644
--- a/ash/system/tray/tray_popup_utils.h
+++ b/ash/system/tray/tray_popup_utils.h
@@ -54,13 +54,15 @@ class TrayPopupUtils {
   // Creates a container view to be used by system menu sub-section header rows.
   // The caller takes over ownership of the created view.
   //
-  // The returned view consists of 2 regions: CENTER, and END having the same
-  // properties as when using |CreateMultiTargetRowView|. The START container is
-  // hidden.
-  // The END container has a fixed minimum width but can grow into the CENTER
-  // container if space is required and available. The CENTER container has a
-  // flexible width.
-  static TriView* CreateSubHeaderRowView();
+  // The returned view contains at least CENTER and END regions having the same
+  // properties as when using |CreateMultiTargetRowView|. |hide_start|
+  // determines whether the START region should be visible or not. If START is
+  // not visible, padding would be increased on that side.

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

I'm not sure about the last sentence here. If START is not visible, don't you
have CENTER beginning left-aligned in the container?

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

What I meant is that when START is not visible, we add extra padding to the left
of the container. Modified the comment to hopefully make it clearer.

+  //
+  // The START (if visible) and END containers have a fixed minimum width but
+  // can grow into the CENTER container if space is required and available. The
+  // CENTER container has a flexible width.
+  static TriView* CreateSubHeaderRowView(bool hide_start);

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

optional: I don't think this is called out explicitly in the style guide, and
maybe this is purely personal preference, but I feel that naming variables in a
'positive' rather than a 'negative' way usually makes things more readable. So
here, consider calling the parameter something like |start_visible|, and
swapping all call sites from true to false and vice-versa.

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

Makes sense. Done.

 
   // Creates a view containing only a label (corresponding to |message_id|),
   // which is to be inserted as a non-targetable row within a system menu