Don't move cursor location when rotation /ui scaling has changed

Don't move cursor location when rotation /ui scaling has changed
 Moved EnsurePointerInDisplays to DisplayController as it needs to access RootWindow.


BUG=226300
TEST=covered by test.

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

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

[Jun Mukai, 2013-04-06 03:13:39]

nitpicks...

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_contr...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_contr...
ash/display/display_controller.cc:741: target_location = location_in_screen;
in which case do we still need to call MoveCursorTo? Can we just return here?

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_contr...
ash/display/display_controller.cc:758: }
It seems that this method doesn't ensure it if dst_root_window is NULL, but it
also seems that dst_root_window never becomes NULL. Why not
DCHECK(dst_root_window != NULL) instead?

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_contr...
ash/display/display_controller.cc:786: gfx::Rect
bounds(root_window->GetHostOrigin(), root_window->GetHostSize());
bounds_in_native?

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

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_manag...
ash/display/display_manager.cc:439: gfx::Point native_mouse_location;
mouse_location_in_native would be more consistent?

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_manag...
ash/display/display_manager.cc:473:
display_controller->UpdateMouseCursor(native_mouse_location);
Why this needs to be called?

[oshima, 2013-04-08 18:00:40]

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_contr...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_contr...
ash/display/display_controller.cc:741: target_location = location_in_screen;
On 2013/04/06 03:13:39, Jun Mukai wrote:
> in which case do we still need to call MoveCursorTo? Can we just return here?

Yes, we need to update the cursor when screen was rotated.

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_contr...
ash/display/display_controller.cc:758: }
On 2013/04/06 03:13:39, Jun Mukai wrote:
> It seems that this method doesn't ensure it if dst_root_window is NULL, but it
> also seems that dst_root_window never becomes NULL. Why not
> DCHECK(dst_root_window != NULL) instead?

good point. done.

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_contr...
ash/display/display_controller.cc:786: gfx::Rect
bounds(root_window->GetHostOrigin(), root_window->GetHostSize());
On 2013/04/06 03:13:39, Jun Mukai wrote:
> bounds_in_native?

Done.

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

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_manag...
ash/display/display_manager.cc:439: gfx::Point native_mouse_location;
On 2013/04/06 03:13:39, Jun Mukai wrote:
> mouse_location_in_native would be more consistent?

Done.

https://codereview.chromium.org/13466022/diff/12003/ash/display/display_manag...
ash/display/display_manager.cc:473:
display_controller->UpdateMouseCursor(native_mouse_location);
On 2013/04/06 03:13:39, Jun Mukai wrote:
> Why this needs to be called?

Yes, we need to update the cursor when rotated.

[Jun Mukai, 2013-04-08 20:34:23]

lgtm

[commit-bot: I haz the power, 2013-04-08 20:38:54]

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

[commit-bot: I haz the power, 2013-04-08 21:00:57]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-04-08 23:22:58]

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

[commit-bot: I haz the power, 2013-04-09 00:30:47]

Failed to trigger a try job on win7_aura
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-04-09 00:30:54]

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

[commit-bot: I haz the power, 2013-04-09 01:02:39]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-04-09 01:05:35]

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

[commit-bot: I haz the power, 2013-04-09 04:48:00]

Change committed as 193008

[dcheng, 2013-04-09 07:54:32]

On 2013/04/09 04:48:00, I haz the power (commit-bot) wrote:
> Change committed as 193008

This appears to be causing issues on the ASAN bots. For example,
http://build.chromium.org/p/chromium.memory/builders/Chromium%20OS%20%28x86%2...:
=================================================================
==14367== ERROR: AddressSanitizer: SEGV on unknown address 0x00000000 (pc
0x6ee62291 sp 0x7fdefc20 bp 0x7fdefcf8 T0)
AddressSanitizer can not provide additional info.
    #0 0x6ee62290 in ash::DisplayController::GetNativeMouseCursorLocation()
const ???:0
    #1 0x6ee7ecfd in
ash::internal::DisplayManager::UpdateDisplays(std::vector<ash::internal::DisplayInfo,
std::allocator<ash::internal::DisplayInfo> > const&) ???:0
    #2 0x6ee840c6 in
ash::internal::DisplayManager::OnNativeDisplaysChanged(std::vector<ash::internal::DisplayInfo,
std::allocator<ash::internal::DisplayInfo> > const&) ???:0
    #3 0x6efdf07a in
ash::internal::DisplayChangeObserverX11::OnDisplayModeChanged() ???:0
    #4 0x6eeea3e7 in ash::Shell::Init() ???:0
    #5 0x6eee9ddc in ash::Shell::CreateInstance(ash::ShellDelegate*) ???:0
    #6 0x6e96dc72 in chrome::OpenAsh() ???:0
    #7 0x6e48872d in ChromeBrowserMainExtraPartsAsh::PreProfileInit() ???:0
    #8 0x71aece79 in ChromeBrowserMainParts::PreProfileInit() ???:0
    #9 0x71af1808 in ChromeBrowserMainPartsLinux::PreProfileInit() ???:0
    #10 0x70c7bdb4 in chromeos::ChromeBrowserMainPartsChromeos::PreProfileInit()
???:0
    #11 0x71ae67c2 in .LBB15_371 chrome/browser/chrome_browser_main.cc:0
    #12 0x71ae3bd7 in ChromeBrowserMainParts::PreMainMessageLoopRun() ???:0
    #13 0x6f1184d7 in .LBB12_65 content/browser/browser_main_loop.cc:0
    #14 0x6f11c908 in
content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&)
???:0
    #15 0x6f113ff3 in content::BrowserMain(content::MainFunctionParams const&)
???:0
    #16 0x66f8db32 in content::RunNamedProcessTypeMain(std::basic_string<char,
std::char_traits<char>, std::allocator<char> > const&,
content::MainFunctionParams const&, content::ContentMainDelegate*) ???:0
    #17 0x66f8f7bc in content::ContentMainRunnerImpl::Run() ???:0
    #18 0x66f8b892 in content::ContentMain(int, char const**,
content::ContentMainDelegate*) ???:0
    #19 0x65dcd3ac in ChromeMain ??:0
    #20 0x64bb9418 in ?? ??:0
    #21 0x65dcd1ac in _start ??:0
Stats: 2M malloced (3M for red zones) by 28480 calls
Stats: 0M realloced by 219 calls
Stats: 1M freed by 19757 calls
Stats: 0M really freed by 0 calls
Stats: 11M (2951 full pages) mmaped in 23 calls
  mmaps   by size class: 7:32760; 8:6141; 9:2046; 10:1022; 11:255; 12:256;
13:64; 14:32; 15:32; 16:8;
  mallocs by size class: 7:25403; 8:1782; 9:540; 10:427; 11:161; 12:115; 13:15;
14:12; 15:21; 16:4;
  frees   by size class: 7:17867; 8:1036; 9:338; 10:250; 11:137; 12:90; 13:12;
14:8; 15:18; 16:1;
  rfrees  by size class:
Stats: malloc large: 25 small slow: 239
==14367== ABORTING

[oshima, 2013-04-09 17:01:17]

Hmm, It looks simple NULL reference and I wonder why it wasn't caught by
other bots.
Anyway thank you for taking care of it and sorry for inconvenience.


On Tue, Apr 9, 2013 at 12:54 AM, <dcheng@chromium.org> wrote:

> On 2013/04/09 04:48:00, I haz the power (commit-bot) wrote:
>
>> Change committed as 193008
>>
>
> This appears to be causing issues on the ASAN bots. For example,
> http://build.chromium.org/p/**chromium.memory/builders/**
>
Chromium%20OS%20%28x86%29%**20ASAN/builds/3630<http://build.chromium.org/p/chromium.memory/builders/Chromium%20OS%20%28x86%29%20ASAN/builds/3630>
> :
> ==============================**==============================**=====
> ==14367== ERROR: AddressSanitizer: SEGV on unknown address 0x00000000 (pc
> 0x6ee62291 sp 0x7fdefc20 bp 0x7fdefcf8 T0)
> AddressSanitizer can not provide additional info.
>     #0 0x6ee62290 in ash::DisplayController::**
> GetNativeMouseCursorLocation()
> const ???:0
>     #1 0x6ee7ecfd in
> ash::internal::DisplayManager:**:UpdateDisplays(std::vector<**
> ash::internal::DisplayInfo,
> std::allocator<ash::internal::**DisplayInfo> > const&) ???:0
>     #2 0x6ee840c6 in
> ash::internal::DisplayManager:**:OnNativeDisplaysChanged(std::**
> vector<ash::internal::**DisplayInfo,
> std::allocator<ash::internal::**DisplayInfo> > const&) ???:0
>     #3 0x6efdf07a in
> ash::internal::**DisplayChangeObserverX11::**OnDisplayModeChanged() ???:0
>     #4 0x6eeea3e7 in ash::Shell::Init() ???:0
>     #5 0x6eee9ddc in ash::Shell::CreateInstance(**ash::ShellDelegate*)
> ???:0
>     #6 0x6e96dc72 in chrome::OpenAsh() ???:0
>     #7 0x6e48872d in ChromeBrowserMainExtraPartsAsh**::PreProfileInit()
> ???:0
>     #8 0x71aece79 in ChromeBrowserMainParts::**PreProfileInit() ???:0
>     #9 0x71af1808 in ChromeBrowserMainPartsLinux::**PreProfileInit() ???:0
>     #10 0x70c7bdb4 in chromeos::**ChromeBrowserMainPartsChromeos**
> ::PreProfileInit()
> ???:0
>     #11 0x71ae67c2 in .LBB15_371 chrome/browser/chrome_browser_**main.cc:0
>     #12 0x71ae3bd7 in ChromeBrowserMainParts::**PreMainMessageLoopRun()
> ???:0
>     #13 0x6f1184d7 in .LBB12_65 content/browser/browser_main_**loop.cc:0
>     #14 0x6f11c908 in
> content::**BrowserMainRunnerImpl::**Initialize(content::**MainFunctionParams
> const&)
> ???:0
>     #15 0x6f113ff3 in content::BrowserMain(content::**MainFunctionParams
> const&)
> ???:0
>     #16 0x66f8db32 in content::**RunNamedProcessTypeMain(std::**
> basic_string<char,
> std::char_traits<char>, std::allocator<char> > const&,
> content::MainFunctionParams const&, content::ContentMainDelegate*) ???:0
>     #17 0x66f8f7bc in content::**ContentMainRunnerImpl::Run() ???:0
>     #18 0x66f8b892 in content::ContentMain(int, char const**,
> content::ContentMainDelegate*) ???:0
>     #19 0x65dcd3ac in ChromeMain ??:0
>     #20 0x64bb9418 in ?? ??:0
>     #21 0x65dcd1ac in _start ??:0
> Stats: 2M malloced (3M for red zones) by 28480 calls
> Stats: 0M realloced by 219 calls
> Stats: 1M freed by 19757 calls
> Stats: 0M really freed by 0 calls
> Stats: 11M (2951 full pages) mmaped in 23 calls
>   mmaps   by size class: 7:32760; 8:6141; 9:2046; 10:1022; 11:255; 12:256;
> 13:64; 14:32; 15:32; 16:8;
>   mallocs by size class: 7:25403; 8:1782; 9:540; 10:427; 11:161; 12:115;
> 13:15;
> 14:12; 15:21; 16:4;
>   frees   by size class: 7:17867; 8:1036; 9:338; 10:250; 11:137; 12:90;
> 13:12;
> 14:8; 15:18; 16:1;
>   rfrees  by size class:
> Stats: malloc large: 25 small slow: 239
> ==14367== ABORTING
>
>
https://chromiumcodereview.**appspot.com/13466022/<https://chromiumcodereview...
>

[oshima, 2013-04-09 18:16:34]

mukai-san, I added bootstrap flag as display controller is no longer NULL during
bootstrap. (This change happened in separate CL in the past)
PTAL.

[Jun Mukai, 2013-04-09 18:28:43]

lgtm

[commit-bot: I haz the power, 2013-04-09 19:17:25]

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

[commit-bot: I haz the power, 2013-04-09 22:00:02]

Change committed as 193227