Revert "Fix the AppWindowLauncherItemController::GetAppWindow crash issue."

This reverts commit aa51435c.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 837244 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2FhNTE0MzVjM2I0MWJkYjk3ZjY5ZmFhYzNmNjE0NDc4MGYyN2U2MWIM

Sample Failed Build: https://ci.chromium.org/b/8860789670201602304

Sample Failed Step: unit_tests

Original change's description:
> Fix the AppWindowLauncherItemController::GetAppWindow crash issue.
> 
> This CL is the 3rd option to resolve the crash of below call stack:
> AppWindowLauncherItemController::GetAppWindow(aura::Window*, bool)
> AppWindowLauncherItemController::SetActiveWindow(aura::Window*)
> AppWindowLauncherController::OnWindowActivated()
> 
> One possible guessing is the AppWindowLauncherItemController object has
> been deleted, but still used by AppServiceAppWindowLauncherController.
> So modify the destruct function, and set the controller as null in
> AppWindowBase.
> 
> BUG=1128491
> 
> Change-Id: Ied59f857653aa0a136cbe9f903ff79ce5856e34c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586647
> Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#837244}


No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG=1128491

Change-Id: Ia518fe2f01bbb128758982250405ffd470008c0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2594556
Cr-Commit-Position: refs/heads/master@{#837449}

chrome/browser/chromeos/plugin_vm/plugin_vm_files.cc

@@ -23,7 +23,6 @@
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/ash/launcher/app_window_base.h"
 #include "chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chromeos/dbus/cicerone/cicerone_service.pb.h"
@@ -31,6 +30,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "ui/base/base_window.h"
 
 namespace plugin_vm {
 
@@ -73,7 +73,7 @@ void FocusAllPluginVmWindows() {
   if (!launcher_item_controller) {
     return;
   }
-  for (auto* app_window : launcher_item_controller->windows()) {
+  for (ui::BaseWindow* app_window : launcher_item_controller->windows()) {
     app_window->Activate();
   }
 }

chrome/browser/chromeos/plugin_vm/plugin_vm_files_unittest.cc

@@ -17,7 +17,6 @@
 #include "chrome/browser/chromeos/plugin_vm/mock_plugin_vm_manager.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_manager_factory.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
-#include "chrome/browser/ui/ash/launcher/app_window_base.h"
 #include "chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/test/base/testing_profile.h"
@@ -27,24 +26,8 @@
 #include "content/public/test/browser_task_environment.h"
 #include "storage/browser/file_system/external_mount_points.h"
 #include "storage/common/file_system/file_system_types.h"
-#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-
-namespace {
-
-class MockAppWindowBase : public AppWindowBase {
- public:
-  MockAppWindowBase(const ash::ShelfID& shelf_id, views::Widget* widget)
-      : AppWindowBase(shelf_id, widget) {}
-  ~MockAppWindowBase() = default;
-  MockAppWindowBase(const MockAppWindowBase&) = delete;
-  MockAppWindowBase& operator=(const MockAppWindowBase&) = delete;
-
-  MOCK_METHOD(gfx::NativeWindow, GetNativeWindow, (), (const));
-  MOCK_METHOD(void, Activate, (), ());
-};
-
-}  // namespace
+#include "ui/base/test/mock_base_window.h"
 
 namespace plugin_vm {
 
@@ -173,10 +156,10 @@ TEST_F(PluginVmFilesTest, LaunchPluginVmApp) {
   std::move(launch_plugin_vm_callback).Run(/*success=*/true);
   ASSERT_FALSE(cicerone_response_callback.is_null());
 
-  ash::ShelfID shelf_id(kPluginVmShelfAppId);
   auto launcher_item_controller =
-      std::make_unique<AppWindowLauncherItemController>(shelf_id);
-  MockAppWindowBase mock_window(shelf_id, nullptr);
+      std::make_unique<AppWindowLauncherItemController>(
+          ash::ShelfID(kPluginVmShelfAppId));
+  ui::test::MockBaseWindow mock_window;
   launcher_item_controller->AddWindow(&mock_window);
   shelf_model.SetShelfItemDelegate(ash::ShelfID(kPluginVmShelfAppId),
                                    std::move(launcher_item_controller));

chrome/browser/ui/ash/chrome_new_window_client.cc

@@ -31,7 +31,6 @@
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
-#include "chrome/browser/ui/ash/launcher/app_window_base.h"
 #include "chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
@@ -69,6 +68,7 @@
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "ui/aura/window.h"
+#include "ui/base/base_window.h"
 #include "ui/base/page_transition_types.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/display/types/display_constants.h"

chrome/browser/ui/ash/launcher/app_service/app_service_app_window_crostini_tracker.cc

@@ -22,7 +22,6 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/launcher/app_service/app_service_app_window_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/app_service/app_service_app_window_launcher_item_controller.h"
-#include "chrome/browser/ui/ash/launcher/app_window_base.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/shelf_spinner_controller.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_helper.h"
@@ -34,6 +33,7 @@
 #include "components/exo/shell_surface_util.h"
 #include "components/user_manager/user_manager.h"
 #include "ui/aura/window.h"
+#include "ui/base/base_window.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/wm/core/window_util.h"
@@ -182,7 +182,7 @@ void AppServiceAppWindowCrostiniTracker::OnAppLaunchRequested(
   // necessary.
   if (!launcher_item_controller)
     return;
-  for (AppWindowBase* app_window : launcher_item_controller->windows()) {
+  for (ui::BaseWindow* app_window : launcher_item_controller->windows()) {
     activation_permissions_.emplace(
         app_window->GetNativeWindow(),
         exo::GrantPermissionToActivate(app_window->GetNativeWindow(),

chrome/browser/ui/ash/launcher/app_service/app_service_app_window_launcher_controller.cc

@@ -556,25 +556,23 @@ void AppServiceAppWindowLauncherController::AddAppWindowToShelf(
 
   AppWindowLauncherItemController* item_controller =
       owner()->shelf_model()->GetAppWindowLauncherItemController(shelf_id);
-  if (item_controller) {
-    item_controller->AddWindow(app_window);
-    app_window->SetController(item_controller);
-    return;
+  if (item_controller == nullptr) {
+    auto controller =
+        std::make_unique<AppServiceAppWindowLauncherItemController>(shelf_id,
+                                                                    this);
+    item_controller = controller.get();
+    if (!owner()->GetItem(shelf_id)) {
+      owner()->CreateAppLauncherItem(std::move(controller),
+                                     ash::STATUS_RUNNING);
+    } else {
+      owner()->shelf_model()->SetShelfItemDelegate(shelf_id,
+                                                   std::move(controller));
+      owner()->SetItemStatus(shelf_id, ash::STATUS_RUNNING);
+    }
   }
 
-  auto controller = std::make_unique<AppServiceAppWindowLauncherItemController>(
-      shelf_id, this);
-  item_controller = controller.get();
   item_controller->AddWindow(app_window);
   app_window->SetController(item_controller);
-
-  if (!owner()->GetItem(shelf_id)) {
-    owner()->CreateAppLauncherItem(std::move(controller), ash::STATUS_RUNNING);
-  } else {
-    owner()->shelf_model()->SetShelfItemDelegate(shelf_id,
-                                                 std::move(controller));
-    owner()->SetItemStatus(shelf_id, ash::STATUS_RUNNING);
-  }
 }
 
 void AppServiceAppWindowLauncherController::RemoveAppWindowFromShelf(

chrome/browser/ui/ash/launcher/app_service/app_service_app_window_launcher_item_controller.cc

@@ -10,7 +10,6 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
 #include "chrome/browser/ui/ash/launcher/app_service/app_service_app_window_launcher_controller.h"
-#include "chrome/browser/ui/ash/launcher/app_window_base.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chromeos/ui/base/window_properties.h"
 #include "chromeos/ui/base/window_state_type.h"
@@ -44,7 +43,7 @@ void AppServiceAppWindowLauncherItemController::ItemSelected(
     // Tapping the shelf icon of an app that's showing PIP means expanding PIP.
     // Even if the app contains multiple windows, we just expand PIP without
     // showing the menu on the shelf icon.
-    for (AppWindowBase* window : windows()) {
+    for (ui::BaseWindow* window : windows()) {
       aura::Window* native_window = window->GetNativeWindow();
       if (native_window->GetProperty(chromeos::kWindowStateTypeKey) ==
           chromeos::WindowStateType::kPip) {

chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc

@@ -11,11 +11,11 @@
 #include "ash/public/cpp/shelf_types.h"
 #include "ash/public/cpp/window_properties.h"
 #include "chrome/browser/ui/ash/ash_util.h"
-#include "chrome/browser/ui/ash/launcher/app_window_base.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/launcher_controller_helper.h"
 #include "chrome/browser/ui/ash/launcher/shelf_context_menu.h"
 #include "ui/aura/client/aura_constants.h"
+#include "ui/base/base_window.h"
 #include "ui/wm/core/window_util.h"
 
 namespace {
@@ -36,7 +36,7 @@ ash::ShelfAction ShowAndActivateOrMinimize(ui::BaseWindow* app_window,
 // Returns the action performed. Should be one of SHELF_ACTION_NONE,
 // SHELF_ACTION_WINDOW_ACTIVATED, or SHELF_ACTION_WINDOW_MINIMIZED.
 ash::ShelfAction ActivateOrAdvanceToNextAppWindow(
-    AppWindowBase* window_to_show,
+    ui::BaseWindow* window_to_show,
     const AppWindowLauncherItemController::WindowList& windows) {
   DCHECK(window_to_show);
 
@@ -63,14 +63,9 @@ AppWindowLauncherItemController::AppWindowLauncherItemController(
     const ash::ShelfID& shelf_id)
     : ash::ShelfItemDelegate(shelf_id) {}
 
-AppWindowLauncherItemController::~AppWindowLauncherItemController() {
-  for (auto* window : windows_)
-    window->SetController(nullptr);
-  for (auto* window : hidden_windows_)
-    window->SetController(nullptr);
-}
+AppWindowLauncherItemController::~AppWindowLauncherItemController() {}
 
-void AppWindowLauncherItemController::AddWindow(AppWindowBase* app_window) {
+void AppWindowLauncherItemController::AddWindow(ui::BaseWindow* app_window) {
   aura::Window* window = app_window->GetNativeWindow();
   if (window && !observed_windows_.IsObserving(window))
     observed_windows_.Add(window);
@@ -85,12 +80,12 @@ AppWindowLauncherItemController::WindowList::iterator
 AppWindowLauncherItemController::GetFromNativeWindow(aura::Window* window,
                                                      WindowList& list) {
   return std::find_if(list.begin(), list.end(),
-                      [window](AppWindowBase* base_window) {
+                      [window](ui::BaseWindow* base_window) {
                         return base_window->GetNativeWindow() == window;
                       });
 }
 
-void AppWindowLauncherItemController::RemoveWindow(AppWindowBase* app_window) {
+void AppWindowLauncherItemController::RemoveWindow(ui::BaseWindow* app_window) {
   DCHECK(app_window);
   aura::Window* window = app_window->GetNativeWindow();
   if (window && observed_windows_.IsObserving(window))
@@ -110,7 +105,7 @@ void AppWindowLauncherItemController::RemoveWindow(AppWindowBase* app_window) {
   UpdateShelfItemIcon();
 }
 
-AppWindowBase* AppWindowLauncherItemController::GetAppWindow(
+ui::BaseWindow* AppWindowLauncherItemController::GetAppWindow(
     aura::Window* window,
     bool include_hidden) {
   auto iter = GetFromNativeWindow(window, windows_);
@@ -126,7 +121,7 @@ AppWindowBase* AppWindowLauncherItemController::GetAppWindow(
 
 void AppWindowLauncherItemController::SetActiveWindow(aura::Window* window) {
   // If the window is hidden, do not set it as last_active_window
-  AppWindowBase* app_window = GetAppWindow(window, false);
+  ui::BaseWindow* app_window = GetAppWindow(window, false);
   if (app_window)
     last_active_window_ = app_window;
   UpdateShelfItemIcon();
@@ -162,7 +157,7 @@ void AppWindowLauncherItemController::ItemSelected(
     last_active = nullptr;
   }
 
-  AppWindowBase* window_to_show =
+  ui::BaseWindow* window_to_show =
       last_active ? last_active : filtered_windows.front();
   // If the event was triggered by a keystroke, we try to advance to the next
   // item if the window we are trying to activate is already active.
@@ -257,7 +252,7 @@ void AppWindowLauncherItemController::OnWindowPropertyChanged(
   }
 }
 
-AppWindowBase* AppWindowLauncherItemController::GetLastActiveWindow() {
+ui::BaseWindow* AppWindowLauncherItemController::GetLastActiveWindow() {
   if (last_active_window_)
     return last_active_window_;
   if (windows_.empty())
@@ -265,12 +260,13 @@ AppWindowBase* AppWindowLauncherItemController::GetLastActiveWindow() {
   return windows_.front();
 }
 
+
 void AppWindowLauncherItemController::UpdateShelfItemIcon() {
   // Set the shelf item icon from the kAppIconKey property of the current
   // (or most recently) active window. If there is no valid icon, ask
   // ChromeLauncherController to update the icon.
   const gfx::ImageSkia* app_icon = nullptr;
-  AppWindowBase* last_active_window = GetLastActiveWindow();
+  ui::BaseWindow* last_active_window = GetLastActiveWindow();
   if (last_active_window && last_active_window->GetNativeWindow()) {
     app_icon = last_active_window->GetNativeWindow()->GetProperty(
         aura::client::kAppIconKey);

chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.h

@@ -15,7 +15,10 @@
 #include "ui/aura/window.h"
 #include "ui/aura/window_observer.h"
 
-class AppWindowBase;
+namespace ui {
+class BaseWindow;
+}
+
 class ShelfContextMenu;
 
 // This is a ShelfItemDelegate for abstract app windows (extension or ARC).
@@ -27,16 +30,16 @@ class ShelfContextMenu;
 class AppWindowLauncherItemController : public ash::ShelfItemDelegate,
                                         public aura::WindowObserver {
  public:
-  using WindowList = std::list<AppWindowBase*>;
+  using WindowList = std::list<ui::BaseWindow*>;
 
   explicit AppWindowLauncherItemController(const ash::ShelfID& shelf_id);
   ~AppWindowLauncherItemController() override;
 
-  void AddWindow(AppWindowBase* window);
-  void RemoveWindow(AppWindowBase* window);
+  void AddWindow(ui::BaseWindow* window);
+  void RemoveWindow(ui::BaseWindow* window);
 
   void SetActiveWindow(aura::Window* window);
-  AppWindowBase* GetAppWindow(aura::Window* window, bool include_hidden);
+  ui::BaseWindow* GetAppWindow(aura::Window* window, bool include_hidden);
 
   // ash::ShelfItemDelegate overrides:
   AppWindowLauncherItemController* AsAppWindowLauncherItemController() override;
@@ -71,7 +74,7 @@ class AppWindowLauncherItemController : public ash::ShelfItemDelegate,
 
  protected:
   // Returns last active window in the controller or first window.
-  AppWindowBase* GetLastActiveWindow();
+  ui::BaseWindow* GetLastActiveWindow();
 
  private:
   friend class ChromeLauncherControllerTest;
@@ -97,7 +100,7 @@ class AppWindowLauncherItemController : public ash::ShelfItemDelegate,
   // Pointer to the most recently active app window
   // TODO(khmel): Get rid of |last_active_window_| and provide more reliable
   // way to determine active window.
-  AppWindowBase* last_active_window_ = nullptr;
+  ui::BaseWindow* last_active_window_ = nullptr;
 
   // Scoped list of observed windows (for removal on destruction)
   ScopedObserver<aura::Window, aura::WindowObserver> observed_windows_{this};