Reuse the code within UpdateCheckMark()

ash/common/system/tray/tray_popup_utils.cc

 // Copyright 2016 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.
 
 #include "ash/common/system/tray/tray_popup_utils.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "ash/common/ash_constants.h"
 #include "ash/common/ash_view_ids.h"
 #include "ash/common/session/session_controller.h"
 #include "ash/common/system/tray/fixed_sized_image_view.h"
+#include "ash/common/system/tray/hover_highlight_view.h"
 #include "ash/common/system/tray/size_range_layout.h"
 #include "ash/common/system/tray/tray_constants.h"
 #include "ash/common/system/tray/tray_popup_item_style.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/shell.h"
 #include "base/memory/ptr_util.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
 #include "ui/views/animation/ink_drop_highlight.h"
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/animation/ink_drop_mask.h"
 #include "ui/views/animation/square_ink_drop_ripple.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/label_button.h"
@@ skipping 396 matching lines @@
 
 bool TrayPopupUtils::CanOpenWebUISettings(LoginStatus status) {
   // TODO(tdanderson): Consider moving this into WmShell, or introduce a
   // CanShowSettings() method in each delegate type that has a
   // ShowSettings() method.
   return status != LoginStatus::NOT_LOGGED_IN &&
          status != LoginStatus::LOCKED &&
          !Shell::Get()->session_controller()->IsInSecondaryLoginScreen();
 }
 
+void TrayPopupUtils::UpdateCheckMarkVisibility(HoverHighlightView* container,

[tdanderson, 2017/04/07 17:54:31]

What do you think about splitting this into two helpers like follows:

void TrayPopupUtils::InitializeAsCheckableRow(HoverHighlightView* container,
bool checked) {
  gfxImageSkia check_mark = CreateVectorIcon(...);
  container->AddRightIcon(...);
  UpdateCheckMarkVisibility(container, checked);
}

void TrayPopupUtils::UpdateCheckMarkVisibility(HoverHighlightView* container,
bool visible) {
  container->SetRightViewVisible(checked);
  container->SetAccessibilityState(checked ? CHECKED_CHECKBOX :
UNCHECKED_CHECKBOX);
}

Then everywhere you currently call UpdateCheckMarkVisibility(), you would call
InitializeAsCheckableRow() instead. The advantage of having
UpdateCheckMarkVisibility() as a separate function is that now you can call it
from common_palette_tool (lines 55 and 66) to eliminate the duplicated code
there.

+                                               bool checked) {
+  if (checked) {
+    gfx::ImageSkia check_mark =
+        CreateVectorIcon(kCheckCircleIcon, gfx::kGoogleGreen700);
+    container->AddRightIcon(check_mark, check_mark.width());
+    container->SetRightViewVisible(true);
+    container->SetAccessiblityState(
+        HoverHighlightView::AccessibilityState::CHECKED_CHECKBOX);
+  } else {
+    container->SetAccessiblityState(
+        HoverHighlightView::AccessibilityState::UNCHECKED_CHECKBOX);
+  }
+}
+
 }  // namespace ash