Skip to content

Commit

Permalink
Move knowledge of screen lock state out of MessageCenter, confine to
Browse files Browse the repository at this point in the history
MessageCenterView.

- Update login state notification blocker to hide all notifications in
  message center when screen is locked to maintain the behavior:
      "Don't make notifications read when opening the message 
       center on lock screen"
- Remove methods/variables/structs related to UnreadCount, which is
  not used anywhere except in tests.
- It's questionable whether we have any reason to still track is_read
  at all, but it's used for UMA in at least one place.

Bug: 755413
Change-Id: Ic00a24900992735870338a9544e6af61da744974
Reviewed-on: https://chromium-review.googlesource.com/757132
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515765}
  • Loading branch information
Evan Stade authored and Commit Bot committed Nov 11, 2017
1 parent 8b7c80a commit 0798aac
Show file tree
Hide file tree
Showing 18 changed files with 55 additions and 236 deletions.
29 changes: 9 additions & 20 deletions ash/message_center/message_center_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#include "ash/message_center/message_center_style.h"
#include "ash/message_center/notifier_settings_view.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/session/session_controller.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
Expand Down Expand Up @@ -116,21 +118,8 @@ MessageCenterView::MessageCenterView(MessageCenter* message_center,
int max_height,
bool initially_settings_visible)
: message_center_(message_center),
tray_(tray),
scroller_(nullptr),
settings_view_(nullptr),
no_notifications_view_(nullptr),
button_bar_(nullptr),
settings_visible_(initially_settings_visible),
source_view_(nullptr),
source_height_(0),
target_view_(nullptr),
target_height_(0),
is_closing_(false),
is_locked_(message_center_->IsLockedState()),
mode_(Mode::NO_NOTIFICATIONS),
context_menu_controller_(this),
focus_manager_(nullptr) {
is_locked_(Shell::Get()->session_controller()->IsScreenLocked()) {
if (is_locked_)
mode_ = Mode::LOCKED;
else if (initially_settings_visible)
Expand Down Expand Up @@ -237,6 +226,12 @@ void MessageCenterView::ClearAllClosableNotifications() {
scroller_->GetVisibleRect());
}

void MessageCenterView::OnLockStateChanged(bool locked) {
is_locked_ = locked;
UpdateButtonBarStatus();
Update(true /* animate */);
}

void MessageCenterView::OnAllNotificationsCleared() {
SetViewHierarchyEnabled(scroller_, true);
button_bar_->SetCloseAllButtonEnabled(false);
Expand Down Expand Up @@ -441,12 +436,6 @@ void MessageCenterView::OnNotificationUpdated(const std::string& id) {
UpdateNotification(id);
}

void MessageCenterView::OnLockedStateChanged(bool locked) {
is_locked_ = locked;
UpdateButtonBarStatus();
Update(true /* animate */);
}

void MessageCenterView::OnQuietModeChanged(bool is_quiet_mode) {
settings_view_->SetQuietModeState(is_quiet_mode);
button_bar_->SetQuietModeState(is_quiet_mode);
Expand Down
31 changes: 19 additions & 12 deletions ash/message_center/message_center_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "ash/ash_export.h"
#include "ash/message_center/message_list_view.h"
#include "ash/session/session_observer.h"
#include "base/macros.h"
#include "ui/gfx/animation/animation_delegate.h"
#include "ui/message_center/message_center_observer.h"
Expand Down Expand Up @@ -43,6 +44,7 @@ class ASH_EXPORT MessageCenterView
: public views::View,
public message_center::MessageCenterObserver,
public message_center::MessageViewDelegate,
public SessionObserver,
public MessageListView::Observer,
public gfx::AnimationDelegate,
public views::FocusChangeListener {
Expand Down Expand Up @@ -94,7 +96,6 @@ class ASH_EXPORT MessageCenterView
void OnNotificationAdded(const std::string& id) override;
void OnNotificationRemoved(const std::string& id, bool by_user) override;
void OnNotificationUpdated(const std::string& id) override;
void OnLockedStateChanged(bool locked) override;
void OnQuietModeChanged(bool is_quiet_mode) override;

// Overridden from MessageViewDelegate:
Expand All @@ -108,6 +109,9 @@ class ASH_EXPORT MessageCenterView
void ClickOnSettingsButton(const std::string& notification_id) override;
void UpdateNotificationSize(const std::string& notification_id) override;

// Overridden from SessionObserver:
void OnLockStateChanged(bool locked) override;

// Overridden from MessageListView::Observer:
void OnAllNotificationsCleared() override;

Expand Down Expand Up @@ -151,11 +155,11 @@ class ASH_EXPORT MessageCenterView
message_center::MessageCenterTray* tray_;

// Child views.
views::ScrollView* scroller_;
views::ScrollView* scroller_ = nullptr;
std::unique_ptr<MessageListView> message_list_view_;
NotifierSettingsView* settings_view_;
views::View* no_notifications_view_;
MessageCenterButtonBar* button_bar_;
NotifierSettingsView* settings_view_ = nullptr;
views::View* no_notifications_view_ = nullptr;
MessageCenterButtonBar* button_bar_ = nullptr;

// Data for transition animation between settings view and message list.
bool settings_visible_;
Expand All @@ -166,25 +170,28 @@ class ASH_EXPORT MessageCenterView

// Helper data to keep track of the transition between settings and
// message center views.
views::View* source_view_;
int source_height_;
views::View* target_view_;
int target_height_;
views::View* source_view_ = nullptr;
int source_height_ = 0;
views::View* target_view_ = nullptr;
int target_height_ = 0;

// True when the widget is closing so that further operations should be
// ignored.
bool is_closing_;
bool is_closing_ = false;

bool is_clearing_all_notifications_ = false;
bool is_locked_ = false;
bool is_locked_;

// Current view mode. During animation, it is the target mode.
Mode mode_ = Mode::NO_NOTIFICATIONS;

message_center::MessageViewContextMenuController context_menu_controller_;
message_center::MessageViewContextMenuController context_menu_controller_{
this};

views::FocusManager* focus_manager_ = nullptr;

ScopedSessionObserver session_observer_{this};

DISALLOW_COPY_AND_ASSIGN(MessageCenterView);
};

Expand Down
4 changes: 1 addition & 3 deletions ash/message_center/message_center_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,8 @@ class FakeMessageCenterImpl : public FakeMessageCenter {
if (type == RemoveType::NON_PINNED)
remove_all_closable_notification_called_ = true;
}
bool IsLockedState() const override { return locked_; }
bool remove_all_closable_notification_called_ = false;
NotificationList::Notifications visible_notifications_;
bool locked_ = false;
};

// This is the class we are testing, but we need to override some functions
Expand Down Expand Up @@ -343,7 +341,7 @@ int MessageCenterViewTest::GetCallCount(CallType type) {
}

void MessageCenterViewTest::SetLockedState(bool locked) {
GetMessageCenterView()->OnLockedStateChanged(locked);
GetMessageCenterView()->OnLockStateChanged(locked);
}

void MessageCenterViewTest::ClickOnNotification(
Expand Down
2 changes: 0 additions & 2 deletions ash/system/status_area_widget.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,6 @@ void StatusAreaWidget::UpdateAfterLoginStatusChange(LoginStatus login_status) {
login_status_ = login_status;
if (system_tray_)
system_tray_->UpdateAfterLoginStatusChange(login_status);
if (web_notification_tray_)
web_notification_tray_->UpdateAfterLoginStatusChange(login_status);
if (logout_button_tray_)
logout_button_tray_->UpdateAfterLoginStatusChange();
if (overview_button_tray_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ LoginStateNotificationBlocker::~LoginStateNotificationBlocker() {
Shell::Get()->session_controller()->RemoveObserver(this);
}

bool LoginStateNotificationBlocker::ShouldShowNotification(
const message_center::Notification& notification) const {
return !Shell::Get()->session_controller()->IsScreenLocked();
}

bool LoginStateNotificationBlocker::ShouldShowNotificationAsPopup(
const message_center::Notification& notification) const {
if (ash::system_notifier::ShouldAlwaysShowPopups(notification.notifier_id()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace ash {

// A notification blocker which suppresses notifications popups based on the
// session state reported by the SessionController. Only active (logged in,
// unlocked) sessions will show popups.
// unlocked) sessions will show notifications.
class ASH_EXPORT LoginStateNotificationBlocker
: public message_center::NotificationBlocker,
public SessionObserver {
Expand All @@ -25,6 +25,8 @@ class ASH_EXPORT LoginStateNotificationBlocker

private:
// message_center::NotificationBlocker overrides:
bool ShouldShowNotification(
const message_center::Notification& notification) const override;
bool ShouldShowNotificationAsPopup(
const message_center::Notification& notification) const override;

Expand Down
6 changes: 0 additions & 6 deletions ash/system/web_notification/web_notification_tray.cc
Original file line number Diff line number Diff line change
Expand Up @@ -402,12 +402,6 @@ bool WebNotificationTray::IsMessageCenterBubbleVisible() const {
message_center_bubble()->bubble()->IsVisible());
}

void WebNotificationTray::UpdateAfterLoginStatusChange(
LoginStatus login_status) {
message_center()->SetLockedState(login_status == LoginStatus::LOCKED);
OnMessageCenterTrayChanged();
}

void WebNotificationTray::UpdateAfterShelfAlignmentChange() {
TrayBackgroundView::UpdateAfterShelfAlignmentChange();
// Destroy any existing bubble so that it will be rebuilt correctly.
Expand Down
3 changes: 0 additions & 3 deletions ash/system/web_notification/web_notification_tray.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ class ASH_EXPORT WebNotificationTray
// Returns true if the message center bubble is visible.
bool IsMessageCenterBubbleVisible() const;

// Called when the login status is changed.
void UpdateAfterLoginStatusChange(LoginStatus login_status);

// Overridden from TrayBackgroundView.
void UpdateAfterShelfAlignmentChange() override;
void AnchorUpdated() override;
Expand Down
10 changes: 0 additions & 10 deletions ui/message_center/fake_message_center.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ size_t FakeMessageCenter::NotificationCount() const {
return 0u;
}

size_t FakeMessageCenter::UnreadNotificationCount() const {
return 0u;
}

bool FakeMessageCenter::HasPopupNotifications() const {
return false;
}
Expand All @@ -43,10 +39,6 @@ bool FakeMessageCenter::IsQuietMode() const {
return false;
}

bool FakeMessageCenter::IsLockedState() const {
return false;
}

message_center::Notification* FakeMessageCenter::FindVisibleNotificationById(
const std::string& id) {
for (auto* notification : GetVisibleNotifications()) {
Expand Down Expand Up @@ -114,8 +106,6 @@ void FakeMessageCenter::DisplayedNotification(const std::string& id,

void FakeMessageCenter::SetQuietMode(bool in_quiet_mode) {}

void FakeMessageCenter::SetLockedState(bool locked) {}

void FakeMessageCenter::EnterQuietModeWithExpire(
const base::TimeDelta& expires_in) {
}
Expand Down
3 changes: 0 additions & 3 deletions ui/message_center/fake_message_center.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ class FakeMessageCenter : public MessageCenter {
void AddNotificationBlocker(NotificationBlocker* blocker) override;
void RemoveNotificationBlocker(NotificationBlocker* blocker) override;
size_t NotificationCount() const override;
size_t UnreadNotificationCount() const override;
bool HasPopupNotifications() const override;
bool IsQuietMode() const override;
bool IsLockedState() const override;
message_center::Notification* FindVisibleNotificationById(
const std::string& id) override;
const NotificationList::Notifications& GetVisibleNotifications() override;
Expand Down Expand Up @@ -59,7 +57,6 @@ class FakeMessageCenter : public MessageCenter {
void DisplayedNotification(const std::string& id,
const DisplaySource source) override;
void SetQuietMode(bool in_quiet_mode) override;
void SetLockedState(bool locked) override;
void EnterQuietModeWithExpire(const base::TimeDelta& expires_in) override;
void SetVisibility(Visibility visible) override;
bool IsMessageCenterVisible() const override;
Expand Down
5 changes: 0 additions & 5 deletions ui/message_center/message_center.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,8 @@ class MESSAGE_CENTER_EXPORT MessageCenter {

// Queries of current notification list status.
virtual size_t NotificationCount() const = 0;
virtual size_t UnreadNotificationCount() const = 0;
virtual bool HasPopupNotifications() const = 0;
virtual bool IsQuietMode() const = 0;
virtual bool IsLockedState() const = 0;

// Find the notification with the corresponding id. Returns null if not
// found. The returned instance is owned by the message center.
Expand Down Expand Up @@ -155,9 +153,6 @@ class MESSAGE_CENTER_EXPORT MessageCenter {
// This can be called to change the quiet mode state (without a timeout).
virtual void SetQuietMode(bool in_quiet_mode) = 0;

// This can be called to change the lock mode state.
virtual void SetLockedState(bool locked) = 0;

// Temporarily enables quiet mode for |expires_in| time.
virtual void EnterQuietModeWithExpire(const base::TimeDelta& expires_in) = 0;

Expand Down
Loading

0 comments on commit 0798aac

Please sign in to comment.