Add shortcut keys to ui scaling

3rd try

Add shortcut keys to ui scaling
- Round root window size because it can be fractional size
  when root_window_scale_ is specified.
- Remove scaling option from about:flags.
- Rotate display where the mouse is in.

BUG=179997, 119268
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188324

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188362

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188386

[James Cook, 2013-03-14 17:50:43]

A few suggestions and questions.

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
ash/accelerators/accelerator_controller.cc:176: static const int size =
arraysize(kScales);
size -> kScalesSize, or inline the use of arraysize(kScales)

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
ash/accelerators/accelerator_controller.cc:180: return
kScales[std::max(std::min(3, index), 0)];
Use size-1 instead of 3.

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
File ash/accelerators/accelerator_table.cc (right):

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
ash/accelerators/accelerator_table.cc:215: const AcceleratorAction
kActionsAllowedAtLoginOrLockScreen[] = {
Should scale-up/down be allowed at lock screen?

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
ash/accelerators/accelerator_table.cc:305: ROTATE_SCREEN,
I think scale-up and scale-down should be non-repeatable, especially if we ever
decide to add animations to them.

https://codereview.chromium.org/12848004/diff/14001/ash/display/display_manag...
File ash/display/display_manager.cc (right):

https://codereview.chromium.org/12848004/diff/14001/ash/display/display_manag...
ash/display/display_manager.cc:209: static bool disabled =
CommandLine::ForCurrentProcess()->
nit: Consider naming this variable "enabled" like above.

[oshima, 2013-03-14 19:56:15]

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
ash/accelerators/accelerator_controller.cc:176: static const int size =
arraysize(kScales);
On 2013/03/14 17:50:43, James Cook (Chromium) wrote:
> size -> kScalesSize, or inline the use of arraysize(kScales)

Done.

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
ash/accelerators/accelerator_controller.cc:180: return
kScales[std::max(std::min(3, index), 0)];
On 2013/03/14 17:50:43, James Cook (Chromium) wrote:
> Use size-1 instead of 3.

Done.

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
File ash/accelerators/accelerator_table.cc (right):

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
ash/accelerators/accelerator_table.cc:215: const AcceleratorAction
kActionsAllowedAtLoginOrLockScreen[] = {
On 2013/03/14 17:50:43, James Cook (Chromium) wrote:
> Should scale-up/down be allowed at lock screen?

Good point. Done.

https://codereview.chromium.org/12848004/diff/14001/ash/accelerators/accelera...
ash/accelerators/accelerator_table.cc:305: ROTATE_SCREEN,
On 2013/03/14 17:50:43, James Cook (Chromium) wrote:
> I think scale-up and scale-down should be non-repeatable, especially if we
ever
> decide to add animations to them.

Done.

https://codereview.chromium.org/12848004/diff/14001/ash/display/display_manag...
File ash/display/display_manager.cc (right):

https://codereview.chromium.org/12848004/diff/14001/ash/display/display_manag...
ash/display/display_manager.cc:209: static bool disabled =
CommandLine::ForCurrentProcess()->
On 2013/03/14 17:50:43, James Cook (Chromium) wrote:
> nit: Consider naming this variable "enabled" like above.

Done.

[James Cook, 2013-03-14 20:42:42]

lgtm

[commit-bot: I haz the power, 2013-03-14 20:52:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/12848004/19002

[commit-bot: I haz the power, 2013-03-14 20:52:15]

Failed to apply patch for ash/accelerators/accelerator_controller.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ash/accelerators/accelerator_controller.cc
  Hunk #1 FAILED at 154.
  Hunk #2 succeeded at 169 with fuzz 1.
  1 out of 4 hunks FAILED -- saving rejects to file
ash/accelerators/accelerator_controller.cc.rej

Patch:       ash/accelerators/accelerator_controller.cc
Index: ash/accelerators/accelerator_controller.cc
diff --git a/ash/accelerators/accelerator_controller.cc
b/ash/accelerators/accelerator_controller.cc
index
62b33ba7be86fc164ade89b5eb0ae96de3906c55..785c325e967c20aef0244dbf85c65e75d5cbbb66
100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -154,7 +154,7 @@ bool HandleRotateActiveWindow() {
   return true;
 }
 
-const DisplayInfo::Rotation GetNextRotation(DisplayInfo::Rotation current) {
+DisplayInfo::Rotation GetNextRotation(DisplayInfo::Rotation current) {
   switch (current) {
     case DisplayInfo::ROTATE_0:
       return DisplayInfo::ROTATE_90;
@@ -169,13 +169,40 @@ const DisplayInfo::Rotation
GetNextRotation(DisplayInfo::Rotation current) {
   return DisplayInfo::ROTATE_0;
 }
 
+float GetNextScale(float scale, int next) {
+  // These scales are equivalent to 1024, 1280, 1600 and 1920 pixel width
+  // respectively on 2560 pixel width 2x density display.
+  static const float kScales[] = {0.8f, 1.0f, 1.25f, 1.5f};
+  static const int kScaleTableSize = arraysize(kScales);
+  int index = std::distance(kScales, std::find(kScales,
+                                               kScales + kScaleTableSize,
+                                               scale));
+  // Fallback to 1.0f if the |scale| wasn't in the list.
+  index = index == kScaleTableSize ? 1 : index + next;
+  return kScales[std::max(std::min(kScaleTableSize - 1, index), 0)];
+}
+
+bool HandleScaleUI(bool up) {
+  // UI Scaling is effective only on internal display.
+  int64 display_id = gfx::Display::InternalDisplayId();
+#if defined(OS_CHROMEOS)
+  // On linux desktop, allow ui scaling on the first dislpay.
+  if (!base::chromeos::IsRunningOnChromeOS())
+    display_id = Shell::GetInstance()->display_manager()->first_display_id();
+#endif
+  const gfx::Display& display = Shell::GetInstance()->display_manager()->
+      GetDisplayForId(display_id);
+  const DisplayInfo& display_info = Shell::GetInstance()->display_manager()->
+      GetDisplayInfo(display);
+  Shell::GetInstance()->display_manager()->SetDisplayUIScale(
+      display.id(), GetNextScale(display_info.ui_scale(), up ? 1 : -1));
+  return true;
+}
+
 // Rotates the screen.
 bool HandleRotateScreen() {
-  aura::Window* active_window = wm::GetActiveWindow();
-  if (!active_window)
-    return false;
-  const gfx::Display& display =
-      Shell::GetScreen()->GetDisplayNearestWindow(active_window);
+  gfx::Point point = Shell::GetScreen()->GetCursorScreenPoint();
+  gfx::Display display = Shell::GetScreen()->GetDisplayNearestPoint(point);
   const DisplayInfo& display_info =
       Shell::GetInstance()->display_manager()->GetDisplayInfo(display);
   Shell::GetInstance()->display_manager()->SetDisplayRotation(
@@ -756,6 +783,10 @@ bool AcceleratorController::PerformAction(int action,
       }
       break;
     }
+    case SCALE_UI_UP:
+      return HandleScaleUI(true /* up */);
+    case SCALE_UI_DOWN:
+      return HandleScaleUI(false /* down */);
     case ROTATE_WINDOW:
       return HandleRotateActiveWindow();
     case ROTATE_SCREEN:
@@ -765,7 +796,7 @@ bool AcceleratorController::PerformAction(int action,
     case TOGGLE_ROOT_WINDOW_FULL_SCREEN:
       return HandleToggleRootWindowFullScreen();
     case DISPLAY_TOGGLE_SCALE:
-      internal::DisplayManager::ToggleDisplayScale();
+      internal::DisplayManager::ToggleDisplayScaleFactor();
       return true;
     case MAGNIFY_SCREEN_ZOOM_IN:
       return HandleMagnifyScreen(1);

[commit-bot: I haz the power, 2013-03-14 21:05:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/12848004/38001

[commit-bot: I haz the power, 2013-03-14 21:05:18]

Presubmit check for 12848004-38001 failed and returned exit status 1.

INFO:root:Found 10 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ash/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    ui/aura/root_window.cc

Presubmit checks took 4.3s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[oshima, 2013-03-14 21:06:56]

oops forgot about ui/aura

Scott, can you take a look?

[sky, 2013-03-14 22:52:17]

LGTM

[commit-bot: I haz the power, 2013-03-15 00:34:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/12848004/45001

[commit-bot: I haz the power, 2013-03-15 10:45:14]

Change committed as 188324

[oshima, 2013-03-15 12:23:16]

Committed patchset #7 manually as r188362 (presubmit successful).

[oshima, 2013-03-15 13:25:37]

Committed patchset #8 manually as r188386 (presubmit successful).

[oshima, 2013-03-15 16:04:07]

On 2013/03/15 13:25:37, oshima wrote:
> Committed patchset #8 manually as r188386 (presubmit successful).

quick update: chrome for chromeos bot (one in chromium.chrome)
was failing to compile the stl code in accelerator_controller.cc for
some reason. It could be because the bot is using older gcc (it says 4.4).

Because there is no bots to test, I simply removed stl code and 
replaced it with simple loop and if, and landed manually.

Please let me know if you have any concern about new code and I'll
address in follow up CL.

Thanks,

[James Cook, 2013-03-15 17:09:38]

LGTM with nit

https://codereview.chromium.org/12848004/diff/57001/ash/accelerators/accelera...
File ash/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/12848004/diff/57001/ash/accelerators/accelera...
ash/accelerators/accelerator_controller.cc:179: if (up && i != kScaleTableSize
-1)
nit: "kScaleTableSize - 1"