mash: remove more shell/shelf WmWindow usage.

mash: remove more shell/shelf WmWindow usage.

Eliminate Shell::GetWmRootWindowForNewWindows().
Convert ShellPort::Get*RootWindow*() to aura::Window.
(use Shell::GetPrimaryRootWindow() instead in many places)
Convert ScopedRootWindowForNewWindows (etc.) to aura::Window.
Convert WmShelf::ForWindow to take aura::Window.
Cleanup some helpers and misc WmWindow usage.

BUG=722551, 687656
TEST=Automated; no behavior changes
R=jamescook@chromium.org,xiyuan@chromium.org

Review-Url: https://codereview.chromium.org/2886253002
Cr-Commit-Position: refs/heads/master@{#472934}
Committed: https://chromium.googlesource.com/chromium/src/+/ad3d9551c524430d184c1d62a128576bcb2dd452

[msw, 2017-05-17 21:07:12]

Description was changed from

==========
mash: remove more shell/shelf WmWindow usage.

BUG=722551,687656
==========

to

==========
mash: remove more shell/shelf WmWindow usage.

Eliminate Shell::GetWmRootWindowForNewWindows().
Convert ShellPort::GetPrimaryRootWindow() to aura::Window.
(use Shell::GetPrimaryRootWindow() instead in many places)
Convert ScopedRootWindowForNewWindows (etc.) to aura::Window.
Convert WmShelf::ForWindow to take aura::Window.
Cleanup some helpers and misc WmWindow usage.

BUG=722551,687656
TEST=Automated; no behavior changes
R=jamescook@chromium.org
==========

[msw, 2017-05-17 21:07:12]

msw@chromium.org changed reviewers:
+ jamescook@chromium.org

[msw, 2017-05-17 21:07:12]

Description was changed from

==========
mash: remove more shell/shelf WmWindow usage.

BUG=722551,687656
==========

to

==========
mash: remove more shell/shelf WmWindow usage.

Eliminate Shell::GetWmRootWindowForNewWindows().
Convert ShellPort::GetPrimaryRootWindow() to aura::Window.
(use Shell::GetPrimaryRootWindow() instead in many places)
Convert ScopedRootWindowForNewWindows (etc.) to aura::Window.
Convert WmShelf::ForWindow to take aura::Window.
Cleanup some helpers and misc WmWindow usage.

BUG=722551,687656
TEST=Automated; no behavior changes
R=jamescook@chromium.org
==========

[msw, 2017-05-17 21:07:13]

msw@chromium.org changed reviewers:
+ jamescook@chromium.org

[msw, 2017-05-17 21:07:37]

Hey James, please take a look; thanks! +CC Scott for FYI.

[msw, 2017-05-17 21:07:43]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-17 21:08:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-17 21:15:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-17 21:15:39]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[msw, 2017-05-17 21:22:00]

The CQ bit was checked by msw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-17 21:23:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2017-05-17 23:41:49]

LGTM with nits.

fyi, varkha has a cl in-flight that touches ash/wm,
https://codereview.chromium.org/2893643003/

https://codereview.chromium.org/2886253002/diff/60001/ash/app_list/app_list_d...
File ash/app_list/app_list_delegate_impl.cc (right):

https://codereview.chromium.org/2886253002/diff/60001/ash/app_list/app_list_d...
ash/app_list/app_list_delegate_impl.cc:29:
ShellPort::Get()->GetRootWindowForDisplayId(display_id)->aura_window();
optional: Do you think it makes sense to change
ShellPort::GetRootWindowForDisplayId() to return aura::Window* in this CL? It
seems like you're touching a lot of the call sites already.

https://codereview.chromium.org/2886253002/diff/60001/ash/first_run/first_run...
File ash/first_run/first_run_helper_impl.cc (right):

https://codereview.chromium.org/2886253002/diff/60001/ash/first_run/first_run...
ash/first_run/first_run_helper_impl.cc:12: #include "ash/shell_port.h"
still needed?

https://codereview.chromium.org/2886253002/diff/60001/ash/metrics/user_metric...
File ash/metrics/user_metrics_recorder.cc (right):

https://codereview.chromium.org/2886253002/diff/60001/ash/metrics/user_metric...
ash/metrics/user_metrics_recorder.cc:17: #include "ash/shell_port.h"
still needed?

(you might want to audit other files too, just noticed this)

https://codereview.chromium.org/2886253002/diff/60001/ash/shelf/shelf_widget_...
File ash/shelf/shelf_widget_unittest.cc (right):

https://codereview.chromium.org/2886253002/diff/60001/ash/shelf/shelf_widget_...
ash/shelf/shelf_widget_unittest.cc:47: ->GetDisplayNearestWindow(root)
Aside: I really miss window->GetDisplayNearestWindow(). I wonder if we should
add a GetDisplay() helper to RootWindowController() for cases like this.

https://codereview.chromium.org/2886253002/diff/60001/ash/shell.h
File ash/shell.h (left):

https://codereview.chromium.org/2886253002/diff/60001/ash/shell.h#oldcode239
ash/shell.h:239: static WmWindow* GetWmRootWindowForNewWindows();
Hooray!

[commit-bot: I haz the power, 2017-05-17 23:56:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-17 23:56:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sky, 2017-05-18 00:09:25]

Thanks Mike! I'm glad to see others chipping in.

On Wed, May 17, 2017 at 2:07 PM, <msw@chromium.org> wrote:

> Reviewers: James Cook
> CL: https://codereview.chromium.org/2886253002/
>
> Message:
> Hey James, please take a look; thanks! +CC Scott for FYI.
>
> Description:
> mash: remove more shell/shelf WmWindow usage.
>
> Eliminate Shell::GetWmRootWindowForNewWindows().
> Convert ShellPort::GetPrimaryRootWindow() to aura::Window.
> (use Shell::GetPrimaryRootWindow() instead in many places)
> Convert ScopedRootWindowForNewWindows (etc.) to aura::Window.
> Convert WmShelf::ForWindow to take aura::Window.
> Cleanup some helpers and misc WmWindow usage.
>
> BUG=722551,687656
> TEST=Automated; no behavior changes
> R=jamescook@chromium.org
>
> Affected files (+207, -249 lines):
> M ash/accelerators/accelerator_controller.cc
> M ash/accelerators/exit_warning_handler.cc
> M ash/app_list/app_list_delegate_impl.cc
> M ash/app_list/app_list_presenter_delegate.cc
> M ash/app_list/app_list_presenter_delegate_unittest.cc
> M ash/aura/shell_port_classic.h
> M ash/aura/shell_port_classic.cc
> M ash/drag_drop/drag_drop_tracker_unittest.cc
> M ash/first_run/first_run_helper_impl.cc
> M ash/metrics/user_metrics_recorder.cc
> M ash/mus/accelerators/accelerator_controller_registrar.cc
> M ash/mus/bridge/shell_port_mash.h
> M ash/mus/bridge/shell_port_mash.cc
> M ash/mus/top_level_window_factory.cc
> M ash/mus/window_manager.h
> M ash/mus/window_manager.cc
> M ash/root_window_controller.cc
> M ash/root_window_controller_unittest.cc
> M ash/scoped_root_window_for_new_windows.h
> M ash/scoped_root_window_for_new_windows.cc
> M ash/shelf/shelf_view.cc
> M ash/shelf/shelf_view_unittest.cc
> M ash/shelf/shelf_widget_unittest.cc
> M ash/shelf/shelf_window_watcher_unittest.cc
> M ash/shelf/wm_shelf.h
> M ash/shelf/wm_shelf.cc
> M ash/shell.h
> M ash/shell.cc
> M ash/shell_port.h
> M ash/shell_port.cc
> M ash/shell_unittest.cc
> M ash/system/palette/palette_utils.cc
> M ash/system/session/logout_confirmation_dialog.cc
> M ash/system/toast/toast_overlay.cc
> M ash/system/tray/system_tray_controller.cc
> M ash/test/ash_test_base.cc
> M ash/wm/immersive_context_ash.cc
> M ash/wm/mru_window_tracker.cc
> M ash/wm/overview/window_grid.cc
> M ash/wm/overview/window_selector.cc
> M ash/wm/system_modal_container_layout_manager_unittest.cc
> M ash/wm/window_animations.cc
> M ash/wm/window_cycle_controller_unittest.cc
> M ash/wm/window_cycle_list.cc
> M ash/wm/window_positioner.cc
> M ash/wm/window_positioner_unittest.cc
> M ash/wm/window_positioning_utils.cc
> M ash/wm/window_util.cc
> M ash/wm/workspace/workspace_layout_manager_keyboard_unittest.cc
> M ash/wm/workspace_controller.cc
> M chrome/browser/chromeos/accessibility/accessibility_manager.cc
> M chrome/browser/chromeos/accessibility/chromevox_panel.cc
> M chrome/browser/chromeos/first_run/steps/tray_step.cc
> M chrome/browser/chromeos/login/login_browsertest.cc
> M chrome/browser/ui/ash/app_list/app_list_interactive_uitest.cc
> M chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
> M chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
> M chrome/browser/ui/ash/multi_user/user_switch_animator_chromeos.cc
> M chrome/browser/ui/ash/networking_config_delegate_chromeos_browsertest.cc
> M chrome/browser/ui/ash/shelf_browsertest.cc
> M chrome/browser/ui/ash/system_tray_client_browsertest.cc
> M chrome/browser/ui/webui/chromeos/first_run/first_run_ui.cc
> M chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc
> M chrome/browser/ui/window_sizer/window_sizer_ash_unittest.cc
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[msw, 2017-05-18 20:27:37]

