Overview Tray Button should not be visible on login screen

ash/system/overview/overview_button_tray.cc

 // Copyright 2014 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/system/overview/overview_button_tray.h"
 
 #include "ash/shelf/shelf_types.h"
 #include "ash/shell.h"
+#include "ash/system/tray/system_tray_delegate.h"
 #include "ash/system/tray/tray_utils.h"
 #include "ash/wm/overview/window_selector_controller.h"
 #include "grit/ash_resources.h"
 #include "grit/ash_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"
 
 namespace {
@@ skipping 15 matching lines @@
       icon_(NULL) {
   SetContentsBackground();
 
   ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
   icon_ = new views::ImageView();
   icon_->SetImage(
       bundle.GetImageNamed(IDR_AURA_UBER_TRAY_OVERVIEW_MODE).ToImageSkia());
   SetIconBorderForShelfAlignment();
   tray_container()->AddChildView(icon_);
 
-  SetVisible(Shell::GetInstance()->IsMaximizeModeWindowManagerEnabled());
+  SetVisible(false);

[flackr, 2014/03/26 21:00:12]

Won't this fail when a display is added while in maximize mode?

 
   Shell::GetInstance()->AddShellObserver(this);
 }
 
 OverviewButtonTray::~OverviewButtonTray() {
   Shell::GetInstance()->RemoveShellObserver(this);
 }
 
 bool OverviewButtonTray::PerformAction(const ui::Event& event) {
   Shell::GetInstance()->window_selector_controller()->ToggleOverview();
   return true;
 }
 
 void OverviewButtonTray::OnMaximizeModeStarted() {
-  SetVisible(true);
+  SetVisible(ShouldBeVisibleForLoginStatus(Shell::GetInstance()->
+      system_tray_delegate()->GetUserLoginStatus()));
 }
 
 void OverviewButtonTray::OnMaximizeModeEnded() {
   SetVisible(false);
 }
 
 bool OverviewButtonTray::ClickedOutsideBubble() {
   // This class has no bubbles dismiss, but acknowledge that the message was
   // handled.
   return true;
@@ skipping 26 matching lines @@
         kHorizontalShelfHorizontalPadding));
   } else {
     icon_->SetBorder(views::Border::CreateEmptyBorder(
         kVerticalShelfVerticalPadding,
         kVerticalShelfHorizontalPadding,
         kVerticalShelfVerticalPadding,
         kVerticalShelfHorizontalPadding));
   }
 }
 
+void OverviewButtonTray::UpdateAfterLoginStatusChange(
+    user::LoginStatus status) {
+  SetVisible(Shell::GetInstance()->IsMaximizeModeWindowManagerEnabled() &&
+             ShouldBeVisibleForLoginStatus(status));

[flackr, 2014/03/26 21:00:12]

Try to avoid repeating logic like this, it makes it easy for someone to
accidentally change it in one place and not the other. I'd suggest a common
UpdateVisibility which (for now) takes the state of maximize mode as an
argument.

[jonross, 2014/03/28 16:47:04]

Done.

+}
+
+bool OverviewButtonTray::ShouldBeVisibleForLoginStatus(
+    user::LoginStatus status) {
+  return status == user::LOGGED_IN_USER ||
+         status == user::LOGGED_IN_OWNER ||
+         status == user::LOGGED_IN_GUEST ||
+         status == user::LOGGED_IN_PUBLIC ||
+         status == user::LOGGED_IN_LOCALLY_MANAGED;

[flackr, 2014/03/26 21:00:12]

Can you try to unify this with
https://code.google.com/p/chromium/codesearch#chromium/src/ash/wm/overview/wi...
? These should use the same condition, probably the same function.

[jonross, 2014/03/28 16:47:04]

Done.

+}
+
 }  // namespace ash