Revert 215745 "Fix crash when reloading packaged app. "

> Fix crash when reloading packaged app. 
> 
> When a shell window was closed, it wasn't removed from the registry immediately 
> (it was removed in a callback from OnNativeClose, which happens a bit later.) 
> This meant that when the app was loaded again, the window was still in the 
> registry with a stale pointer to the old extension. ExtensionSettingsHandler 
> was tickling this by iterating shell windows on extension load.
> 
> BUG=174250
> TEST=On chromeos, enable the 'Enable debugging for packed apps.' flag. Then
> open chrome://extensions. Then open a packaged app, and reload it. Chrome
> should not crash.
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=214844
> 
> Review URL: https://chromiumcodereview.appspot.com/20243003

TBR=benwells@chromium.org

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

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

6 files changed, 16 insertions, 66 deletions

diff --git a/apps/shell_window.cc b/apps/shell_window.cc
index 4f8bd42..d0dd9cc 100644
--- a/apps/shell_window.cc
+++ b/apps/shell_window.cc
@@ -280,15 +280,9 @@ void ShellWindow::RequestToLockMouse(WebContents* web_contents,
 }
 
 void ShellWindow::OnNativeClose() {
-  if (extension_) {
-    // If |extension_| is NULL here, it is because the extension has been
-    // unloaded. In this case, the window will have already been removed from
-    // the ShellWindowRegistry, and the app can not be informed of the window
-    // closing as it has gone.
-    extensions::ShellWindowRegistry::Get(profile_)->RemoveShellWindow(this);
-    if (shell_window_contents_)
-      shell_window_contents_->NativeWindowClosed();
-  }
+  extensions::ShellWindowRegistry::Get(profile_)->RemoveShellWindow(this);
+  if (shell_window_contents_)
+    shell_window_contents_->NativeWindowClosed();
   delete this;
 }
 
