Straighten out the way the system menu works a bit. We no longer create the menu every time it is shown (or reset it). Resetting it caused some system-wide system menu items to be voted off the island (e.g. nView desktop manager, and some other third party addins). Resetting also caused some of the NC overpainting issues to be worse.

This simplifies things a bit by creating the menu at frame creation time, then when the menu is run doing enabling only.
Review URL: http://codereview.chromium.org/7497

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

7 files changed, 32 insertions, 41 deletions

diff --git a/chrome/browser/browser_commands.cc b/chrome/browser/browser_commands.cc
index 20bba20..37eebe8 100644
--- a/chrome/browser/browser_commands.cc
+++ b/chrome/browser/browser_commands.cc
@@ -104,6 +104,7 @@ void Browser::InitCommandState() {
   controller_.UpdateCommandEnabled(IDC_SHOW_HISTORY, true);
   controller_.UpdateCommandEnabled(IDC_SHOW_BOOKMARKS_BAR, true);
   controller_.UpdateCommandEnabled(IDC_SHOW_DOWNLOADS, true);
+  controller_.UpdateCommandEnabled(IDC_ENCODING, true);
   controller_.UpdateCommandEnabled(IDC_ENCODING_AUTO_DETECT, true);
   controller_.UpdateCommandEnabled(IDC_ENCODING_UTF8, true);
   controller_.UpdateCommandEnabled(IDC_ENCODING_UTF16LE, true);
diff --git a/chrome/browser/views/frame/aero_glass_frame.cc b/chrome/browser/views/frame/aero_glass_frame.cc
index f38735a..d12ac1d 100644
--- a/chrome/browser/views/frame/aero_glass_frame.cc
+++ b/chrome/browser/views/frame/aero_glass_frame.cc
@@ -102,10 +102,6 @@ void AeroGlassFrame::OnEndSession(BOOL ending, UINT logoff) {
   FrameUtil::EndSession();
 }
 
-void AeroGlassFrame::OnExitMenuLoop(bool is_track_popup_menu) {
-  browser_view_->SystemMenuEnded();
-}
-
 LRESULT AeroGlassFrame::OnMouseActivate(HWND window, UINT hittest_code,
                                         UINT message) {
   return browser_view_->ActivateAppModalDialog() ? MA_NOACTIVATEANDEAT
diff --git a/chrome/browser/views/frame/aero_glass_frame.h b/chrome/browser/views/frame/aero_glass_frame.h
index 09e391a..ee402af 100644
--- a/chrome/browser/views/frame/aero_glass_frame.h
+++ b/chrome/browser/views/frame/aero_glass_frame.h
@@ -46,7 +46,6 @@ class AeroGlassFrame : public BrowserFrame,
   // Overridden from views::ContainerWin:
   virtual void OnInitMenuPopup(HMENU menu, UINT position, BOOL is_system_menu);
   virtual void OnEndSession(BOOL ending, UINT logoff);
-  virtual void OnExitMenuLoop(bool is_track_popup_menu);
   virtual LRESULT OnMouseActivate(HWND window,
                                   UINT hittest_code,
                                   UINT message);
diff --git a/chrome/browser/views/frame/browser_view2.cc b/chrome/browser/views/frame/browser_view2.cc
index afa5f3c..70ff11d 100644
--- a/chrome/browser/views/frame/browser_view2.cc
+++ b/chrome/browser/views/frame/browser_view2.cc
@@ -228,27 +228,17 @@ SkBitmap BrowserView2::GetOTRAvatarIcon() {
 }
 
 void BrowserView2::PrepareToRunSystemMenu(HMENU menu) {
-  system_menu_.reset(new Menu(menu));
-  int insertion_index = std::max(0, system_menu_->ItemCount() - 1);
-  // We add the menu items in reverse order so that insertion_index never needs
-  // to change.
-  if (browser_->GetType() == BrowserType::TABBED_BROWSER) {
-    system_menu_->AddSeparator(insertion_index);
-    system_menu_->AddMenuItemWithLabel(insertion_index, IDC_TASKMANAGER,
-                                       l10n_util::GetString(IDS_TASKMANAGER));
-    // If it's a regular browser window with tabs, we don't add any more items,
-    // since it already has menus (Page, Chrome).
-    return;
-  } else {
-    BuildMenuForTabStriplessWindow(system_menu_.get(), insertion_index);
+  for (int i = 0; i < arraysize(kMenuLayout); ++i) {
+    int command = kMenuLayout[i].command;
+    // |command| can be zero on submenu items (IDS_ENCODING,
+    // IDS_ZOOM) and on separators.
+    if (command != 0) {
+      system_menu_->EnableMenuItemByID(command,
+                                       browser_->IsCommandEnabled(command));
+    }
   }
 }
 
-void BrowserView2::SystemMenuEnded() {
-  system_menu_.reset();
-  encoding_menu_delegate_.reset();
-}
-
 bool BrowserView2::SupportsWindowFeature(WindowFeature feature) const {
   return !!(FeaturesForBrowserType(browser_->GetType()) & feature);
 }
@@ -299,6 +289,8 @@ void BrowserView2::Init() {
         browser_->profile(), this);
   }
 #endif
+
+  InitSystemMenu();
 }
 
 void BrowserView2::Show(int command, bool adjust_to_fit) {
@@ -845,6 +837,23 @@ int BrowserView2::OnPerformDrop(const views::DropTargetEvent& event) {
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView2, private:
 
+void BrowserView2::InitSystemMenu() {
+  HMENU system_menu = GetSystemMenu(frame_->GetWindow()->GetHWND(), FALSE);
+  system_menu_.reset(new Menu(system_menu));
+  int insertion_index = std::max(0, system_menu_->ItemCount() - 1);
+  // We add the menu items in reverse order so that insertion_index never needs
+  // to change.
+  if (browser_->GetType() == BrowserType::TABBED_BROWSER) {
+    system_menu_->AddSeparator(insertion_index);
+    system_menu_->AddMenuItemWithLabel(insertion_index, IDC_TASKMANAGER,
+                                       l10n_util::GetString(IDS_TASKMANAGER));
+    // If it's a regular browser window with tabs, we don't add any more items,
+    // since it already has menus (Page, Chrome).
+  } else {
+    BuildMenuForTabStriplessWindow(system_menu_.get(), insertion_index);
+  }
+}
+
 bool BrowserView2::ShouldForwardToTabStrip(
     const views::DropTargetEvent& event) {
   if (!tabstrip_->IsVisible())
@@ -1121,12 +1130,6 @@ void BrowserView2::BuildMenuForTabStriplessWindow(Menu* menu,
       } else {
         menu->AddMenuItemWithLabel(insertion_index, command,
                                    l10n_util::GetString(kMenuLayout[i].label));
-        // |command| can be zero on submenu items (IDS_ENCODING,
-        // IDS_ZOOM) and on separators.
-        if (command != 0) {
-          menu->EnableMenuItemAt(insertion_index,
-                                 browser_->IsCommandEnabled(command));
-        }
       }
     }
   }
diff --git a/chrome/browser/views/frame/browser_view2.h b/chrome/browser/views/frame/browser_view2.h
index 5dd812d..a2d65f7 100644
--- a/chrome/browser/views/frame/browser_view2.h
+++ b/chrome/browser/views/frame/browser_view2.h
@@ -112,10 +112,6 @@ class BrowserView2 : public BrowserWindow,
   // to add or delete entries.
   void PrepareToRunSystemMenu(HMENU menu);
 
-  // Called after the system menu has ended, and disposes of the
-  // current System menu object.
-  void SystemMenuEnded();
-
   // Possible elements of the Browser window.
   enum WindowFeature {
     FEATURE_TITLEBAR = 1,
@@ -232,6 +228,9 @@ class BrowserView2 : public BrowserWindow,
   virtual int OnPerformDrop(const views::DropTargetEvent& event);
 
  private:
+  // Creates the system menu.
+  void InitSystemMenu();
+
   // Returns true if the event should be forwarded to the TabStrip. This
   // returns true if y coordinate is less than the bottom of the tab strip, and
   // is not over another child view.
diff --git a/chrome/browser/views/frame/opaque_frame.cc b/chrome/browser/views/frame/opaque_frame.cc
index 4f898d7..e8740b9 100644
--- a/chrome/browser/views/frame/opaque_frame.cc
+++ b/chrome/browser/views/frame/opaque_frame.cc
@@ -77,15 +77,9 @@ void OpaqueFrame::OnEndSession(BOOL ending, UINT logoff) {
   FrameUtil::EndSession();
 }
 
-void OpaqueFrame::OnExitMenuLoop(bool is_track_popup_menu) {
-  //browser_view_->SystemMenuEnded();
-  SetMsgHandled(FALSE);
-}
-
 void OpaqueFrame::OnInitMenuPopup(HMENU menu, UINT position,
                                   BOOL is_system_menu) {
-  //browser_view_->PrepareToRunSystemMenu(menu);
-  SetMsgHandled(FALSE);
+  browser_view_->PrepareToRunSystemMenu(menu);
 }
 
 LRESULT OpaqueFrame::OnMouseActivate(HWND window, UINT hittest_code,
diff --git a/chrome/browser/views/frame/opaque_frame.h b/chrome/browser/views/frame/opaque_frame.h
index fb531b0..f36a25a 100644
--- a/chrome/browser/views/frame/opaque_frame.h
+++ b/chrome/browser/views/frame/opaque_frame.h
@@ -45,7 +45,6 @@ class OpaqueFrame : public BrowserFrame,
   virtual bool AcceleratorPressed(views::Accelerator* accelerator);
   virtual bool GetAccelerator(int cmd_id, views::Accelerator* accelerator);
   virtual void OnEndSession(BOOL ending, UINT logoff);
-  virtual void OnExitMenuLoop(bool is_track_popup_menu);
   virtual void OnInitMenuPopup(HMENU menu, UINT position, BOOL is_system_menu);
   virtual LRESULT OnMouseActivate(HWND window,
                                   UINT hittest_code,