Re-introduce the partial magnifier

chrome/browser/chromeos/accessibility/accessibility_util.cc

Index: chrome/browser/chromeos/accessibility/accessibility_util.cc
diff --git a/chrome/browser/chromeos/accessibility/accessibility_util.cc b/chrome/browser/chromeos/accessibility/accessibility_util.cc
index faa58f9ba2cfa828ce3117e50900bb4c542067e7..a91037e986532d004dc961cd1c5c30f6d59d29ef 100644
--- a/chrome/browser/chromeos/accessibility/accessibility_util.cc
+++ b/chrome/browser/chromeos/accessibility/accessibility_util.cc
@@ -50,10 +50,6 @@ using content::RenderViewHost;
 namespace chromeos {
 namespace accessibility {
 
-const char kScreenMagnifierOff[] = "";
-const char kScreenMagnifierFull[] = "full";
-const char kScreenMagnifierPartial[] = "partial";
-
 // Helper class that directly loads an extension's content scripts into
 // all of the frames corresponding to a given RenderViewHost.
 class ContentScriptLoader {
@@ -123,10 +119,12 @@ void UpdateChromeOSAccessibilityHistograms() {
                         IsHighContrastEnabled());
   UMA_HISTOGRAM_BOOLEAN("Accessibility.CrosVirtualKeyboard",
                         IsVirtualKeyboardEnabled());
-  if (MagnificationManager::Get())
-    UMA_HISTOGRAM_ENUMERATION("Accessibility.CrosScreenMagnifier",
-                              MagnificationManager::Get()->GetMagnifierType(),
-                              3);
+  if (MagnificationManager::Get()) {
+    uint32 type = MagnificationManager::Get()->IsMagnifierEnabled() ?
+                      MagnificationManager::Get()->GetMagnifierType() : 0;
+    // '0' means magnifier is disabled.
+    UMA_HISTOGRAM_ENUMERATION("Accessibility.CrosScreenMagnifier", type, 3);

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

is there any chance that more values will be added in the future?  if so, do you
need to reserve more space here?  can you double-check with mark pearson?

also, i'd recommend adding some kNumMagnifierTypes = 3 enum value in
magnifier_constants.h and using that here instead of hardcoding a constant. 
(i'm not sure whether kNumMagnifierTypes or NUM_MAGNIFIER_TYPES is more common
here; you might want to try to see what's done elsewhere.)

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

This value is same as the previous code, which is wrote by dmazzoni@. I think
this is ok to keep as it is.

And changed '3' with |kNumMagnifierTypes| defined in magnifier_constants.h.

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

+  }
 }
 
 void Initialize() {
@@ -290,27 +288,6 @@ bool IsVirtualKeyboardEnabled() {
   return virtual_keyboard_enabled;
 }
 
-ash::MagnifierType MagnifierTypeFromName(const char type_name[]) {
-  if (0 == strcmp(type_name, kScreenMagnifierFull))
-    return ash::MAGNIFIER_FULL;
-  else if (0 == strcmp(type_name, kScreenMagnifierPartial))
-    return ash::MAGNIFIER_PARTIAL;
-  else
-    return ash::MAGNIFIER_OFF;
-}
-
-const char* ScreenMagnifierNameFromType(ash::MagnifierType type) {
-  switch (type) {
-    case ash::MAGNIFIER_OFF:
-      return kScreenMagnifierOff;
-    case ash::MAGNIFIER_FULL:
-      return kScreenMagnifierFull;
-    case ash::MAGNIFIER_PARTIAL:
-      return kScreenMagnifierPartial;
-  }
-  return kScreenMagnifierOff;
-}
-
 void MaybeSpeak(const std::string& utterance) {
   if (IsSpokenFeedbackEnabled())
     Speak(utterance);