Revert 182194

> Fix issue 174250, 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
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/12210107

TBR=jeremya@chromium.org
Review URL: https://codereview.chromium.org/12207155

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

chrome/browser/extensions/platform_app_browsertest.cc

@@ -28,7 +28,6 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/web_contents_modal_dialog_manager.h"
 #include "chrome/common/chrome_notification_types.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/devtools_agent_host.h"
@@ -687,7 +686,7 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, ReloadRelaunches) {
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
   ASSERT_TRUE(GetFirstShellWindow());
 
-  // Now tell the app to reload itself.
+  // Now tell the app to reload itself
   ExtensionTestMessageListener launched_listener2("Launched", false);
   launched_listener.Reply("reload");
   ASSERT_TRUE(launched_listener2.WaitUntilSatisfied());
@@ -852,22 +851,4 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, MAYBE_WebContentsHasFocus) {
   CloseShellWindow(window);
 }
 
-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

chrome/browser/extensions/shell_window_registry.cc

@@ -67,9 +67,8 @@ void ShellWindowRegistry::ShellWindowIconChanged(ShellWindow* shell_window) {
 }
 
 void ShellWindowRegistry::RemoveShellWindow(ShellWindow* shell_window) {
-  size_t num_erased = shell_windows_.erase(shell_window);
-  if (num_erased > 0)
-    FOR_EACH_OBSERVER(Observer, observers_, OnShellWindowRemoved(shell_window));
+  shell_windows_.erase(shell_window);
+  FOR_EACH_OBSERVER(Observer, observers_, OnShellWindowRemoved(shell_window));
 }
 
 void ShellWindowRegistry::AddObserver(Observer* observer) {

chrome/browser/ui/cocoa/extensions/native_app_window_cocoa.mm

@@ -4,9 +4,7 @@
 
 #include "chrome/browser/ui/cocoa/extensions/native_app_window_cocoa.h"
 
-#include "base/bind.h"
 #include "base/mac/mac_util.h"
-#include "base/message_loop.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/cocoa/browser_window_utils.h"
@@ -681,13 +679,7 @@ gfx::Insets NativeAppWindowCocoa::GetFrameInsets() const {
 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().
-  MessageLoop::current()->PostTask(
-      FROM_HERE,
-      base::Bind(&ShellWindow::OnNativeClose,
-                 base::Unretained(shell_window_)));
+  shell_window_->OnNativeClose();
 }
 
 void NativeAppWindowCocoa::WindowDidBecomeKey() {

chrome/browser/ui/extensions/shell_window.cc

@@ -244,11 +244,6 @@ ShellWindow::~ShellWindow() {
   chrome::EndKeepAlive();
 }
 
-void ShellWindow::Close() {
-  extensions::ShellWindowRegistry::Get(profile_)->RemoveShellWindow(this);
-  native_app_window_->Close();
-}
-
 void ShellWindow::RequestMediaAccessPermission(
     content::WebContents* web_contents,
     const content::MediaStreamRequest& request,
@@ -348,16 +343,9 @@ void ShellWindow::RequestToLockMouse(WebContents* web_contents,
 }
 
 void ShellWindow::OnNativeClose() {
-  // This method is shared between the path for the user clicking the close
-  // button and the path where a close is triggered by code (e.g. by the
-  // extension being unloaded). In the latter case, this RemoveShellWindow is
-  // superfluous, since it will already have been removed, but the call is
-  // idempotent so it's harmless the second time.
   extensions::ShellWindowRegistry::Get(profile_)->RemoveShellWindow(this);
-  if (extension_) {
-    content::RenderViewHost* rvh = web_contents_->GetRenderViewHost();
-    rvh->Send(new ExtensionMsg_AppWindowClosed(rvh->GetRoutingID()));
-  }
+  content::RenderViewHost* rvh = web_contents_->GetRenderViewHost();
+  rvh->Send(new ExtensionMsg_AppWindowClosed(rvh->GetRoutingID()));
   delete this;
 }
 
@@ -499,7 +487,7 @@ void ShellWindow::UpdateExtensionAppIcon() {
 
 void ShellWindow::CloseContents(WebContents* contents) {
   DCHECK(contents == web_contents_);
-  Close();
+  native_app_window_->Close();
 }
 
 bool ShellWindow::ShouldSuppressDialogs() {
@@ -573,16 +561,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";

chrome/browser/ui/extensions/shell_window.h

@@ -133,11 +133,6 @@ class ShellWindow : public content::NotificationObserver,
   // per platform). Public users of ShellWindow should use ShellWindow::Create.
   void Init(const GURL& url, const CreateParams& params);
 
-  // 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();
-
   // content::WebContentsObserver implementation.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 

chrome/browser/ui/gtk/extensions/native_app_window_gtk.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/ui/gtk/extensions/native_app_window_gtk.h"
 
-#include "base/message_loop.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/gtk/extensions/extension_keybinding_registry_gtk.h"
@@ -185,15 +184,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().
-  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() {