Close AppWindow to perform clean shutdown

It results in a use-after-free if AppWindow is not closed.

Add a new method DesktopController::CreateAppWindow()
-Rename existing CreateAppWindow() method to CreateShellAppWindow().
-Add a new method CreateAppWindow() to remember the AppWindow.
-Close the AppWindow in CloseAppWindows().

Implement ShellNativeAppWindow::Close()
-Call AppWindow::OnNativeClose() to destruct the AppWindow.

BUG=387288

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

Cr-Commit-Position: refs/heads/master@{#294124}

athena/main/athena_main.cc

@@ -51,9 +51,9 @@ class AthenaDesktopController : public extensions::DesktopController {
     return athena::AthenaEnv::Get()->GetHost();
   }
 
-  // Creates a new app window and adds it to the desktop. The desktop maintains
-  // ownership of the window.
-  virtual extensions::ShellAppWindow* CreateAppWindow(
+  // Creates a new ShellAppWindow and adds it to the desktop. The desktop
+  // maintains ownership of the window.
+  virtual extensions::ShellAppWindow* CreateShellAppWindow(
       content::BrowserContext* context,
       const extensions::Extension* extension) OVERRIDE {
     extensions::ShellAppWindow* app_window = new extensions::ShellAppWindow();
@@ -64,6 +64,15 @@ class AthenaDesktopController : public extensions::DesktopController {
     return app_window;
   }
 
+  // Creates a new app window and adds it to the desktop. The desktop maintains
+  // ownership of the window.
+  virtual extensions::AppWindow* CreateAppWindow(
+      content::BrowserContext* context,
+      const extensions::Extension* extension) OVERRIDE {
+    NOTIMPLEMENTED();
+    return NULL;
+  }
+
   // Adds the window to the desktop.
   virtual void AddAppWindow(aura::Window* window) OVERRIDE {
     NOTIMPLEMENTED();

extensions/shell/browser/api/shell/shell_api.cc

@@ -54,8 +54,9 @@ ExtensionFunction::ResponseAction ShellCreateWindowFunction::Run() {
     return RespondNow(Error(kInvalidArguments));
 
   // The desktop keeps ownership of the window.
-  ShellAppWindow* app_window = DesktopController::instance()->CreateAppWindow(
-      browser_context(), extension());
+  ShellAppWindow* app_window =
+      DesktopController::instance()->CreateShellAppWindow(
+          browser_context(), extension());
   app_window->LoadURL(url);
 
   // Create the reply to send to the renderer.

extensions/shell/browser/desktop_controller.h

@@ -15,6 +15,7 @@ class BrowserContext;
 }
 
 namespace extensions {
+class AppWindow;
 class Extension;
 class ShellAppWindow;
 
@@ -35,11 +36,17 @@ class DesktopController {
   // Returns the WindowTreeHost created by this DesktopController.
   virtual aura::WindowTreeHost* GetHost() = 0;
 
+  // Creates a new ShellAppWindow and adds it to the desktop. The desktop
+  // maintains ownership of the window. The window must be closed before
+  // |extension| is destroyed.
+  virtual ShellAppWindow* CreateShellAppWindow(content::BrowserContext* context,
+                                               const Extension* extension) = 0;
+
   // Creates a new app window and adds it to the desktop. The desktop maintains
   // ownership of the window. The window must be closed before |extension| is
   // destroyed.
-  virtual ShellAppWindow* CreateAppWindow(content::BrowserContext* context,
-                                          const Extension* extension) = 0;
+  virtual AppWindow* CreateAppWindow(content::BrowserContext* context,
+                                     const Extension* extension) = 0;
 
   // Attaches the window to our window hierarchy.
   virtual void AddAppWindow(aura::Window* window) = 0;

extensions/shell/browser/shell_apps_client.cc

@@ -6,7 +6,6 @@
 
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/shell/browser/desktop_controller.h"
-#include "extensions/shell/browser/shell_app_delegate.h"
 #include "extensions/shell/browser/shell_native_app_window.h"
 
 namespace extensions {
@@ -25,7 +24,7 @@ ShellAppsClient::GetLoadedBrowserContexts() {
 
 AppWindow* ShellAppsClient::CreateAppWindow(content::BrowserContext* context,
                                             const Extension* extension) {
-  return new AppWindow(context, new ShellAppDelegate, extension);
+  return DesktopController::instance()->CreateAppWindow(context, extension);
 }
 
 NativeAppWindow* ShellAppsClient::CreateNativeAppWindow(

extensions/shell/browser/shell_desktop_controller.cc

@@ -5,6 +5,9 @@
 #include "extensions/shell/browser/shell_desktop_controller.h"
 
 #include "base/command_line.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/native_app_window.h"
+#include "extensions/shell/browser/shell_app_delegate.h"
 #include "extensions/shell/browser/shell_app_window.h"
 #include "extensions/shell/common/switches.h"
 #include "ui/aura/client/cursor_client.h"
@@ -153,7 +156,8 @@ class AppsFocusRules : public wm::BaseFocusRules {
 
 }  // namespace
 
-ShellDesktopController::ShellDesktopController() {
+ShellDesktopController::ShellDesktopController()
+    : app_window_(NULL) {
 #if defined(OS_CHROMEOS)
   chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
       AddObserver(this);
@@ -167,7 +171,7 @@ ShellDesktopController::ShellDesktopController() {
 }
 
 ShellDesktopController::~ShellDesktopController() {
-  app_window_.reset();
+  CloseAppWindows();
   DestroyRootWindow();
 #if defined(OS_CHROMEOS)
   chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
@@ -179,20 +183,27 @@ aura::WindowTreeHost* ShellDesktopController::GetHost() {
   return host_.get();
 }
 
-ShellAppWindow* ShellDesktopController::CreateAppWindow(
+ShellAppWindow* ShellDesktopController::CreateShellAppWindow(
     content::BrowserContext* context,
     const Extension* extension) {
   aura::Window* root_window = GetHost()->window();
 
-  app_window_.reset(new ShellAppWindow);
-  app_window_->Init(context, extension, root_window->bounds().size());
+  shell_app_window_.reset(new ShellAppWindow);
+  shell_app_window_->Init(context, extension, root_window->bounds().size());
 
   // Attach the web contents view to our window hierarchy.
-  aura::Window* content = app_window_->GetNativeWindow();
+  aura::Window* content = shell_app_window_->GetNativeWindow();
   AddAppWindow(content);
   content->Show();
 
-  return app_window_.get();
+  return shell_app_window_.get();
+}
+
+AppWindow* ShellDesktopController::CreateAppWindow(
+    content::BrowserContext* context,
+    const Extension* extension) {
+  app_window_ = new AppWindow(context, new ShellAppDelegate, extension);
+  return app_window_;
 }
 
 void ShellDesktopController::AddAppWindow(aura::Window* window) {
@@ -201,7 +212,11 @@ void ShellDesktopController::AddAppWindow(aura::Window* window) {
 }
 
 void ShellDesktopController::CloseAppWindows() {
-  app_window_.reset();
+  shell_app_window_.reset();
+  if (app_window_) {
+    app_window_->GetBaseWindow()->Close();  // Close() deletes |app_window_|.
+    app_window_ = NULL;
+  }
 }
 
 aura::Window* ShellDesktopController::GetDefaultParent(

extensions/shell/browser/shell_desktop_controller.h

@@ -67,8 +67,11 @@ class ShellDesktopController : public DesktopController,
 
   // DesktopController:
   virtual aura::WindowTreeHost* GetHost() OVERRIDE;
-  virtual ShellAppWindow* CreateAppWindow(content::BrowserContext* context,
-                                          const Extension* extension) OVERRIDE;
+  virtual ShellAppWindow* CreateShellAppWindow(
+      content::BrowserContext* context,
+      const Extension* extension) OVERRIDE;
+  virtual AppWindow* CreateAppWindow(content::BrowserContext* context,
+                                     const Extension* extension) OVERRIDE;
   virtual void AddAppWindow(aura::Window* window) OVERRIDE;
   virtual void CloseAppWindows() OVERRIDE;
 
@@ -131,7 +134,8 @@ class ShellDesktopController : public DesktopController,
 #endif
 
   // The desktop supports a single app window.
-  scoped_ptr<ShellAppWindow> app_window_;
+  scoped_ptr<ShellAppWindow> shell_app_window_;
+  AppWindow* app_window_;  // NativeAppWindow::Close() deletes this.
 
   DISALLOW_COPY_AND_ASSIGN(ShellDesktopController);
 };

extensions/shell/browser/shell_native_app_window.cc

@@ -80,7 +80,7 @@ void ShellNativeAppWindow::ShowInactive() {
 }
 
 void ShellNativeAppWindow::Close() {
-  NOTIMPLEMENTED();
+  app_window_->OnNativeClose();
 }
 
 void ShellNativeAppWindow::Activate() {