From e5bd68e150c9d2b46187d2d1a7d948f709760d34 Mon Sep 17 00:00:00 2001 From: "gbillock@chromium.org" Date: Fri, 29 Mar 2013 18:12:51 +0000 Subject: Revert 191376 "Propegate setting autohide behaviour to prefs" > Propegate setting autohide behaviour to prefs > > When the ShelfLayoutManager changes the autohide behaviour this needs to be > propegated to the ChromeLauncherController otherwise when other parts of the > system query the autohide status they can end up with an incorrect value. This > resolves most of the outstanding issues with the "Autohide Launcher" checkbox in > the context menu. > > These is still one issue, specifically if you set autohide off, swipe the > launcher off the screen and then make it reappear, via either swiping or mouse > over. The checkbox will not be checked in this case, but the moment you touch or > click off of the launcher it will hide. After hiding the checkbox will be set as > expected, thus partially resolving that fact that you had to turn on autohide > before you could turn it off. This issue due to the fact that the > ShelfLayoutManager is actually turning off autohide, but setting a handler to > watch for future events and when you interact outside of the launcher setting > autohide on again. Resolving this is outside the scope of this CL/bug since it > either requires changing our UI behjaviour or plumbing information about being > in this state to the LaunchContextMenu, both of which would be unlikely to be > backported to 27. > > BUG=chromium:173295 > TEST=Visually confirmed that the checkbox is repersenative of the state of > autohide or is only transistorially incorrect. > > Review URL: https://chromiumcodereview.appspot.com/12636012 TBR=rharrison@chromium.org Review URL: https://codereview.chromium.org/13322004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191399 0039d316-1c4b-4281-b951-d872f2087c98 --- ash/launcher/launcher.cc | 2 -- ash/launcher/launcher_delegate.h | 9 ------ ash/shelf/shelf_layout_manager.cc | 2 -- ash/shelf/shelf_layout_manager.h | 7 ----- ash/shell/launcher_delegate_impl.cc | 6 ---- ash/shell/launcher_delegate_impl.h | 2 -- ash/test/test_launcher_delegate.cc | 6 ---- ash/test/test_launcher_delegate.h | 2 -- .../launcher/chrome_launcher_controller_per_app.cc | 34 --------------------- .../launcher/chrome_launcher_controller_per_app.h | 31 ++++++------------- .../chrome_launcher_controller_per_browser.cc | 35 ---------------------- .../chrome_launcher_controller_per_browser.h | 13 +------- 12 files changed, 10 insertions(+), 139 deletions(-) diff --git a/ash/launcher/launcher.cc b/ash/launcher/launcher.cc index de7daf0..c291cb9 100644 --- a/ash/launcher/launcher.cc +++ b/ash/launcher/launcher.cc @@ -51,11 +51,9 @@ Launcher::Launcher(LauncherModel* launcher_model, shelf_widget_->GetNativeView()->SetName("LauncherView"); shelf_widget_->GetNativeView()->SetProperty( internal::kStayInSameRootWindowKey, true); - delegate_->OnLauncherCreated(this); } Launcher::~Launcher() { - delegate_->OnLauncherDestroyed(this); } // static diff --git a/ash/launcher/launcher_delegate.h b/ash/launcher/launcher_delegate.h index bcde60f..5ca045e 100644 --- a/ash/launcher/launcher_delegate.h +++ b/ash/launcher/launcher_delegate.h @@ -19,7 +19,6 @@ class Event; } namespace ash { -class Launcher; // A special menu model which keeps track of an "active" menu item. class ASH_EXPORT LauncherMenuModel : public ui::SimpleMenuModel { @@ -88,14 +87,6 @@ class ASH_EXPORT LauncherDelegate { // Returns true if a tooltip should be shown for the item. virtual bool ShouldShowTooltip(const LauncherItem& item) = 0; - - // Callback used to allow delegate to perform initialization actions that - // depend on the Launcher being in a known state. - virtual void OnLauncherCreated(Launcher* launcher) = 0; - - // Callback used to inform the delegate that a specific launcher no longer - // exists. - virtual void OnLauncherDestroyed(Launcher* launcher) = 0; }; } // namespace ash diff --git a/ash/shelf/shelf_layout_manager.cc b/ash/shelf/shelf_layout_manager.cc index b38b803d..305283f 100644 --- a/ash/shelf/shelf_layout_manager.cc +++ b/ash/shelf/shelf_layout_manager.cc @@ -180,8 +180,6 @@ void ShelfLayoutManager::SetAutoHideBehavior(ShelfAutoHideBehavior behavior) { UpdateVisibilityState(); FOR_EACH_OBSERVER(Observer, observers_, OnAutoHideStateChanged(state_.auto_hide_state)); - FOR_EACH_OBSERVER(Observer, observers_, - OnAutoHideBehaviorChanged(auto_hide_behavior_)); } bool ShelfLayoutManager::IsVisible() const { diff --git a/ash/shelf/shelf_layout_manager.h b/ash/shelf/shelf_layout_manager.h index fa16158..40e59a7 100644 --- a/ash/shelf/shelf_layout_manager.h +++ b/ash/shelf/shelf_layout_manager.h @@ -49,9 +49,6 @@ class ASH_EXPORT ShelfLayoutManager : public ash::ShellObserver, public aura::client::ActivationChangeObserver { public: - - // TODO(rharrison): Move this observer out of ash::internal:: - // namespace. Tracked in crosbug.com/223936 class ASH_EXPORT Observer { public: // Called when the target ShelfLayoutManager will be deleted. @@ -62,10 +59,6 @@ class ASH_EXPORT ShelfLayoutManager : // Called when the auto hide state is changed. virtual void OnAutoHideStateChanged(ShelfAutoHideState new_state) {} - - // Called when the auto hide behavior is changed. - virtual void OnAutoHideBehaviorChanged( - ShelfAutoHideBehavior new_behavior) {} }; // We reserve a small area at the bottom of the workspace area to ensure that diff --git a/ash/shell/launcher_delegate_impl.cc b/ash/shell/launcher_delegate_impl.cc index ec97cd7..fd52c34 100644 --- a/ash/shell/launcher_delegate_impl.cc +++ b/ash/shell/launcher_delegate_impl.cc @@ -69,11 +69,5 @@ bool LauncherDelegateImpl::ShouldShowTooltip(const ash::LauncherItem& item) { return true; } -void LauncherDelegateImpl::OnLauncherCreated(Launcher* launcher) { -} - -void LauncherDelegateImpl::OnLauncherDestroyed(Launcher* launcher) { -} - } // namespace shell } // namespace ash diff --git a/ash/shell/launcher_delegate_impl.h b/ash/shell/launcher_delegate_impl.h index 79c6f59..edf58e8 100644 --- a/ash/shell/launcher_delegate_impl.h +++ b/ash/shell/launcher_delegate_impl.h @@ -39,8 +39,6 @@ class LauncherDelegateImpl : public ash::LauncherDelegate { virtual ash::LauncherID GetIDByWindow(aura::Window* window) OVERRIDE; virtual bool IsDraggable(const ash::LauncherItem& item) OVERRIDE; virtual bool ShouldShowTooltip(const LauncherItem& item) OVERRIDE; - virtual void OnLauncherCreated(Launcher* launcher) OVERRIDE; - virtual void OnLauncherDestroyed(Launcher* launcher) OVERRIDE; private: // Used to update Launcher. Owned by main. diff --git a/ash/test/test_launcher_delegate.cc b/ash/test/test_launcher_delegate.cc index d9ea76b..13ab706 100644 --- a/ash/test/test_launcher_delegate.cc +++ b/ash/test/test_launcher_delegate.cc @@ -120,11 +120,5 @@ bool TestLauncherDelegate::ShouldShowTooltip(const ash::LauncherItem& item) { return true; } -void TestLauncherDelegate::OnLauncherCreated(Launcher* launcher) { -} - -void TestLauncherDelegate::OnLauncherDestroyed(Launcher* launcher) { -} - } // namespace test } // namespace ash diff --git a/ash/test/test_launcher_delegate.h b/ash/test/test_launcher_delegate.h index ccbca32..f249bee 100644 --- a/ash/test/test_launcher_delegate.h +++ b/ash/test/test_launcher_delegate.h @@ -48,8 +48,6 @@ class TestLauncherDelegate : public LauncherDelegate, virtual ash::LauncherID GetIDByWindow(aura::Window* window) OVERRIDE; virtual bool IsDraggable(const ash::LauncherItem& item) OVERRIDE; virtual bool ShouldShowTooltip(const LauncherItem& item) OVERRIDE; - virtual void OnLauncherCreated(Launcher* launcher) OVERRIDE; - virtual void OnLauncherDestroyed(Launcher* launcher) OVERRIDE; private: typedef std::map WindowToID; diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc index eda32cf..ceb0858 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc @@ -9,8 +9,6 @@ #include "ash/ash_switches.h" #include "ash/launcher/launcher_model.h" #include "ash/launcher/launcher_util.h" -#include "ash/root_window_controller.h" -#include "ash/shelf/shelf_widget.h" #include "ash/shell.h" #include "ash/wm/window_util.h" #include "base/command_line.h" @@ -236,11 +234,6 @@ ChromeLauncherControllerPerApp::~ChromeLauncherControllerPerApp() { // Reset the shell window controller here since it has a weak pointer to this. shell_window_controller_.reset(); - for (std::set::iterator iter = launchers_.begin(); - iter != launchers_.end(); - ++iter) - (*iter)->shelf_widget()->shelf_layout_manager()->RemoveObserver(this); - model_->RemoveObserver(this); BrowserList::RemoveObserver(this); if (ash::Shell::HasInstance()) @@ -604,21 +597,6 @@ void ChromeLauncherControllerPerApp::SetAppImage( } } -void ChromeLauncherControllerPerApp::OnAutoHideBehaviorChanged( - ash::ShelfAutoHideBehavior new_behavior) { - ash::Shell::RootWindowList root_windows; - if (ash::Shell::IsLauncherPerDisplayEnabled()) - root_windows = ash::Shell::GetAllRootWindows(); - else - root_windows.push_back(ash::Shell::GetPrimaryRootWindow()); - - for (ash::Shell::RootWindowList::const_iterator iter = - root_windows.begin(); - iter != root_windows.end(); ++iter) { - SetShelfAutoHideBehaviorPrefs(new_behavior, *iter); - } -} - void ChromeLauncherControllerPerApp::SetLauncherItemImage( ash::LauncherID launcher_id, const gfx::ImageSkia& image) { @@ -957,18 +935,6 @@ bool ChromeLauncherControllerPerApp::ShouldShowTooltip( return true; } -void ChromeLauncherControllerPerApp::OnLauncherCreated( - ash::Launcher* launcher) { - launchers_.insert(launcher); - launcher->shelf_widget()->shelf_layout_manager()->AddObserver(this); -} - -void ChromeLauncherControllerPerApp::OnLauncherDestroyed( - ash::Launcher* launcher) { - launchers_.erase(launcher); - launcher->shelf_widget()->shelf_layout_manager()->RemoveObserver(this); -} - void ChromeLauncherControllerPerApp::LauncherItemAdded(int index) { } diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h index f5a1338..7c688e2 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h @@ -7,14 +7,12 @@ #include #include -#include #include #include #include "ash/display/display_controller.h" #include "ash/launcher/launcher_model_observer.h" #include "ash/launcher/launcher_types.h" -#include "ash/shelf/shelf_layout_manager.h" #include "ash/shelf/shelf_types.h" #include "ash/shell_observer.h" #include "base/basictypes.h" @@ -63,17 +61,15 @@ class WebContents; // * App shell windows have ShellWindowLauncherItemController, owned by // ShellWindowLauncherController. // * Shortcuts have no LauncherItemController. -class ChromeLauncherControllerPerApp - : public ash::LauncherModelObserver, - public ash::ShellObserver, - public ash::DisplayController::Observer, - public ChromeLauncherController, - public content::NotificationObserver, - public PrefServiceSyncableObserver, - public AppSyncUIStateObserver, - public ExtensionEnableFlowDelegate, - public chrome::BrowserListObserver, - public ash::internal::ShelfLayoutManager::Observer { +class ChromeLauncherControllerPerApp : public ash::LauncherModelObserver, + public ash::ShellObserver, + public ash::DisplayController::Observer, + public ChromeLauncherController, + public content::NotificationObserver, + public PrefServiceSyncableObserver, + public AppSyncUIStateObserver, + public ExtensionEnableFlowDelegate, + public chrome::BrowserListObserver { public: ChromeLauncherControllerPerApp(Profile* profile, ash::LauncherModel* model); virtual ~ChromeLauncherControllerPerApp(); @@ -266,8 +262,6 @@ class ChromeLauncherControllerPerApp virtual ash::LauncherID GetIDByWindow(aura::Window* window) OVERRIDE; virtual bool IsDraggable(const ash::LauncherItem& item) OVERRIDE; virtual bool ShouldShowTooltip(const ash::LauncherItem& item) OVERRIDE; - virtual void OnLauncherCreated(ash::Launcher* launcher) OVERRIDE; - virtual void OnLauncherDestroyed(ash::Launcher* launcher) OVERRIDE; // ash::LauncherModelObserver overrides: virtual void LauncherItemAdded(int index) OVERRIDE; @@ -303,10 +297,6 @@ class ChromeLauncherControllerPerApp virtual void SetAppImage(const std::string& app_id, const gfx::ImageSkia& image) OVERRIDE; - // ash::internal::ShelfLayoutManager::Observer overrides: - virtual void OnAutoHideBehaviorChanged( - ash::ShelfAutoHideBehavior new_behavior) OVERRIDE; - // Get the list of all running incarnations of this item. // |event_flags| specifies the flags which were set by the event which // triggered this menu generation. It can be used to generate different lists. @@ -474,9 +464,6 @@ class ChromeLauncherControllerPerApp scoped_ptr extension_enable_flow_; - // Launchers that are currently being observed. - std::set launchers_; - DISALLOW_COPY_AND_ASSIGN(ChromeLauncherControllerPerApp); }; diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.cc index 34fa3b2..78c7d14 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.cc +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.cc @@ -7,8 +7,6 @@ #include #include "ash/launcher/launcher_model.h" -#include "ash/root_window_controller.h" -#include "ash/shelf/shelf_widget.h" #include "ash/shell.h" #include "ash/wm/window_util.h" #include "base/command_line.h" @@ -301,11 +299,6 @@ ChromeLauncherControllerPerBrowser::~ChromeLauncherControllerPerBrowser() { // this. shell_window_controller_.reset(); - for (std::set::iterator iter = launchers_.begin(); - iter != launchers_.end(); - ++iter) - (*iter)->shelf_widget()->shelf_layout_manager()->RemoveObserver(this); - model_->RemoveObserver(this); if (ash::Shell::HasInstance()) ash::Shell::GetInstance()->display_controller()->RemoveObserver(this); @@ -644,22 +637,6 @@ void ChromeLauncherControllerPerBrowser::SetAppImage( } } -void ChromeLauncherControllerPerBrowser::OnAutoHideBehaviorChanged( - ash::ShelfAutoHideBehavior new_behavior) { - std::string behavior_string; - ash::Shell::RootWindowList root_windows; - if (ash::Shell::IsLauncherPerDisplayEnabled()) - root_windows = ash::Shell::GetAllRootWindows(); - else - root_windows.push_back(ash::Shell::GetPrimaryRootWindow()); - - for (ash::Shell::RootWindowList::const_iterator iter = - root_windows.begin(); - iter != root_windows.end(); ++iter) { - SetShelfAutoHideBehaviorPrefs(new_behavior, *iter); - } -} - void ChromeLauncherControllerPerBrowser::SetLauncherItemImage( ash::LauncherID launcher_id, const gfx::ImageSkia& image) { @@ -935,18 +912,6 @@ bool ChromeLauncherControllerPerBrowser::ShouldShowTooltip( return true; } -void ChromeLauncherControllerPerBrowser::OnLauncherCreated( - ash::Launcher* launcher) { - launchers_.insert(launcher); - launcher->shelf_widget()->shelf_layout_manager()->AddObserver(this); -} - -void ChromeLauncherControllerPerBrowser::OnLauncherDestroyed( - ash::Launcher* launcher) { - launchers_.erase(launcher); - launcher->shelf_widget()->shelf_layout_manager()->RemoveObserver(this); -} - void ChromeLauncherControllerPerBrowser::LauncherItemAdded(int index) { } diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.h b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.h index a88b98f..b9ef547 100644 --- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.h +++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_browser.h @@ -13,7 +13,6 @@ #include "ash/launcher/launcher_delegate.h" #include "ash/launcher/launcher_model_observer.h" #include "ash/launcher/launcher_types.h" -#include "ash/shelf/shelf_layout_manager.h" #include "ash/shelf/shelf_types.h" #include "ash/shell_observer.h" #include "base/basictypes.h" @@ -66,8 +65,7 @@ class ChromeLauncherControllerPerBrowser public content::NotificationObserver, public PrefServiceSyncableObserver, public AppSyncUIStateObserver, - public ExtensionEnableFlowDelegate, - public ash::internal::ShelfLayoutManager::Observer { + public ExtensionEnableFlowDelegate { public: ChromeLauncherControllerPerBrowser(Profile* profile, ash::LauncherModel* model); @@ -255,8 +253,6 @@ class ChromeLauncherControllerPerBrowser virtual ash::LauncherID GetIDByWindow(aura::Window* window) OVERRIDE; virtual bool IsDraggable(const ash::LauncherItem& item) OVERRIDE; virtual bool ShouldShowTooltip(const ash::LauncherItem& item) OVERRIDE; - virtual void OnLauncherCreated(ash::Launcher* launcher) OVERRIDE; - virtual void OnLauncherDestroyed(ash::Launcher* launcher) OVERRIDE; // ash::LauncherModelObserver overrides: virtual void LauncherItemAdded(int index) OVERRIDE; @@ -292,10 +288,6 @@ class ChromeLauncherControllerPerBrowser virtual void SetAppImage(const std::string& app_id, const gfx::ImageSkia& image) OVERRIDE; - // ash::internal::ShelfLayoutManager::Observer overrides: - virtual void OnAutoHideBehaviorChanged( - ash::ShelfAutoHideBehavior new_behavior) OVERRIDE; - protected: // ChromeLauncherController overrides: @@ -393,9 +385,6 @@ class ChromeLauncherControllerPerBrowser scoped_ptr extension_enable_flow_; - // Launchers that are currently being observed. - std::set launchers_; - DISALLOW_COPY_AND_ASSIGN(ChromeLauncherControllerPerBrowser); }; -- cgit v1.1