Skip to content

Commit

Permalink
Getting rid of GetDefaultProfile & fixing multi user issues with acce…
Browse files Browse the repository at this point in the history
…ssibility

As discussed: I removed the occurances, switched the accessibility functionality with a user change and made sure that the accessibility menu is presented when at least one user has one accessibility feature turned on.

Not done:
- ChromeVox - this will most likely only partially work in multi user scenarios. It makes no sense to get this 80% working if we are in the midst of overhauling the use of ChromeVox. This needs to be re-visited once the changes are done.
- BrailleDisplayAPI - Since this is coupled with ChromeVox I guess that this falls into the same problem.
- MagnifierManager - A refactor should be done which merges this into the AccessibilityManager, but that would go beyond what we currently have to do. Besides I have found a comment which indicates that this might even be underway.


BUG=322682
TEST=unit tests (and multi profile by visual tests)

Review URL: https://codereview.chromium.org/102483006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241010 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
skuhne@chromium.org committed Dec 16, 2013
1 parent d28775d commit c201225
Show file tree
Hide file tree
Showing 14 changed files with 163 additions and 26 deletions.
5 changes: 2 additions & 3 deletions ash/accessibility_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,8 @@ class ASH_EXPORT AccessibilityDelegate {
// Returns if autoclick is enabled or not.
virtual bool IsAutoclickEnabled() const = 0;

// Returns true if the user wants to show accesibility menu even when all the
// accessibility features are disabled.
virtual bool ShouldAlwaysShowAccessibilityMenu() const = 0;
// Returns true when the accessibility menu should be shown.
virtual bool ShouldShowAccessibilityMenu() const = 0;

// Cancel all current and queued speech immediately.
virtual void SilenceSpokenFeedback() const = 0;
Expand Down
1 change: 1 addition & 0 deletions ash/ash.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@
'screensaver/screensaver_view.h',
'screenshot_delegate.h',
'session_state_delegate.h',
'session_state_observer.cc',
'session_state_observer.h',
'shelf/alternate_app_list_button.cc',
'shelf/alternate_app_list_button.h',
Expand Down
8 changes: 6 additions & 2 deletions ash/default_accessibility_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,12 @@ bool DefaultAccessibilityDelegate::IsAutoclickEnabled() const {
return autoclick_enabled_;
}

bool DefaultAccessibilityDelegate::ShouldAlwaysShowAccessibilityMenu() const {
return false;
bool DefaultAccessibilityDelegate::ShouldShowAccessibilityMenu() const {
return spoken_feedback_enabled_ ||
high_contrast_enabled_ ||
screen_magnifier_enabled_ ||
large_cursor_enabled_ ||
autoclick_enabled_;
}

void DefaultAccessibilityDelegate::SilenceSpokenFeedback() const {
Expand Down
2 changes: 1 addition & 1 deletion ash/default_accessibility_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class ASH_EXPORT DefaultAccessibilityDelegate : public AccessibilityDelegate {
virtual bool IsLargeCursorEnabled() const OVERRIDE;
virtual void SetAutoclickEnabled(bool enabled) OVERRIDE;
virtual bool IsAutoclickEnabled() const OVERRIDE;
virtual bool ShouldAlwaysShowAccessibilityMenu() const OVERRIDE;
virtual bool ShouldShowAccessibilityMenu() const OVERRIDE;
virtual void SilenceSpokenFeedback() const OVERRIDE;
virtual void ToggleSpokenFeedback(
AccessibilityNotificationVisibility notify) OVERRIDE;
Expand Down
24 changes: 24 additions & 0 deletions ash/session_state_observer.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2013 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/session_state_observer.h"

#include "ash/session_state_delegate.h"
#include "ash/shell.h"

namespace ash {

ScopedSessionStateObserver::ScopedSessionStateObserver(
ash::SessionStateObserver* observer) : observer_(observer) {
ash::Shell::GetInstance()->session_state_delegate()->
AddSessionStateObserver(observer_);
}

ScopedSessionStateObserver::~ScopedSessionStateObserver() {
ash::Shell::GetInstance()->session_state_delegate()->
RemoveSessionStateObserver(observer_);
}

} // namespace ash

14 changes: 14 additions & 0 deletions ash/session_state_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <string>

#include "ash/ash_export.h"
#include "base/basictypes.h"

namespace ash {

Expand All @@ -23,6 +24,19 @@ class ASH_EXPORT SessionStateObserver {
virtual ~SessionStateObserver() {}
};

// A class to attach / detach an object as a session state observer with a
// scoped pointer.
class ASH_EXPORT ScopedSessionStateObserver {
public:
explicit ScopedSessionStateObserver(ash::SessionStateObserver* observer);
virtual ~ScopedSessionStateObserver();

private:
ash::SessionStateObserver* observer_;

DISALLOW_COPY_AND_ASSIGN(ScopedSessionStateObserver);
};

} // namespace ash

#endif // ASH_SESSION_STATE_OBSERVER_H_
7 changes: 3 additions & 4 deletions ash/system/tray_accessibility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,9 @@ views::View* TrayAccessibility::CreateDefaultView(user::LoginStatus status) {
AccessibilityDelegate* delegate =
Shell::GetInstance()->accessibility_delegate();
if (login_ != user::LOGGED_IN_NONE &&
!delegate->ShouldAlwaysShowAccessibilityMenu() &&
// On login screen, keeps the initial visivility of the menu.
(status != user::LOGGED_IN_LOCKED || !show_a11y_menu_on_lock_screen_) &&
GetAccessibilityState() == A11Y_NONE)
!delegate->ShouldShowAccessibilityMenu() &&
// On login screen, keeps the initial visibility of the menu.
(status != user::LOGGED_IN_LOCKED || !show_a11y_menu_on_lock_screen_))
return NULL;

CHECK(default_ == NULL);
Expand Down
36 changes: 34 additions & 2 deletions chrome/browser/chromeos/accessibility/accessibility_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "ash/autoclick/autoclick_controller.h"
#include "ash/high_contrast/high_contrast_controller.h"
#include "ash/metrics/user_metrics_recorder.h"
#include "ash/session_state_delegate.h"
#include "ash/shell.h"
#include "ash/system/tray/system_tray_notifier.h"
#include "ash/wm/event_rewriter_event_filter.h"
Expand Down Expand Up @@ -322,6 +323,7 @@ AccessibilityManager::AccessibilityManager()
notification_registrar_.Add(this,
chrome::NOTIFICATION_EXTENSION_UNLOADED,
content::NotificationService::AllSources());

GetBrailleController()->AddObserver(this);

ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
Expand All @@ -347,6 +349,27 @@ AccessibilityManager::~AccessibilityManager() {
}
}

bool AccessibilityManager::ShouldShowAccessibilityMenu() {
// If any of the loaded profiles has an accessibility feature turned on - or
// enforced to always show the menu - we return true to show the menu.
std::vector<Profile*> profiles =
g_browser_process->profile_manager()->GetLoadedProfiles();
for (std::vector<Profile*>::iterator it = profiles.begin();
it != profiles.end();
++it) {
PrefService* pref_service = (*it)->GetPrefs();
if (pref_service->GetBoolean(prefs::kStickyKeysEnabled) ||
pref_service->GetBoolean(prefs::kLargeCursorEnabled) ||
pref_service->GetBoolean(prefs::kSpokenFeedbackEnabled) ||
pref_service->GetBoolean(prefs::kHighContrastEnabled) ||
pref_service->GetBoolean(prefs::kAutoclickEnabled) ||
pref_service->GetBoolean(prefs::kShouldAlwaysShowAccessibilityMenu) ||
pref_service->GetBoolean(prefs::kScreenMagnifierEnabled))
return true;
}
return false;
}

void AccessibilityManager::EnableLargeCursor(bool enabled) {
if (!profile_)
return;
Expand Down Expand Up @@ -739,6 +762,10 @@ void AccessibilityManager::SetProfile(Profile* profile) {
UpdateAutoclickDelayFromPref();
}

void AccessibilityManager::ActiveUserChanged(const std::string& user_id) {
SetProfile(ProfileManager::GetActiveUserProfile());
}

void AccessibilityManager::SetProfileForTest(Profile* profile) {
SetProfile(profile);
}
Expand Down Expand Up @@ -807,17 +834,22 @@ void AccessibilityManager::Observe(
switch (type) {
case chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE: {
// Update |profile_| when entering the login screen.
Profile* profile = ProfileManager::GetDefaultProfile();
Profile* profile = ProfileManager::GetActiveUserProfile();
if (ProfileHelper::IsSigninProfile(profile))
SetProfile(profile);
break;
}
case chrome::NOTIFICATION_SESSION_STARTED:
// Update |profile_| when entering a session.
SetProfile(ProfileManager::GetDefaultProfile());
SetProfile(ProfileManager::GetActiveUserProfile());

// Ensure ChromeVox makes announcements at the start of new sessions.
should_speak_chrome_vox_announcements_on_user_screen_ = true;

// Add a session state observer to be able to monitor session changes.
if (!session_state_observer_.get() && ash::Shell::HasInstance())
session_state_observer_.reset(
new ash::ScopedSessionStateObserver(this));
break;
case chrome::NOTIFICATION_PROFILE_DESTROYED: {
// Update |profile_| when exiting a session or shutting down.
Expand Down
11 changes: 10 additions & 1 deletion chrome/browser/chromeos/accessibility/accessibility_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define CHROME_BROWSER_CHROMEOS_ACCESSIBILITY_ACCESSIBILITY_MANAGER_H_

#include "ash/accessibility_delegate.h"
#include "ash/session_state_observer.h"
#include "base/memory/weak_ptr.h"
#include "base/prefs/pref_change_registrar.h"
#include "base/time/time.h"
Expand Down Expand Up @@ -40,7 +41,8 @@ struct AccessibilityStatusEventDetails {
// TODO(yoshiki): merge MagnificationManager with AccessibilityManager.
class AccessibilityManager : public content::NotificationObserver,
public extensions::EventRouter::Observer,
extensions::api::braille_display_private::BrailleObserver {
extensions::api::braille_display_private::BrailleObserver,
public ash::SessionStateObserver {
public:
// Creates an instance of AccessibilityManager, this should be called once,
// because only one instance should exist at the same time.
Expand All @@ -67,6 +69,9 @@ class AccessibilityManager : public content::NotificationObserver,
const char* pref_path_;
};

// Returns true when the accessibility menu should be shown.
bool ShouldShowAccessibilityMenu();

// Enables or disables the large cursor.
void EnableLargeCursor(bool enabled);
// Returns true if the large cursor is enabled, or false if not.
Expand Down Expand Up @@ -110,6 +115,9 @@ class AccessibilityManager : public content::NotificationObserver,
// Returns the autoclick delay in milliseconds.
int GetAutoclickDelay() const;

// SessionStateObserver overrides:
virtual void ActiveUserChanged(const std::string& user_id) OVERRIDE;

void SetProfileForTest(Profile* profile);

static void SetBrailleControllerForTest(
Expand Down Expand Up @@ -185,6 +193,7 @@ class AccessibilityManager : public content::NotificationObserver,
content::NotificationRegistrar notification_registrar_;
scoped_ptr<PrefChangeRegistrar> pref_change_registrar_;
scoped_ptr<PrefChangeRegistrar> local_state_pref_change_registrar_;
scoped_ptr<ash::ScopedSessionStateObserver> session_state_observer_;

PrefHandler large_cursor_pref_handler_;
PrefHandler spoken_feedback_pref_handler_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ bool IsLargeCursorEnabled() {
return AccessibilityManager::Get()->IsLargeCursorEnabled();
}

bool ShouldShowAccessibilityMenu() {
return AccessibilityManager::Get()->ShouldShowAccessibilityMenu();
}

void SetHighContrastEnabled(bool enabled) {
return AccessibilityManager::Get()->EnableHighContrast(enabled);
}
Expand Down Expand Up @@ -560,4 +564,41 @@ IN_PROC_BROWSER_TEST_P(AccessibilityManagerUserTypeTest,
EXPECT_EQ(kTestAutoclickDelayMs, GetAutoclickDelayFromPref());
}

IN_PROC_BROWSER_TEST_F(AccessibilityManagerTest, AcessibilityMenuVisibility) {
// Log in.
UserManager::Get()->UserLoggedIn(kTestUserName, kTestUserName, true);
UserManager::Get()->SessionStarted();

// Confirms that the features are disabled.
EXPECT_FALSE(IsLargeCursorEnabled());
EXPECT_FALSE(IsSpokenFeedbackEnabled());
EXPECT_FALSE(IsHighContrastEnabled());
EXPECT_FALSE(IsAutoclickEnabled());
EXPECT_FALSE(ShouldShowAccessibilityMenu());

// Check large cursor.
SetLargeCursorEnabled(true);
EXPECT_TRUE(ShouldShowAccessibilityMenu());
SetLargeCursorEnabled(false);
EXPECT_FALSE(ShouldShowAccessibilityMenu());

// Check spoken feedback.
SetSpokenFeedbackEnabled(true);
EXPECT_TRUE(ShouldShowAccessibilityMenu());
SetSpokenFeedbackEnabled(false);
EXPECT_FALSE(ShouldShowAccessibilityMenu());

// Check high contrast.
SetHighContrastEnabled(true);
EXPECT_TRUE(ShouldShowAccessibilityMenu());
SetHighContrastEnabled(false);
EXPECT_FALSE(ShouldShowAccessibilityMenu());

// Check autoclick.
SetAutoclickEnabled(true);
EXPECT_TRUE(ShouldShowAccessibilityMenu());
SetAutoclickEnabled(false);
EXPECT_FALSE(ShouldShowAccessibilityMenu());
}

} // namespace chromeos
16 changes: 15 additions & 1 deletion chrome/browser/chromeos/accessibility/magnification_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "ash/magnifier/magnification_controller.h"
#include "ash/magnifier/partial_magnification_controller.h"
#include "ash/session_state_delegate.h"
#include "ash/shell.h"
#include "ash/shell_delegate.h"
#include "ash/system/tray/system_tray_notifier.h"
Expand Down Expand Up @@ -35,7 +36,8 @@ static MagnificationManager* g_magnification_manager = NULL;
}

class MagnificationManagerImpl : public MagnificationManager,
public content::NotificationObserver {
public content::NotificationObserver,
public ash::SessionStateObserver {
public:
MagnificationManagerImpl()
: first_time_update_(true),
Expand Down Expand Up @@ -105,6 +107,11 @@ class MagnificationManagerImpl : public MagnificationManager,
SetProfile(profile);
}

// SessionStateObserver overrides:
virtual void ActiveUserChanged(const std::string& user_id) OVERRIDE {
SetProfile(ProfileManager::GetActiveUserProfile());
}

private:
void SetProfile(Profile* profile) {
pref_change_registrar_.reset();
Expand Down Expand Up @@ -209,6 +216,11 @@ class MagnificationManagerImpl : public MagnificationManager,
case chrome::NOTIFICATION_SESSION_STARTED:
// Update |profile_| when entering a session.
SetProfile(ProfileManager::GetDefaultProfile());

// Add a session state observer to be able to monitor session changes.
if (!session_state_observer_.get() && ash::Shell::HasInstance())
session_state_observer_.reset(
new ash::ScopedSessionStateObserver(this));
break;
case chrome::NOTIFICATION_PROFILE_DESTROYED: {
// Update |profile_| when exiting a session or shutting down.
Expand All @@ -229,8 +241,10 @@ class MagnificationManagerImpl : public MagnificationManager,

ash::MagnifierType type_;
bool enabled_;

content::NotificationRegistrar registrar_;
scoped_ptr<PrefChangeRegistrar> pref_change_registrar_;
scoped_ptr<ash::ScopedSessionStateObserver> session_state_observer_;

DISALLOW_COPY_AND_ASSIGN(MagnificationManagerImpl);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,14 @@ bool BrailleDisplayPrivateAPI::IsProfileActive() {
Profile* active_profile;
chromeos::ScreenLocker* screen_locker =
chromeos::ScreenLocker::default_screen_locker();
if (screen_locker && screen_locker->locked())
if (screen_locker && screen_locker->locked()) {
active_profile = chromeos::ProfileHelper::GetSigninProfile();
else
active_profile = ProfileManager::GetDefaultProfile();
} else {
// Since we are creating one instance per profile / user, we should be fine
// comparing against the active user. That said - if we ever change that,
// this code will need to be changed.
active_profile = ProfileManager::GetActiveUserProfile();
}
return profile_->IsSameProfile(active_profile);
#else // !defined(OS_CHROMEOS)
return true;
Expand Down
12 changes: 4 additions & 8 deletions chrome/browser/ui/ash/chrome_shell_delegate_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,10 @@ class AccessibilityDelegateImpl : public ash::AccessibilityDelegate {
return chromeos::AccessibilityManager::Get()->IsAutoclickEnabled();
}

virtual bool ShouldAlwaysShowAccessibilityMenu() const OVERRIDE {
Profile* profile = ProfileManager::GetActiveUserProfileOrOffTheRecord();
if (!profile)
return false;

PrefService* user_pref_service = profile->GetPrefs();
return user_pref_service && user_pref_service->GetBoolean(
prefs::kShouldAlwaysShowAccessibilityMenu);
virtual bool ShouldShowAccessibilityMenu() const OVERRIDE {
DCHECK(chromeos::AccessibilityManager::Get());
return chromeos::AccessibilityManager::Get()->
ShouldShowAccessibilityMenu();
}

virtual void SilenceSpokenFeedback() const OVERRIDE {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/ash/chrome_shell_delegate_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class EmptyAccessibilityDelegate : public ash::AccessibilityDelegate {
return std::numeric_limits<double>::min();
}

virtual bool ShouldAlwaysShowAccessibilityMenu() const OVERRIDE {
virtual bool ShouldShowAccessibilityMenu() const OVERRIDE {
return false;
}

Expand Down

0 comments on commit c201225

Please sign in to comment.