@@ -453,7 +447,7 @@ void ShellWindow::UpdateExtensionAppIcon() {
 }
 
 void ShellWindow::CloseContents(WebContents* contents) {
-  Close();
+  native_app_window_->Close();
 }
 
 bool ShellWindow::ShouldSuppressDialogs() {
@@ -532,16 +526,12 @@ void ShellWindow::Observe(int type,
       const extensions::Extension* unloaded_extension =
           content::Details<extensions::UnloadedExtensionInfo>(
               details)->extension;
-      if (extension_ == unloaded_extension) {
-        Close();
-        // After this notification finishes processing, the Extension will be
-        // deleted, so we null out our reference to avoid bad access.
-        extension_ = NULL;
-      }
+      if (extension_ == unloaded_extension)
+        native_app_window_->Close();
       break;
     }
     case chrome::NOTIFICATION_APP_TERMINATING:
-      Close();
+      native_app_window_->Close();
       break;
     default:
       NOTREACHED() << "Received unexpected notification";
@@ -567,11 +557,6 @@ WebContentsModalDialogHost* ShellWindow::GetWebContentsModalDialogHost() {
   return native_app_window_.get();
 }
 
-void ShellWindow::Close() {
-  extensions::ShellWindowRegistry::Get(profile_)->RemoveShellWindow(this);
-  native_app_window_->Close();
-}
-
 void ShellWindow::AddMessageToDevToolsConsole(ConsoleMessageLevel level,
                                               const std::string& message) {
   content::RenderViewHost* rvh = web_contents()->GetRenderViewHost();
diff --git a/apps/shell_window.h b/apps/shell_window.h
index a1e667b..a10cb5b 100644
--- a/apps/shell_window.h
+++ b/apps/shell_window.h
@@ -181,6 +181,7 @@ class ShellWindow : public content::NotificationObserver,
             ShellWindowContents* shell_window_contents,
             const CreateParams& params);
 
+
   const std::string& window_key() const { return window_key_; }
   const SessionID& session_id() const { return session_id_; }
   const extensions::Extension* extension() const { return extension_; }
@@ -298,11 +299,6 @@ class ShellWindow : public content::NotificationObserver,
   virtual bool IsWebContentsVisible(
       content::WebContents* web_contents) OVERRIDE;
 
-  // Remove the window from the ShellWindowRegistry and tell the native
-  // window to close. The native window won't be actually closed until
-  // OnNativeClose().
-  void Close();
-
   // Helper method to add a message to the renderer's DevTools console.
   void AddMessageToDevToolsConsole(content::ConsoleMessageLevel level,
                                    const std::string& message);
diff --git a/chrome/browser/extensions/platform_app_browsertest.cc b/chrome/browser/extensions/platform_app_browsertest.cc
index 7d13d5e..55cf56d 100644
--- a/chrome/browser/extensions/platform_app_browsertest.cc
+++ b/chrome/browser/extensions/platform_app_browsertest.cc
@@ -34,7 +34,6 @@
 #include "chrome/browser/ui/extensions/native_app_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -1084,22 +1083,5 @@ IN_PROC_BROWSER_TEST_F(PlatformAppIncognitoBrowserTest, IncognitoComponentApp) {
 
 #endif  // defined(OS_CHROMEOS)
 
-IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, DontCrashWhenReloading) {
-  // Regression test for http://crbug.com/174250
-  browser()->profile()->GetPrefs()->SetBoolean(
-      prefs::kExtensionsUIDeveloperMode, true);
-  ui_test_utils::NavigateToURL(browser(), GURL("chrome://extensions"));
-  ExtensionTestMessageListener launched_listener("Launched", true);
-  const Extension* extension = LoadAndLaunchPlatformApp("reload");
-  ASSERT_TRUE(extension);
-  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
-  ASSERT_TRUE(GetFirstShellWindow());
-
-  // Now tell the app to reload itself.
-  ExtensionTestMessageListener launched_listener2("Launched", false);
-  launched_listener.Reply("reload");
-  ASSERT_TRUE(launched_listener2.WaitUntilSatisfied());
-  ASSERT_TRUE(GetFirstShellWindow());
-}
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/shell_window_registry.cc b/chrome/browser/extensions/shell_window_registry.cc
index 216da68..b9c939d 100644
--- a/chrome/browser/extensions/shell_window_registry.cc
+++ b/chrome/browser/extensions/shell_window_registry.cc
@@ -85,10 +85,9 @@ void ShellWindowRegistry::RemoveShellWindow(ShellWindow* shell_window) {
   const ShellWindowList::iterator it = std::find(shell_windows_.begin(),
                                                  shell_windows_.end(),
                                                  shell_window);
-  if (it != shell_windows_.end()) {
+  if (it != shell_windows_.end())
     shell_windows_.erase(it);
-    FOR_EACH_OBSERVER(Observer, observers_, OnShellWindowRemoved(shell_window));
-  }
+  FOR_EACH_OBSERVER(Observer, observers_, OnShellWindowRemoved(shell_window));
 }
 
 void ShellWindowRegistry::AddObserver(Observer* observer) {
diff --git a/chrome/browser/ui/cocoa/extensions/native_app_window_cocoa.mm b/chrome/browser/ui/cocoa/extensions/native_app_window_cocoa.mm
index 49f2218..67611f4 100644
--- a/chrome/browser/ui/cocoa/extensions/native_app_window_cocoa.mm
+++ b/chrome/browser/ui/cocoa/extensions/native_app_window_cocoa.mm
@@ -4,10 +4,8 @@
 
 #include "chrome/browser/ui/cocoa/extensions/native_app_window_cocoa.h"
 
-#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/mac/mac_util.h"
-#include "base/message_loop/message_loop.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/cocoa/browser_window_utils.h"
@@ -760,13 +758,7 @@ void NativeAppWindowCocoa::RemoveObserver(
 void NativeAppWindowCocoa::WindowWillClose() {
   [window_controller_ setAppWindow:NULL];
   shell_window_->OnNativeWindowChanged();
-  // On other platforms, the native window doesn't get destroyed synchronously.
-  // We simulate that here so that ShellWindow can assume that it doesn't get
-  // deleted immediately upon calling Close().
-  base::MessageLoop::current()->PostTask(
-      FROM_HERE,
-      base::Bind(&ShellWindow::OnNativeClose,
-                 base::Unretained(shell_window_)));
+  shell_window_->OnNativeClose();
 }
 
 void NativeAppWindowCocoa::WindowDidBecomeKey() {
diff --git a/chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc b/chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc
index 0d88fcc..4154834 100644
--- a/chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc
+++ b/chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc
@@ -7,7 +7,6 @@
 #include <gdk/gdkx.h>
 #include <vector>
 
-#include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_pump_gtk.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
@@ -244,15 +243,12 @@ void NativeAppWindowGtk::Close() {
   // To help catch bugs in any event handlers that might get fired during the
   // destruction, set window_ to NULL before any handlers will run.
   window_ = NULL;
-  gtk_widget_destroy(window);
 
-  // On other platforms, the native window doesn't get destroyed synchronously.
-  // We simulate that here so that ShellWindow can assume that it doesn't get
-  // deleted immediately upon calling Close().
-  base::MessageLoop::current()->PostTask(
-      FROM_HERE,
-      base::Bind(&ShellWindow::OnNativeClose,
-                 base::Unretained(shell_window_)));
+  // OnNativeClose does a delete this so no other members should
+  // be accessed after. gtk_widget_destroy is safe (and must
+  // be last).
+  shell_window_->OnNativeClose();
+  gtk_widget_destroy(window);
 }
 
 void NativeAppWindowGtk::Activate() {