Revert of Make hidden app windows have no shelf presence [ChromeOS]. (https://codereview.chromium.org/234673002/)

Reason for revert:
Revert to reland in more merge-able chunks. See crbug.com/360896
for details.

Original issue's description:
> Make hidden app windows have no shelf presence [ChromeOS].
> 
> This CL fixes an issue with hidden app windows appearing in the shelf by
> unregistering the app windows when they are hidden and registering them
> again when shown.
> 
> Window visibility wasn't enough here as the window is not visible
> if it is minimized.
> 
> This CL adds OnAppWindowShown() and OnAppWindowHidden() methods to
> AppWindowObserver.
> 
> BUG=360896
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=265903

TBR=stevenjb@chromium.org, benwells@chromium.org, skuhne@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/262723003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267500 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 22 insertions, 208 deletions

diff --git a/apps/app_window.cc b/apps/app_window.cc
index cfb1213..efe31d2 100644
--- a/apps/app_window.cc
+++ b/apps/app_window.cc
@@ -280,11 +280,7 @@ void AppWindow::Init(const GURL& url,
 
   native_app_window_.reset(delegate_->CreateNativeAppWindow(this, new_params));
 
-  if (new_params.hidden) {
-    // Although the window starts hidden by default, calling Hide() here
-    // notifies observers of the window being hidden.
-    Hide();
-  } else {
+  if (!new_params.hidden) {
     // Panels are not activated by default.
     Show(window_type_is_panel() || !new_params.focused ? SHOW_INACTIVE
                                                        : SHOW_ACTIVE);
@@ -676,7 +672,6 @@ void AppWindow::Show(ShowType show_type) {
       GetBaseWindow()->ShowInactive();
       break;
   }
-  AppWindowRegistry::Get(browser_context_)->AppWindowShown(this);
 }
 
 void AppWindow::Hide() {
@@ -686,7 +681,6 @@ void AppWindow::Hide() {
   // show will not be delayed.
   show_on_first_paint_ = false;
   GetBaseWindow()->Hide();
-  AppWindowRegistry::Get(browser_context_)->AppWindowHidden(this);
 }
 
 void AppWindow::SetAlwaysOnTop(bool always_on_top) {
diff --git a/apps/app_window_registry.cc b/apps/app_window_registry.cc
index 4ed0b8b..b362a5b 100644
--- a/apps/app_window_registry.cc
+++ b/apps/app_window_registry.cc
@@ -45,12 +45,6 @@ std::string GetWindowKeyForRenderViewHost(
 
 namespace apps {
 
-void AppWindowRegistry::Observer::OnAppWindowHidden(AppWindow* app_window) {}
-
-void AppWindowRegistry::Observer::OnAppWindowShown(AppWindow* app_window) {}
-
-AppWindowRegistry::Observer::~Observer() {}
-
 AppWindowRegistry::AppWindowRegistry(content::BrowserContext* context)
     : context_(context),
       devtools_callback_(base::Bind(&AppWindowRegistry::OnDevToolsStateChanged,
@@ -83,14 +77,6 @@ void AppWindowRegistry::AppWindowActivated(AppWindow* app_window) {
   BringToFront(app_window);
 }
 
-void AppWindowRegistry::AppWindowHidden(AppWindow* app_window) {
-  FOR_EACH_OBSERVER(Observer, observers_, OnAppWindowHidden(app_window));
-}
-
-void AppWindowRegistry::AppWindowShown(AppWindow* app_window) {
-  FOR_EACH_OBSERVER(Observer, observers_, OnAppWindowShown(app_window));
-}
-
 void AppWindowRegistry::RemoveAppWindow(AppWindow* app_window) {
   const AppWindowList::iterator it =
       std::find(app_windows_.begin(), app_windows_.end(), app_window);
diff --git a/apps/app_window_registry.h b/apps/app_window_registry.h
index ebde350..cc3ee35 100644
--- a/apps/app_window_registry.h
+++ b/apps/app_window_registry.h
@@ -37,15 +37,9 @@ class AppWindowRegistry : public KeyedService {
     virtual void OnAppWindowIconChanged(apps::AppWindow* app_window) = 0;
     // Called just after a app window was removed.
     virtual void OnAppWindowRemoved(apps::AppWindow* app_window) = 0;
-    // Called just after a app window was hidden. This is different from
-    // window visibility as a minimize does not hide a window, but does make
-    // it not visible.
-    virtual void OnAppWindowHidden(apps::AppWindow* app_window);
-    // Called just after a app window was shown.
-    virtual void OnAppWindowShown(apps::AppWindow* app_window);
 
    protected:
-    virtual ~Observer();
+    virtual ~Observer() {}
   };
 
   typedef std::list<apps::AppWindow*> AppWindowList;
@@ -64,8 +58,6 @@ class AppWindowRegistry : public KeyedService {
   void AppWindowIconChanged(apps::AppWindow* app_window);
   // Called by |app_window| when it is activated.
   void AppWindowActivated(apps::AppWindow* app_window);
-  void AppWindowHidden(apps::AppWindow* app_window);
-  void AppWindowShown(apps::AppWindow* app_window);
   void RemoveAppWindow(apps::AppWindow* app_window);
 
   void AddObserver(Observer* observer);
diff --git a/chrome/browser/ui/ash/launcher/app_window_launcher_controller.cc b/chrome/browser/ui/ash/launcher/app_window_launcher_controller.cc
index d5a65bb..e12e8bb 100644
--- a/chrome/browser/ui/ash/launcher/app_window_launcher_controller.cc
+++ b/chrome/browser/ui/ash/launcher/app_window_launcher_controller.cc
@@ -85,6 +85,9 @@ void AppWindowLauncherController::AdditionalUserAddedToSession(
 }
 
 void AppWindowLauncherController::OnAppWindowAdded(AppWindow* app_window) {
+  if (!ControlsWindow(app_window->GetNativeWindow()))
+    return;
+  RegisterApp(app_window);
 }
 
 void AppWindowLauncherController::OnAppWindowIconChanged(
@@ -107,24 +110,6 @@ void AppWindowLauncherController::OnAppWindowRemoved(AppWindow* app_window) {
   // OnWindowDestroying() has been called, doing the removal.
 }
 
-void AppWindowLauncherController::OnAppWindowShown(AppWindow* app_window) {
-  aura::Window* window = app_window->GetNativeWindow();
-  if (!ControlsWindow(window))
-    return;
-
-  if (!IsRegisteredApp(window))
-    RegisterApp(app_window);
-}
-
-void AppWindowLauncherController::OnAppWindowHidden(AppWindow* app_window) {
-  aura::Window* window = app_window->GetNativeWindow();
-  if (!ControlsWindow(window))
-    return;
-
-  if (IsRegisteredApp(window))
-    UnregisterApp(window);
-}
-
 // Called from aura::Window::~Window(), before delegate_->OnWindowDestroyed()
 // which destroys AppWindow, so both |window| and the associated AppWindow
 // are valid here.
diff --git a/chrome/browser/ui/ash/launcher/app_window_launcher_controller.h b/chrome/browser/ui/ash/launcher/app_window_launcher_controller.h
index 9ac4b9b..5f51a62 100644
--- a/chrome/browser/ui/ash/launcher/app_window_launcher_controller.h
+++ b/chrome/browser/ui/ash/launcher/app_window_launcher_controller.h
@@ -53,8 +53,6 @@ class AppWindowLauncherController
   virtual void OnAppWindowAdded(apps::AppWindow* app_window) OVERRIDE;
   virtual void OnAppWindowIconChanged(apps::AppWindow* app_window) OVERRIDE;
   virtual void OnAppWindowRemoved(apps::AppWindow* app_window) OVERRIDE;
-  virtual void OnAppWindowShown(apps::AppWindow* app_window) OVERRIDE;
-  virtual void OnAppWindowHidden(apps::AppWindow* app_window) OVERRIDE;
 
   // Overriden from aura::WindowObserver:
   virtual void OnWindowDestroying(aura::Window* window) OVERRIDE;
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
index ef83a90..02ff9a0 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
@@ -1381,43 +1381,6 @@ IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, LaunchPanelWindow) {
   EXPECT_EQ(item_count, shelf_model()->item_count());
 }
 
-// Test that we get correct shelf presence with hidden app windows.
-IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, HiddenAppWindows) {
-  int item_count = shelf_model()->item_count();
-  const Extension* extension = LoadAndLaunchPlatformApp("launch");
-  AppWindow::CreateParams params;
-
-  // Create a hidden window.
-  params.hidden = true;
-  AppWindow* window_1 = CreateAppWindowFromParams(extension, params);
-  EXPECT_EQ(item_count, shelf_model()->item_count());
-
-  // Create a visible window.
-  params.hidden = false;
-  AppWindow* window_2 = CreateAppWindowFromParams(extension, params);
-  ++item_count;
-  EXPECT_EQ(item_count, shelf_model()->item_count());
-
-  // Minimize the visible window.
-  window_2->Minimize();
-  EXPECT_EQ(item_count, shelf_model()->item_count());
-
-  // Hide the visible window.
-  window_2->Hide();
-  --item_count;
-  EXPECT_EQ(item_count, shelf_model()->item_count());
-
-  // Show the originally hidden window.
-  window_1->Show(AppWindow::SHOW_ACTIVE);
-  ++item_count;
-  EXPECT_EQ(item_count, shelf_model()->item_count());
-
-  // Close the originally hidden window.
-  CloseAppWindow(window_1);
-  --item_count;
-  EXPECT_EQ(item_count, shelf_model()->item_count());
-}
-
 // Test attention states of windows.
 IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, WindowAttentionStatus) {
   const Extension* extension = LoadAndLaunchPlatformApp("launch");
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
index b0b4a01..8d9c413 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
@@ -2303,80 +2303,6 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,
   EXPECT_FALSE(v2_app_5.window()->GetNativeWindow()->IsVisible());
   SwitchActiveUser(profile1->GetProfileName());
   EXPECT_TRUE(v2_app_5.window()->GetNativeWindow()->IsVisible());
-
-  // Switching to desktop #2, hiding the app window and creating an app should
-  // teleport there automatically.
-  SwitchActiveUser(profile2->GetProfileName());
-  v2_app_1.window()->Hide();
-  V2App v2_app_6(profile1, extension1_);
-  EXPECT_FALSE(v2_app_1.window()->GetNativeWindow()->IsVisible());
-  EXPECT_FALSE(v2_app_2.window()->GetNativeWindow()->IsVisible());
-  EXPECT_TRUE(v2_app_6.window()->GetNativeWindow()->IsVisible());
-}
-
-// Check that V2 applications hide correctly on the shelf when the app window
-// is hidden.
-TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,
-       V2AppHiddenWindows) {
-  InitLauncherController();
-
-  TestingProfile* profile2 = CreateMultiUserProfile("user-2");
-  SwitchActiveUser(profile()->GetProfileName());
-  EXPECT_EQ(2, model_->item_count());
-
-  V2App v2_app_1(profile(), extension1_);
-  EXPECT_EQ(3, model_->item_count());
-  {
-    // Hide and show the app.
-    v2_app_1.window()->Hide();
-    EXPECT_EQ(2, model_->item_count());
-
-    v2_app_1.window()->Show(apps::AppWindow::SHOW_ACTIVE);
-    EXPECT_EQ(3, model_->item_count());
-  }
-  {
-    // Switch user, hide and show the app and switch back.
-    SwitchActiveUser(profile2->GetProfileName());
-    EXPECT_EQ(2, model_->item_count());
-
-    v2_app_1.window()->Hide();
-    EXPECT_EQ(2, model_->item_count());
-
-    v2_app_1.window()->Show(apps::AppWindow::SHOW_ACTIVE);
-    EXPECT_EQ(2, model_->item_count());
-
-    SwitchActiveUser(profile()->GetProfileName());
-    EXPECT_EQ(3, model_->item_count());
-  }
-  {
-    // Switch user, hide the app, switch back and then show it again.
-    SwitchActiveUser(profile2->GetProfileName());
-    EXPECT_EQ(2, model_->item_count());
-
-    v2_app_1.window()->Hide();
-    EXPECT_EQ(2, model_->item_count());
-
-    SwitchActiveUser(profile()->GetProfileName());
-    EXPECT_EQ(2, model_->item_count());
-
-    v2_app_1.window()->Show(apps::AppWindow::SHOW_ACTIVE);
-    EXPECT_EQ(3, model_->item_count());
-  }
-  {
-    // Create a second app, hide and show it and then hide both apps.
-    V2App v2_app_2(profile(), extension1_);
-    EXPECT_EQ(3, model_->item_count());
-
-    v2_app_2.window()->Hide();
-    EXPECT_EQ(3, model_->item_count());
-
-    v2_app_2.window()->Show(apps::AppWindow::SHOW_ACTIVE);
-    EXPECT_EQ(3, model_->item_count());
-
-    v2_app_1.window()->Hide();
-    v2_app_2.window()->Hide();
-    EXPECT_EQ(2, model_->item_count());
-  }
 }
 #endif  // defined(OS_CHROMEOS)
 
diff --git a/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc b/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc
index d7f7233..4f44d57 100644
--- a/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc
+++ b/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc
@@ -5,7 +5,6 @@
 #include "chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h"
 
 #include "apps/app_window.h"
-#include "apps/ui/native_app_window.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
@@ -58,9 +57,7 @@ void MultiProfileAppWindowLauncherController::ActiveUserChanged(
     Profile* profile =
         Profile::FromBrowserContext(app_window->browser_context());
     if (multi_user_util::IsProfileFromActiveUser(profile) &&
-        !IsRegisteredApp(app_window->GetNativeWindow()) &&
-        (app_window->GetBaseWindow()->IsMinimized() ||
-         app_window->GetNativeWindow()->IsVisible()))
+        !IsRegisteredApp(app_window->GetNativeWindow()))
       RegisterApp(*it);
   }
 }
@@ -77,50 +74,25 @@ void MultiProfileAppWindowLauncherController::OnAppWindowAdded(
     apps::AppWindow* app_window) {
   if (!ControlsWindow(app_window->GetNativeWindow()))
     return;
-
   app_window_list_.push_back(app_window);
   Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
-  // If the window got created for a non active user but the user allowed to
-  // teleport to the current user's desktop, we teleport it now.
-  if (!multi_user_util::IsProfileFromActiveUser(profile) &&
-      UserHasAppOnActiveDesktop(app_window)) {
-    chrome::MultiUserWindowManager::GetInstance()->ShowWindowForUser(
-        app_window->GetNativeWindow(), multi_user_util::GetCurrentUserId());
-  }
-}
-
-void MultiProfileAppWindowLauncherController::OnAppWindowShown(
-    apps::AppWindow* app_window) {
-  if (!ControlsWindow(app_window->GetNativeWindow()))
-    return;
-
-  Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
-
-  if (multi_user_util::IsProfileFromActiveUser(profile) &&
-      !IsRegisteredApp(app_window->GetNativeWindow())) {
+  if (multi_user_util::IsProfileFromActiveUser(profile)) {
     RegisterApp(app_window);
-    return;
-  }
-
-  // The panel layout manager only manages windows which are anchored.
-  // Since this window did never had an anchor, it would stay hidden. We
-  // therefore make it visible now.
-  if (UserHasAppOnActiveDesktop(app_window) &&
-      app_window->GetNativeWindow()->type() == ui::wm::WINDOW_TYPE_PANEL &&
-      !app_window->GetNativeWindow()->layer()->GetTargetOpacity()) {
-    app_window->GetNativeWindow()->layer()->SetOpacity(1.0f);
-  }
-}
-
-void MultiProfileAppWindowLauncherController::OnAppWindowHidden(
-    apps::AppWindow* app_window) {
-  if (!ControlsWindow(app_window->GetNativeWindow()))
-    return;
-
-  Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
-  if (multi_user_util::IsProfileFromActiveUser(profile) &&
-      IsRegisteredApp(app_window->GetNativeWindow())) {
-    UnregisterApp(app_window->GetNativeWindow());
+  } else {
+    // If the window got created for a non active user but the user allowed to
+    // teleport to the current user's desktop, we teleport it now.
+    if (UserHasAppOnActiveDesktop(app_window)) {
+      chrome::MultiUserWindowManager::GetInstance()->ShowWindowForUser(
+          app_window->GetNativeWindow(),
+          multi_user_util::GetCurrentUserId());
+      if (app_window->GetNativeWindow()->type() == ui::wm::WINDOW_TYPE_PANEL &&
+          !app_window->GetNativeWindow()->layer()->GetTargetOpacity()) {
+        // The panel layout manager only manages windows which are anchored.
+        // Since this window did never had an anchor, it would stay hidden. We
+        // therefore make it visible now.
+        app_window->GetNativeWindow()->layer()->SetOpacity(1.0f);
+      }
+    }
   }
 }
 
diff --git a/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h b/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h
index 546cdbe..c31862c 100644
--- a/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h
+++ b/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h
@@ -23,8 +23,6 @@ class MultiProfileAppWindowLauncherController
   // Overridden from AppWindowRegistry::Observer:
   virtual void OnAppWindowAdded(apps::AppWindow* app_window) OVERRIDE;
   virtual void OnAppWindowRemoved(apps::AppWindow* app_window) OVERRIDE;
-  virtual void OnAppWindowShown(apps::AppWindow* app_window) OVERRIDE;
-  virtual void OnAppWindowHidden(apps::AppWindow* app_window) OVERRIDE;
 
  private:
   typedef std::vector<apps::AppWindow*> AppWindowList;