The CQ bit was checked by msw@chromium.org

[msw, 2017-05-18 20:27:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2886253002/#ps120001
(title: "Fix WmShelf::ForWindow.")

[msw, 2017-05-18 20:28:01]

Done landing (so long as tests are green).

https://codereview.chromium.org/2886253002/diff/60001/ash/app_list/app_list_d...
File ash/app_list/app_list_delegate_impl.cc (right):

https://codereview.chromium.org/2886253002/diff/60001/ash/app_list/app_list_d...
ash/app_list/app_list_delegate_impl.cc:29:
ShellPort::Get()->GetRootWindowForDisplayId(display_id)->aura_window();
On 2017/05/17 23:41:49, James Cook wrote:
> optional: Do you think it makes sense to change
> ShellPort::GetRootWindowForDisplayId() to return aura::Window* in this CL? It
> seems like you're touching a lot of the call sites already.

Done.

https://codereview.chromium.org/2886253002/diff/60001/ash/first_run/first_run...
File ash/first_run/first_run_helper_impl.cc (right):

https://codereview.chromium.org/2886253002/diff/60001/ash/first_run/first_run...
ash/first_run/first_run_helper_impl.cc:12: #include "ash/shell_port.h"
On 2017/05/17 23:41:49, James Cook wrote:
> still needed?

Done.

https://codereview.chromium.org/2886253002/diff/60001/ash/metrics/user_metric...
File ash/metrics/user_metrics_recorder.cc (right):

https://codereview.chromium.org/2886253002/diff/60001/ash/metrics/user_metric...
ash/metrics/user_metrics_recorder.cc:17: #include "ash/shell_port.h"
On 2017/05/17 23:41:49, James Cook wrote:
> still needed?
> 
> (you might want to audit other files too, just noticed this)

Done, and audit found one more I removed.

https://codereview.chromium.org/2886253002/diff/60001/ash/shelf/shelf_widget_...
File ash/shelf/shelf_widget_unittest.cc (right):

https://codereview.chromium.org/2886253002/diff/60001/ash/shelf/shelf_widget_...
ash/shelf/shelf_widget_unittest.cc:47: ->GetDisplayNearestWindow(root)
On 2017/05/17 23:41:49, James Cook wrote:
> Aside: I really miss window->GetDisplayNearestWindow(). I wonder if we should
> add a GetDisplay() helper to RootWindowController() for cases like this.

Ditto, I think this is worth separate consideration/followup.

https://codereview.chromium.org/2886253002/diff/60001/ash/shell.h
File ash/shell.h (left):

https://codereview.chromium.org/2886253002/diff/60001/ash/shell.h#oldcode239
ash/shell.h:239: static WmWindow* GetWmRootWindowForNewWindows();
On 2017/05/17 23:41:49, James Cook wrote:
> Hooray!

Acknowledged.

[commit-bot: I haz the power, 2017-05-18 20:29:24]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 20:45:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-18 20:45:43]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[msw, 2017-05-18 20:49:52]

