Re-introduce the partial magnifier

chrome/browser/chromeos/accessibility/magnification_manager.cc

 // Copyright (c) 2012 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 "chrome/browser/chromeos/accessibility/magnification_manager.h"
 
 #include "ash/magnifier/magnification_controller.h"
 #include "ash/magnifier/partial_magnification_controller.h"
 #include "ash/shell.h"
 #include "ash/system/tray/system_tray_notifier.h"
@@ skipping 16 matching lines @@
 namespace {
 const double kInitialMagnifiedScale = 2.0;
 static MagnificationManager* g_magnification_manager = NULL;
 }
 
 class MagnificationManagerImpl : public MagnificationManager,
                                  public content::NotificationObserver {
  public:
   MagnificationManagerImpl() : first_time_update_(true),
                                profile_(NULL),
-                               type_(ash::MAGNIFIER_OFF) {
+                               type_(ash::DEFAULT_MAGNIFIER_TYPE),
+                               enabled_(false) {
     registrar_.Add(this,
                   chrome::NOTIFICATION_SESSION_STARTED,
                   content::NotificationService::AllSources());
     registrar_.Add(this,
                   chrome::NOTIFICATION_PROFILE_DESTROYED,
                   content::NotificationService::AllSources());
     registrar_.Add(this,
                   chrome::NOTIFICATION_LOGIN_WEBUI_VISIBLE,
                   content::NotificationService::AllSources());
   }
 
   virtual ~MagnificationManagerImpl() {
     CHECK(this == g_magnification_manager);
   }
 
   // MagnificationManager implimentation:
-  ash::MagnifierType GetMagnifierType() OVERRIDE {
+  bool IsMagnifierEnabled() const OVERRIDE {
+    return enabled_;
+  }
+
+  ash::MagnifierType GetMagnifierType() const OVERRIDE {
     return type_;
   }
 
-  void SetMagnifier(ash::MagnifierType type) OVERRIDE {
-    if (type == type_ && type == ash::MAGNIFIER_OFF)
+  void SetMagnifierEnabled(bool enabled) OVERRIDE {
+    if (enabled_ == enabled && !enabled)

[Daniel Erat, 2013/01/08 00:28:49]

i'm a bit surprised by this -- you need to do the rest of the work if enabled_
stayed true?  if this is correct, please add a comment explaining why it's
needed.

[yoshiki, 2013/01/09 03:03:15]

We need the rest of the work. Added a comment.

On 2013/01/08 00:28:49, Daniel Erat wrote:

       return;
 
+    enabled_ = enabled;
+
+    if (profile_) {
+      PrefService* prefs = profile_->GetPrefs();
+      DCHECK(prefs);
+      if (enabled != prefs->GetBoolean(prefs::kScreenMagnifierEnabled)) {

[Daniel Erat, 2013/01/08 00:28:49]

shouldn't we already match the pref?  why is this check needed?

[yoshiki, 2013/01/09 03:03:15]

We need to keep it for the reason in the above comment.

On 2013/01/08 00:28:49, Daniel Erat wrote:

+        prefs->SetBoolean(prefs::kScreenMagnifierEnabled, enabled);
+        prefs->CommitPendingWrite();
+      }
+    }
+
+    NotifyMagnifierChanged();
+
+    if (type_ == ash::MAGNIFIER_FULL) {
+      ash::Shell::GetInstance()->magnification_controller()->SetEnabled(
+          enabled_);
+    } else {
+      ash::Shell::GetInstance()->partial_magnification_controller()->SetEnabled(
+          enabled_);
+    }
+  }
+
+  void SetMagnifierType(ash::MagnifierType type) OVERRIDE {
+    if (type_ == type)
+      return;
+
+    DCHECK(type == ash::MAGNIFIER_FULL || type == ash::MAGNIFIER_PARTIAL);
     type_ = type;
 
     if (profile_) {
       PrefService* prefs = profile_->GetPrefs();
-      if (prefs) {
-        bool enabled = (type != ash::MAGNIFIER_OFF);
-        if (enabled != prefs->GetBoolean(prefs::kScreenMagnifierEnabled)) {
-          prefs->SetBoolean(prefs::kScreenMagnifierEnabled, enabled);
-          prefs->CommitPendingWrite();
-        }
+      DCHECK(prefs);
+      if (type != prefs->GetInteger(prefs::kScreenMagnifierType)) {

[Daniel Erat, 2013/01/08 00:28:49]

ditto here

[yoshiki, 2013/01/09 03:03:15]

Done. This can be removed.

+        prefs->SetInteger(prefs::kScreenMagnifierType, type);
+        prefs->CommitPendingWrite();
       }
     }
 
-    accessibility::AccessibilityStatusEventDetails details(
-        type != ash::MAGNIFIER_OFF, ash::A11Y_NOTIFICATION_NONE);
-    content::NotificationService::current()->Notify(
-        chrome::NOTIFICATION_CROS_ACCESSIBILITY_TOGGLE_SCREEN_MAGNIFIER,
-        content::NotificationService::AllSources(),
-        content::Details<accessibility::AccessibilityStatusEventDetails>(
-            &details));
+    NotifyMagnifierChanged();
 
-    ash::Shell::GetInstance()->magnification_controller()->SetEnabled(
-        type == ash::MAGNIFIER_FULL);
-    ash::Shell::GetInstance()->partial_magnification_controller()->SetEnabled(
-        type == ash::MAGNIFIER_PARTIAL);
+    if (enabled_) {
+      ash::Shell::GetInstance()->magnification_controller()->SetEnabled(
+          type_ == ash::MAGNIFIER_FULL);
+      ash::Shell::GetInstance()->partial_magnification_controller()->SetEnabled(
+          type_ == ash::MAGNIFIER_PARTIAL);
+    }
   }
 
   void SaveScreenMagnifierScale(double scale) OVERRIDE {
     Profile* profile = ProfileManager::GetDefaultProfileOrOffTheRecord();
+    DCHECK(profile->GetPrefs());
     profile->GetPrefs()->SetDouble(prefs::kScreenMagnifierScale, scale);
   }
 
-  double GetSavedScreenMagnifierScale() OVERRIDE {
+  double GetSavedScreenMagnifierScale() const OVERRIDE {
     Profile* profile = ProfileManager::GetDefaultProfileOrOffTheRecord();
+    DCHECK(profile->GetPrefs());
     if (profile->GetPrefs()->HasPrefPath(prefs::kScreenMagnifierScale))
       return profile->GetPrefs()->GetDouble(prefs::kScreenMagnifierScale);
     return std::numeric_limits<double>::min();
   }
 
  private:
+  void NotifyMagnifierChanged() {
+      accessibility::AccessibilityStatusEventDetails details(
+          enabled_, type_, ash::A11Y_NOTIFICATION_NONE);
+      content::NotificationService::current()->Notify(
+          chrome::NOTIFICATION_CROS_ACCESSIBILITY_TOGGLE_SCREEN_MAGNIFIER,
+          content::NotificationService::AllSources(),
+          content::Details<accessibility::AccessibilityStatusEventDetails>(
+              &details));
+  }
+
+  bool IsMagnifierEnabledFromPref() {
+    if (!profile_)
+      return false;
+
+    DCHECK(profile_->GetPrefs());
+    return profile_->GetPrefs()->GetBoolean(prefs::kScreenMagnifierEnabled);
+  }
+
   ash::MagnifierType GetMagnifierTypeFromPref() {
     if (!profile_)
-      return ash::MAGNIFIER_OFF;
+      return ash::DEFAULT_MAGNIFIER_TYPE;
 
-    PrefService* prefs = profile_->GetPrefs();
-    if (!prefs)
-      return ash::MAGNIFIER_OFF;
+    DCHECK(profile_->GetPrefs());
+    ash::MagnifierType type = static_cast<ash::MagnifierType>(
+        profile_->GetPrefs()->GetInteger(prefs::kScreenMagnifierType));
 
-    return prefs->GetBoolean(prefs::kScreenMagnifierEnabled) ?
-        ash::MAGNIFIER_FULL : ash::MAGNIFIER_OFF;
+    if (type == ash::MAGNIFIER_FULL || type == ash::MAGNIFIER_PARTIAL)
+      return type;
+
+    return ash::DEFAULT_MAGNIFIER_TYPE;
   }
 
   void SetProfile(Profile* profile) {
     if (pref_change_registrar_) {
       pref_change_registrar_.reset();
     }
 
     if (profile) {
       pref_change_registrar_.reset(new PrefChangeRegistrar);
       pref_change_registrar_->Init(profile->GetPrefs());
       pref_change_registrar_->Add(
           prefs::kScreenMagnifierEnabled,
           base::Bind(&MagnificationManagerImpl::UpdateMagnifierStatus,
                      base::Unretained(this)));
+      pref_change_registrar_->Add(
+          prefs::kScreenMagnifierType,
+          base::Bind(&MagnificationManagerImpl::UpdateMagnifierStatus,
+                     base::Unretained(this)));
     }
 
     profile_ = profile;
     UpdateMagnifierStatus();
   }
 
   void UpdateMagnifierStatus() {

[Daniel Erat, 2013/01/08 00:28:49]

nit: maybe rename this to something like UpdateMagnifierStatusFromPref()?

[yoshiki, 2013/01/09 03:03:15]

Done.

-    ash::MagnifierType type = GetMagnifierTypeFromPref();
-    SetMagnifier(type);
+    bool enabled = IsMagnifierEnabledFromPref();
+    if (!enabled) {
+      SetMagnifierEnabled(enabled);
+      SetMagnifierType(GetMagnifierTypeFromPref());
+    } else {
+      SetMagnifierType(GetMagnifierTypeFromPref());
+      SetMagnifierEnabled(enabled);
+    }
   }
 
   // content::NotificationObserver implimentation:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE {
     switch (type) {
       case chrome::NOTIFICATION_LOGIN_WEBUI_VISIBLE:
       case chrome::NOTIFICATION_SESSION_STARTED: {
         Profile* profile = ProfileManager::GetDefaultProfileOrOffTheRecord();
         SetProfile(profile);
         break;
       }
       case chrome::NOTIFICATION_PROFILE_DESTROYED: {
         SetProfile(NULL);
         break;
       }
     }
   }
 
   bool first_time_update_;
   Profile* profile_;
   ash::MagnifierType type_;
+  bool enabled_;
   content::NotificationRegistrar registrar_;
   scoped_ptr<PrefChangeRegistrar> pref_change_registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(MagnificationManagerImpl);
 };
 
 // static
 void MagnificationManager::Initialize() {
   CHECK(g_magnification_manager == NULL);
   g_magnification_manager = new MagnificationManagerImpl();
 }
 
 // static
 void MagnificationManager::Shutdown() {
   CHECK(g_magnification_manager);
   delete g_magnification_manager;
   g_magnification_manager = NULL;
 }
 
 // static
 MagnificationManager* MagnificationManager::Get() {
   return g_magnification_manager;
 }
 
 }  // namespace chromeos