New issue
Advanced search Search tips

Issue 699231 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 671246



Sign in to add a comment

Replace usage of WmActivationObserver with aura::client::ActivationChangeObserver

Project Member Reported by sky@chromium.org, Mar 7 2017

Issue description

The two are roughly the same, the former was added when we couldn't use aura. Now that mash uses aura we can remove WmActivationObserver. This means class that subclasses WmActivationObserver should be converted to subclass aura::client::ActivationChangeObserver and registration should move to Shell::GetInstance()->activation_client()->AddObserver().

Once this is done, WmShell::AddActivationObserver/RemoveActivationObserver can be removed.
 

Comment 1 by sky@chromium.org, Mar 7 2017

Owner: sky@chromium.org
Status: Started (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/589a5bec0318f116d4f269facc6d9b6b79fda9c1

commit 589a5bec0318f116d4f269facc6d9b6b79fda9c1
Author: sky <sky@chromium.org>
Date: Wed Mar 08 05:53:30 2017

Converts WmActivationObserver to aura::client::ActivationChangeObserver

I had to make Shell add WmShell as the ActivationChangeObserver
because of ordering issues. This is ugly, but my next patch will move
this to Shell, so it's temporary.

BUG= 699231 
TEST=covered by tests
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2736753005
Cr-Commit-Position: refs/heads/master@{#455386}

[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/BUILD.gn
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/aura/wm_shell_aura.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/shelf/shelf_layout_manager.h
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/system/tray/system_tray.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/dock/docked_window_layout_manager.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/dock/docked_window_layout_manager.h
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/mru_window_tracker.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/mru_window_tracker.h
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/overview/window_selector.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/overview/window_selector.h
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/panels/panel_layout_manager.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/panels/panel_layout_manager.h
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/workspace/workspace_layout_manager.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm/workspace/workspace_layout_manager.h
[delete] https://crrev.com/d265114438e787fbcefdbe0e081491349a5816b4/ash/common/wm_activation_observer.h
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm_shell.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/common/wm_shell.h
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/display/screen_orientation_controller_chromeos.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/display/screen_orientation_controller_chromeos.h
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/ash/shell.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/chrome/browser/chromeos/arc/boot_phase_monitor/arc_instance_throttle.cc
[modify] https://crrev.com/589a5bec0318f116d4f269facc6d9b6b79fda9c1/chrome/browser/chromeos/arc/boot_phase_monitor/arc_instance_throttle.h

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2734438cde42ce98227c4292b35a092fb75ee371

commit 2734438cde42ce98227c4292b35a092fb75ee371
Author: sky <sky@chromium.org>
Date: Wed Mar 08 21:30:32 2017

Moves maintaining root_window_for_new_windows_ to Shell

There is no reason for this to be on WmShell anymore. This also
includes the following:
. Renames Shell::GetTargetRootWindow() to
  GetRootWindowForNewWindows(). This name better indicates what the
  function is for.
. Added version of the above that returns WmWindow. This will be
  removed once WmWindow is removed.
. Removes Shell::GetTargetDisplayId() as it was only used in one place
  (and it's name is rather confusing).
. Makes FocusController's implementation of ActivationClient and
  FocusClient public. FocusController's is intended to be the
  ActivationClient and the FocusClient, so making it protected just
  leads to awkwardness in using the class.

BUG= 699231 
TEST=covered by tests

Review-Url: https://codereview.chromium.org/2739553005
Cr-Commit-Position: refs/heads/master@{#455546}

[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/accelerators/accelerator_controller.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/accelerators/exit_warning_handler.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/scoped_root_window_for_new_windows.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/system/toast/toast_overlay.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/wm/container_finder.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/wm/mru_window_tracker.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/wm/window_cycle_list.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/wm/window_positioner.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/wm/window_positioning_utils.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/wm_shell.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/common/wm_shell.h
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/drag_drop/drag_drop_tracker_unittest.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/extended_desktop_unittest.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/magnifier/magnification_controller.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/mus/accelerators/accelerator_controller_registrar.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/mus/top_level_window_factory.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/root_window_controller.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/root_window_controller.h
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/shell.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/shell.h
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/shell/panel_window.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/sticky_keys/sticky_keys_overlay.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/test/ash_test_helper.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/test/ui_controls_factory_ash.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/wm/ash_focus_rules.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ash/wm/drag_window_resizer_unittest.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/chromeos/input_method/candidate_window_controller_impl.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/chromeos/profiles/multiprofiles_intro_dialog.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/chromeos/profiles/multiprofiles_session_aborted_dialog.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/chromeos/ui/idle_app_name_notification_view.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/chromeos/ui/kiosk_external_update_notification.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/ui/ash/app_list/app_list_service_ash.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/ui/ash/chrome_keyboard_ui.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/ui/ash/multi_user/multi_user_warning_dialog.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/ui/window_sizer/window_sizer.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/chrome/browser/ui/window_sizer/window_sizer_ash_unittest.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/components/exo/wm_helper_ash.cc
[modify] https://crrev.com/2734438cde42ce98227c4292b35a092fb75ee371/ui/wm/core/focus_controller.h

Comment 4 by sky@chromium.org, Mar 8 2017

Status: Fixed (was: Started)

Comment 5 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 6 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment