Fix leaks caused by main menu singleton.

 Simply creating & deleting it per window instance for now.
 No delay on dev machine, but may cause a delay on netbook.
 MainMenu(aka AppMenu) is to be rewritten, so we'll revisit  the performance issue (if any) later.

BUG=32624,37334
TEST=no crash on exit.

Review URL: http://codereview.chromium.org/668076

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

4 files changed, 37 insertions, 88 deletions

diff --git a/chrome/browser/chromeos/app_launcher.cc b/chrome/browser/chromeos/app_launcher.cc
index 0dc8b55..e0f9090 100644
--- a/chrome/browser/chromeos/app_launcher.cc
+++ b/chrome/browser/chromeos/app_launcher.cc
@@ -117,6 +117,8 @@ class NavigationBar : public views::View,
   }
 
   virtual ~NavigationBar() {
+    if (location_entry_view_->native_view())
+      location_entry_view_->Detach();
   }
 
   // views::View overrides.
@@ -285,28 +287,6 @@ void AppLauncher::BubbleContainer::Layout() {
 ////////////////////////////////////////////////////////////////////////////////
 // AppLauncher
 
-// static
-void AppLauncher::Show(Browser* browser) {
-  AppLauncher::Get()->ShowImpl(browser);
-}
-
-// static
-void AppLauncher::ScheduleCreation() {
-  MessageLoop::current()->PostDelayedTask(FROM_HERE, new LoadTask(), 5000);
-}
-
-AppLauncher::~AppLauncher() {
-  // NOTE: we leak the contents_rvh_ and popup_ as by the time we get here the
-  // message loop and notification services have been shutdown.
-  // TODO(sky): fix this.
-  // contents_rvh_->Shutdown();
-  // popup_->CloseNow();
-  // if (location_entry_view_->native_view())
-  //   location_entry_view_->Detach();
-  // etc
-  ActiveWindowWatcherX::RemoveObserver(this);
-}
-
 AppLauncher::AppLauncher()
     : browser_(NULL),
       popup_(NULL),
@@ -365,9 +345,10 @@ AppLauncher::AppLauncher()
   ActiveWindowWatcherX::AddObserver(this);
 }
 
-// static
-AppLauncher* AppLauncher::Get() {
-  return Singleton<AppLauncher>::get();
+AppLauncher::~AppLauncher() {
+  contents_rvh_->Shutdown();
+  popup_->CloseNow();
+  ActiveWindowWatcherX::RemoveObserver(this);
 }
 
 void AppLauncher::Update(Browser* browser) {
@@ -387,7 +368,7 @@ void AppLauncher::Update(Browser* browser) {
   top_container_->Layout();
 }
 
-void AppLauncher::ShowImpl(Browser* browser) {
+void AppLauncher::Show(Browser* browser) {
   Cleanup();
 
   Update(browser);
@@ -513,13 +494,4 @@ void AppLauncher::TabContentsDelegateImpl::OpenURLFromTab(
   app_launcher_->Hide();
 }
 
-// LoadTask -------------------------------------------------------------------
-
-void AppLauncher::LoadTask::Run() {
-  if (BrowserList::begin() == BrowserList::end())
-    return;  // No browser are around. Generally only happens during testing.
-
-  AppLauncher::Get();
-}
-
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/app_launcher.h b/chrome/browser/chromeos/app_launcher.h
index 548d312..f9fbdbc 100644
--- a/chrome/browser/chromeos/app_launcher.h
+++ b/chrome/browser/chromeos/app_launcher.h
@@ -9,7 +9,6 @@
 
 #include "app/active_window_watcher_x.h"
 #include "base/scoped_ptr.h"
-#include "base/singleton.h"
 #include "base/task.h"
 #include "chrome/browser/renderer_host/render_view_host_delegate.h"
 #include "chrome/browser/tab_contents/render_view_host_delegate_helper.h"
@@ -56,37 +55,18 @@ class NavigationBar;
 //
 // When a new url is opened, or the user clicks outsides the bounds of the
 // widget the app launcher is closed.
-//
-// AppLauncher manages its own lifetime and currently creates one instance for
-// the life of the browser. This is done to make sure we have the html page
-// loaded when the user clicks on it.
 class AppLauncher : public RenderViewHostDelegate,
                     public RenderViewHostDelegate::View,
                     public ActiveWindowWatcherX::Observer,
                     public views::AcceleratorTarget {
  public:
-  // Shows the app launcher.
-  static void Show(Browser* browser);
-
-  // Schedules creation of the shared AppLauncher.
-  static void ScheduleCreation();
-
+  AppLauncher();
   ~AppLauncher();
 
- private:
-  friend struct DefaultSingletonTraits<AppLauncher>;
-
-  // Task used to ask for the AppLauncher instance. This is scheduled from
-  // ScheduleCreation.
-  class LoadTask : public Task {
-   public:
-    LoadTask() {}
-    virtual void Run();
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(LoadTask);
-  };
+  // Shows the menu.
+  void Show(Browser* browser);
 
+ private:
   // TabContentsDelegate and RenderViewHostDelegate::View have some methods
   // in common (with differing signatures). The TabContentsDelegate methods are
   // implemented by this class.
@@ -154,14 +134,6 @@ class AppLauncher : public RenderViewHostDelegate,
   friend class TabContentsDelegateImpl;
   friend class TopContainer;
 
-  AppLauncher();
-
-  // Returns the single AppLauncher instance.
-  static AppLauncher* Get();
-
-  // Shows the app launcher for the specified browser.
-  void ShowImpl(Browser* browser);
-
   // Hides the app launcher.
   void Hide();
 
diff --git a/chrome/browser/chromeos/frame/browser_view.cc b/chrome/browser/chromeos/frame/browser_view.cc
index 4428dea..032cf2d 100644
--- a/chrome/browser/chromeos/frame/browser_view.cc
+++ b/chrome/browser/chromeos/frame/browser_view.cc
@@ -136,7 +136,7 @@ class BrowserViewLayout : public ::BrowserViewLayout {
   // BrowserViewLayout overrides:
 
   void Installed(views::View* host) {
-    main_menu_ = NULL;
+    main_menu_button_ = NULL;
     compact_navigation_bar_ = NULL;
     status_area_ = NULL;
     spacer_ = NULL;
@@ -151,7 +151,7 @@ class BrowserViewLayout : public ::BrowserViewLayout {
         spacer_ = view;
         break;
       case VIEW_ID_MAIN_MENU:
-        main_menu_ = view;
+        main_menu_button_ = view;
         break;
       case VIEW_ID_STATUS_AREA:
         status_area_ = static_cast<chromeos::StatusAreaView*>(view);
@@ -171,7 +171,7 @@ class BrowserViewLayout : public ::BrowserViewLayout {
   virtual int LayoutTabStrip() {
     if (browser_view_->IsFullscreen() ||
         !browser_view_->IsTabStripVisible()) {
-      main_menu_->SetVisible(false);
+      main_menu_button_->SetVisible(false);
       compact_navigation_bar_->SetVisible(false);
       status_area_->SetVisible(false);
       otr_avatar_icon_->SetVisible(false);
@@ -213,9 +213,9 @@ class BrowserViewLayout : public ::BrowserViewLayout {
   bool IsPointInViewsInTitleArea(const gfx::Point& point)
       const {
     gfx::Point point_in_main_menu_coords(point);
-    views::View::ConvertPointToView(browser_view_, main_menu_,
+    views::View::ConvertPointToView(browser_view_, main_menu_button_,
                                     &point_in_main_menu_coords);
-    if (main_menu_->HitTest(point_in_main_menu_coords))
+    if (main_menu_button_->HitTest(point_in_main_menu_coords))
       return true;
 
     gfx::Point point_in_status_area_coords(point);
@@ -242,7 +242,7 @@ class BrowserViewLayout : public ::BrowserViewLayout {
     if (bounds.IsEmpty()) {
       return 0;
     }
-    main_menu_->SetVisible(true);
+    main_menu_button_->SetVisible(true);
     compact_navigation_bar_->SetVisible(
         chromeos_browser_view()->is_compact_style());
     tabstrip_->SetVisible(true);
@@ -265,9 +265,9 @@ class BrowserViewLayout : public ::BrowserViewLayout {
     */
 
     // Layout main menu before tab strip.
-    gfx::Size main_menu_size = main_menu_->GetPreferredSize();
-    main_menu_->SetBounds(0, bounds.y(),
-                          main_menu_size.width(), bounds.height());
+    gfx::Size main_menu_size = main_menu_button_->GetPreferredSize();
+    main_menu_button_->SetBounds(0, bounds.y(),
+                                 main_menu_size.width(), bounds.height());
 
     status_area_->Update();
     // Layout status area after tab strip.
@@ -327,7 +327,7 @@ class BrowserViewLayout : public ::BrowserViewLayout {
   }
 
 
-  views::View* main_menu_;
+  views::View* main_menu_button_;
   chromeos::StatusAreaView* status_area_;
   views::View* compact_navigation_bar_;
   views::View* spacer_;
@@ -339,6 +339,7 @@ class BrowserViewLayout : public ::BrowserViewLayout {
 BrowserView::BrowserView(Browser* browser)
     : ::BrowserView(browser),
       main_menu_(NULL),
+      main_menu_button_(NULL),
       status_area_(NULL),
       compact_navigation_bar_(NULL),
       // Standard style is default.
@@ -357,15 +358,15 @@ BrowserView::~BrowserView() {
 
 void BrowserView::Init() {
   ::BrowserView::Init();
-  main_menu_ = new views::ImageButton(this);
-  main_menu_->SetID(VIEW_ID_MAIN_MENU);
+  main_menu_button_ = new views::ImageButton(this);
+  main_menu_button_->SetID(VIEW_ID_MAIN_MENU);
   ThemeProvider* theme_provider =
       frame()->GetThemeProviderForFrame();
   SkBitmap* image = theme_provider->GetBitmapNamed(IDR_APP_LAUNCHER_BUTTON);
-  main_menu_->SetImage(views::CustomButton::BS_NORMAL, image);
-  main_menu_->SetImage(views::CustomButton::BS_HOT, image);
-  main_menu_->SetImage(views::CustomButton::BS_PUSHED, image);
-  AddChildView(main_menu_);
+  main_menu_button_->SetImage(views::CustomButton::BS_NORMAL, image);
+  main_menu_button_->SetImage(views::CustomButton::BS_HOT, image);
+  main_menu_button_->SetImage(views::CustomButton::BS_PUSHED, image);
+  AddChildView(main_menu_button_);
 
   compact_location_bar_host_.reset(
       new chromeos::CompactLocationBarHost(this));
@@ -387,7 +388,7 @@ void BrowserView::Init() {
   AddChildView(spacer_);
 
   InitSystemMenu();
-  chromeos::AppLauncher::ScheduleCreation();
+  main_menu_.reset(new AppLauncher());
 
   // The ContextMenuController has to be set to a NonClientView but
   // not to a NonClientFrameView because a TabStrip is not a child of
@@ -458,7 +459,7 @@ void BrowserView::ShowBookmarkBubble(const GURL& url, bool already_bookmarked) {
 // views::ButtonListener overrides.
 void BrowserView::ButtonPressed(views::Button* sender,
                                 const views::Event& event) {
-  chromeos::AppLauncher::Show(browser());
+  main_menu_->Show(browser());
 }
 
 // views::ContextMenuController overrides.
diff --git a/chrome/browser/chromeos/frame/browser_view.h b/chrome/browser/chromeos/frame/browser_view.h
index e4a57ef..223c190 100644
--- a/chrome/browser/chromeos/frame/browser_view.h
+++ b/chrome/browser/chromeos/frame/browser_view.h
@@ -26,13 +26,14 @@ class BrowserStatusAreaView;
 class CompactLocationBar;
 class CompactLocationBarHost;
 class CompactNavigationBar;
+class AppLauncher;
 class StatusAreaButton;
 
 // chromeos::BrowserView adds ChromeOS specific controls and menus to a
 // BrowserView created with Browser::TYPE_NORMAL. This extender adds controls
 // to the title bar as follows:
 //                  ____  __ __
-//      [MainMenu] /    \   \  \     [StatusArea]
+//      [AppLauncher] /    \   \  \     [StatusArea]
 //
 // and adds the system context menu to the remaining arae of the titlebar.
 class BrowserView : public ::BrowserView,
@@ -93,8 +94,11 @@ class BrowserView : public ::BrowserView,
  private:
   void InitSystemMenu();
 
-  // Main menu button.
-  views::ImageButton* main_menu_;
+  // AppLauncher instance.
+  scoped_ptr<AppLauncher> main_menu_;
+
+  // AppLauncher button.
+  views::ImageButton* main_menu_button_;
 
   // Status Area view.
   BrowserStatusAreaView* status_area_;