Description was changed from

==========
mash: remove more shell/shelf WmWindow usage.

Eliminate Shell::GetWmRootWindowForNewWindows().
Convert ShellPort::GetPrimaryRootWindow() to aura::Window.
(use Shell::GetPrimaryRootWindow() instead in many places)
Convert ScopedRootWindowForNewWindows (etc.) to aura::Window.
Convert WmShelf::ForWindow to take aura::Window.
Cleanup some helpers and misc WmWindow usage.

BUG=722551,687656
TEST=Automated; no behavior changes
R=jamescook@chromium.org
==========

to

==========
mash: remove more shell/shelf WmWindow usage.

Eliminate Shell::GetWmRootWindowForNewWindows().
Convert ShellPort::Get*RootWindow*() to aura::Window.
(use Shell::GetPrimaryRootWindow() instead in many places)
Convert ScopedRootWindowForNewWindows (etc.) to aura::Window.
Convert WmShelf::ForWindow to take aura::Window.
Cleanup some helpers and misc WmWindow usage.

BUG=722551,687656
TEST=Automated; no behavior changes
R=jamescook@chromium.org,xiyuan@chromium.org
==========

[msw, 2017-05-18 20:49:52]

msw@chromium.org changed reviewers:
+ xiyuan@chromium.org

[msw, 2017-05-18 20:50:14]

Xiyuan, please take a look at chrome/browser/chromeos/*.

[xiyuan, 2017-05-18 21:02:09]

chromeos lgtm

[msw, 2017-05-18 21:14:21]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2017-05-18 21:15:58]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 21:23:39]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1495142061502480,
"parent_rev": "f88f6d0e6225a933e189cf4c14f51084d889c6f1", "commit_rev":
"ad3d9551c524430d184c1d62a128576bcb2dd452"}

[commit-bot: I haz the power, 2017-05-18 21:23:51]

Description was changed from

==========
mash: remove more shell/shelf WmWindow usage.

Eliminate Shell::GetWmRootWindowForNewWindows().
Convert ShellPort::Get*RootWindow*() to aura::Window.
(use Shell::GetPrimaryRootWindow() instead in many places)
Convert ScopedRootWindowForNewWindows (etc.) to aura::Window.
Convert WmShelf::ForWindow to take aura::Window.
Cleanup some helpers and misc WmWindow usage.

BUG=722551,687656
TEST=Automated; no behavior changes
R=jamescook@chromium.org,xiyuan@chromium.org
==========

to

==========
mash: remove more shell/shelf WmWindow usage.

Eliminate Shell::GetWmRootWindowForNewWindows().
Convert ShellPort::Get*RootWindow*() to aura::Window.
(use Shell::GetPrimaryRootWindow() instead in many places)
Convert ScopedRootWindowForNewWindows (etc.) to aura::Window.
Convert WmShelf::ForWindow to take aura::Window.
Cleanup some helpers and misc WmWindow usage.

BUG=722551,687656
TEST=Automated; no behavior changes
R=jamescook@chromium.org,xiyuan@chromium.org

Review-Url: https://codereview.chromium.org/2886253002
Cr-Commit-Position: refs/heads/master@{#472934}
Committed:
https://chromium.googlesource.com/chromium/src/+/ad3d9551c524430d184c1d62a128...
==========

[commit-bot: I haz the power, 2017-05-18 21:23:53]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/ad3d9551c524430d184c1d62a128...