Attempt 2 at landing bookmark menus.

The build breakage was the result of a bad ifdef in MenuController,
specifically around the OnKeyDown method.

BUG=none
TEST=none
TBR=ben@chromium.org

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

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

14 files changed, 227 insertions, 65 deletions

diff --git a/views/controls/button/text_button.cc b/views/controls/button/text_button.cc
index a31f6c9..b5733c0 100644
--- a/views/controls/button/text_button.cc
+++ b/views/controls/button/text_button.cc
@@ -21,9 +21,12 @@ static const int kIconTextPadding = 5;
 static const int kPreferredPaddingHorizontal = 6;
 static const int kPreferredPaddingVertical = 5;
 
-static const SkColor kEnabledColor = SkColorSetRGB(6, 45, 117);
-static const SkColor kHighlightColor = SkColorSetARGB(200, 255, 255, 255);
-static const SkColor kDisabledColor = SkColorSetRGB(161, 161, 146);
+// static
+const SkColor TextButton::kEnabledColor = SkColorSetRGB(6, 45, 117);
+// static
+const SkColor TextButton::kHighlightColor = SkColorSetARGB(200, 255, 255, 255);
+// static
+const SkColor TextButton::kDisabledColor = SkColorSetRGB(161, 161, 146);
 
 // How long the hover fade animation should last.
 static const int kHoverAnimationDurationMs = 170;
diff --git a/views/controls/button/text_button.h b/views/controls/button/text_button.h
index e18ef59..3fbc2b4 100644
--- a/views/controls/button/text_button.h
+++ b/views/controls/button/text_button.h
@@ -7,6 +7,7 @@
 
 #include "app/gfx/font.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "views/border.h"
 #include "views/controls/button/custom_button.h"
 
@@ -102,6 +103,11 @@ class TextButton : public CustomButton {
   virtual gfx::Size GetMinimumSize();
   virtual void SetEnabled(bool enabled);
 
+  // Text colors.
+  static const SkColor kEnabledColor;
+  static const SkColor kHighlightColor;
+  static const SkColor kDisabledColor;
+
  protected:
   virtual bool OnMousePressed(const MouseEvent& e);
   virtual void Paint(gfx::Canvas* canvas);
diff --git a/views/controls/menu/menu_config_gtk.cc b/views/controls/menu/menu_config_gtk.cc
index 1930824..acb77ff 100644
--- a/views/controls/menu/menu_config_gtk.cc
+++ b/views/controls/menu/menu_config_gtk.cc
@@ -4,12 +4,23 @@
 
 #include "views/controls/menu/menu_config.h"
 
+#include "app/resource_bundle.h"
+#include "grit/app_resources.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
 namespace views {
 
 // static
 MenuConfig* MenuConfig::Create() {
   // TODO: decide what we want this to look like.
-  return new MenuConfig();
+  MenuConfig* config = new MenuConfig();
+  ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+  config->font = rb.GetFont(ResourceBundle::BaseFont);
+  config->arrow_width = rb.GetBitmapNamed(IDR_MENU_ARROW)->width();
+  // Add 4 to force some padding between check and label.
+  config->check_width = rb.GetBitmapNamed(IDR_MENU_CHECK)->width() + 4;
+  config->check_height = rb.GetBitmapNamed(IDR_MENU_CHECK)->height();
+  return config;
 }
 
 }  // namespace views
diff --git a/views/controls/menu/menu_controller.cc b/views/controls/menu/menu_controller.cc
index 7c54aa4..7f19592 100644
--- a/views/controls/menu/menu_controller.cc
+++ b/views/controls/menu/menu_controller.cc
@@ -7,6 +7,7 @@
 #include "app/gfx/canvas.h"
 #include "app/l10n_util.h"
 #include "app/os_exchange_data.h"
+#include "base/keyboard_codes.h"
 #include "base/time.h"
 #include "views/controls/menu/menu_scroll_view_container.h"
 #include "views/controls/menu/submenu_view.h"
@@ -345,10 +346,7 @@ void MenuController::OnMousePressed(SubmenuView* source,
     // event.
 #if defined(OS_WIN)
     RepostEvent(source, event);
-#else
-    // Do we really need the repost logic for linux? I tend to think not but I
-    // need to verify that
-    NOTIMPLEMENTED();
+    // NOTE: not reposting on linux seems fine.
 #endif
 
     // And close.
@@ -680,10 +678,10 @@ bool MenuController::Dispatch(const MSG& msg) {
     // NOTE: focus wasn't changed when the menu was shown. As such, don't
     // dispatch key events otherwise the focused window will get the events.
     case WM_KEYDOWN:
-      return OnKeyDown(msg);
+      return OnKeyDown(msg.wParam, msg);
 
     case WM_CHAR:
-      return OnChar(msg);
+      return !SelectByChar(static_cast<wchar_t>(msg.wParam));
 
     case WM_KEYUP:
       return true;
@@ -708,35 +706,65 @@ bool MenuController::Dispatch(const MSG& msg) {
   return !exit_all_;
 }
 
-bool MenuController::OnKeyDown(const MSG& msg) {
+#else
+bool MenuController::Dispatch(GdkEvent* event) {
+  gtk_main_do_event(event);
+
+  if (exit_all_)
+    return false;
+
+  switch (event->type) {
+    case GDK_KEY_PRESS: {
+      if (!OnKeyDown(event->key.keyval))
+        return false;
+      guint32 keycode = gdk_keyval_to_unicode(event->key.keyval);
+      if (keycode)
+        return !SelectByChar(keycode);
+      return true;
+    }
+
+    default:
+      break;
+  }
+
+  return !exit_all_;
+}
+#endif
+
+bool MenuController::OnKeyDown(int key_code
+#if defined(OS_WIN)
+                               , const MSG& msg
+#else
+#endif
+                               ) {
   DCHECK(blocking_run_);
 
-  switch (msg.wParam) {
-    case VK_UP:
+  switch (key_code) {
+    case base::VKEY_UP:
       IncrementSelection(-1);
       break;
 
-    case VK_DOWN:
+    case base::VKEY_DOWN:
       IncrementSelection(1);
       break;
 
     // Handling of VK_RIGHT and VK_LEFT is different depending on the UI
     // layout.
-    case VK_RIGHT:
+    case base::VKEY_RIGHT:
       if (l10n_util::TextDirection() == l10n_util::RIGHT_TO_LEFT)
         CloseSubmenu();
       else
         OpenSubmenuChangeSelectionIfCan();
       break;
 
-    case VK_LEFT:
+    case base::VKEY_LEFT:
       if (l10n_util::TextDirection() == l10n_util::RIGHT_TO_LEFT)
         OpenSubmenuChangeSelectionIfCan();
       else
         CloseSubmenu();
       break;
 
-    case VK_RETURN:
+    case base::VKEY_RETURN:
       if (pending_state_.item) {
         if (pending_state_.item->HasSubmenu()) {
           OpenSubmenuChangeSelectionIfCan();
@@ -747,7 +775,7 @@ bool MenuController::OnKeyDown(const MSG& msg) {
       }
       break;
 
-    case VK_ESCAPE:
+    case base::VKEY_ESCAPE:
       if (!state_.item->GetParentMenuItem() ||
           (!state_.item->GetParentMenuItem()->GetParentMenuItem() &&
            (!state_.item->HasSubmenu() ||
@@ -760,32 +788,20 @@ bool MenuController::OnKeyDown(const MSG& msg) {
       }
       break;
 
+#if defined(OS_WIN)
     case VK_APPS:
       break;
+#endif
 
     default:
+#if defined(OS_WIN)
       TranslateMessage(&msg);
+#endif
       break;
   }
   return true;
 }
 
-bool MenuController::OnChar(const MSG& msg) {
-  DCHECK(blocking_run_);
-
-  return !SelectByChar(static_cast<wchar_t>(msg.wParam));
-}
-#else
-bool MenuController::Dispatch(GdkEvent* event) {
-  gtk_main_do_event(event);
-  if (exit_all_)
-    return false;
-
-  NOTIMPLEMENTED();
-  return !exit_all_;
-}
-#endif
-
 MenuController::MenuController(bool blocking)
     : blocking_run_(blocking),
       showing_(false),
diff --git a/views/controls/menu/menu_controller.h b/views/controls/menu/menu_controller.h
index 2512ede..ac84af5 100644
--- a/views/controls/menu/menu_controller.h
+++ b/views/controls/menu/menu_controller.h
@@ -158,15 +158,19 @@ class MenuController : public MessageLoopForUI::Dispatcher {
   // if the message is such that the menu should be closed.
   virtual bool Dispatch(const MSG& msg);
 
-  // Key processing. The return value of these is returned from Dispatch.
-  // In other words, if these return false (which they do if escape was
-  // pressed, or a matching mnemonic was found) the message loop returns.
-  bool OnKeyDown(const MSG& msg);
-  bool OnChar(const MSG& msg);
 #else
   virtual bool Dispatch(GdkEvent* event);
 #endif
 
+  // Key processing. The return value of this is returned from Dispatch.
+  // In other words, if this returns false (which happens if escape was
+  // pressed, or a matching mnemonic was found) the message loop returns.
+#if defined(OS_WIN)
+  bool OnKeyDown(int key_code, const MSG& msg);
+#else
+  bool OnKeyDown(int key_code);
+#endif
+
   // Creates a MenuController. If blocking is true, Run blocks the caller
   explicit MenuController(bool blocking);
 
diff --git a/views/controls/menu/menu_host_gtk.cc b/views/controls/menu/menu_host_gtk.cc
index b87aa23..9990aab 100644
--- a/views/controls/menu/menu_host_gtk.cc
+++ b/views/controls/menu/menu_host_gtk.cc
@@ -17,7 +17,8 @@ namespace views {
 MenuHost::MenuHost(SubmenuView* submenu)
     : WidgetGtk(WidgetGtk::TYPE_POPUP),
       closed_(false),
-      submenu_(submenu) {
+      submenu_(submenu),
+      did_pointer_grab_(false) {
   // TODO(sky): make sure this is needed.
   GdkModifierType current_event_mod;
   if (gtk_get_current_event_state(&current_event_mod)) {
@@ -39,7 +40,26 @@ void MenuHost::Init(gfx::NativeView parent,
   // TODO(sky): see if there is some way to show without changing focus.
   Show();
   if (do_capture) {
+    // Release the current grab.
+    GtkWidget* current_grab_window = gtk_grab_get_current();
+    if (current_grab_window)
+      gtk_grab_remove(current_grab_window);
+
+    // Make sure all app mouse events are targetted at us only.
     DoGrab();
+
+    // And do a grab.
+    // NOTE: we do this to ensure we get mouse events from other apps, a grab
+    // done with gtk_grab_add doesn't get events from other apps.
+    GdkGrabStatus grab_status =
+        gdk_pointer_grab(window_contents()->window, FALSE,
+                         static_cast<GdkEventMask>(
+                             GDK_BUTTON_PRESS_MASK | GDK_BUTTON_RELEASE_MASK |
+                             GDK_POINTER_MOTION_MASK),
+                         NULL, NULL, GDK_CURRENT_TIME);
+    did_pointer_grab_ = (grab_status == GDK_GRAB_SUCCESS);
+    DCHECK(did_pointer_grab_);
+    // need keyboard grab.
 #ifdef DEBUG_MENU
     DLOG(INFO) << "Doing capture";
 #endif
@@ -61,8 +81,8 @@ void MenuHost::Hide() {
   // remove them so that View doesn't try to access deleted objects.
   static_cast<MenuHostRootView*>(GetRootView())->suspend_events();
   GetRootView()->RemoveAllChildViews(false);
-  closed_ = true;
   ReleaseGrab();
+  closed_ = true;
   WidgetGtk::Hide();
 }
 
@@ -80,14 +100,16 @@ RootView* MenuHost::CreateRootView() {
   return new MenuHostRootView(this, submenu_);
 }
 
-void MenuHost::OnCancelMode() {
-  // TODO(sky): see if there is an equivalent to this.
-  if (!closed_) {
-#ifdef DEBUG_MENU
-    DLOG(INFO) << "OnCanceMode, closing menu";
-#endif
+gboolean MenuHost::OnGrabBrokeEvent(GtkWidget* widget, GdkEvent* event) {
+  if (!closed_)
     submenu_->GetMenuItem()->GetMenuController()->Cancel(true);
-  }
+  return WidgetGtk::OnGrabBrokeEvent(widget, event);
+}
+
+void MenuHost::OnGrabNotify(GtkWidget* widget, gboolean was_grabbed) {
+  if (!closed_ && (widget != window_contents() || !was_grabbed))
+    submenu_->GetMenuItem()->GetMenuController()->Cancel(true);
+  WidgetGtk::OnGrabNotify(widget, was_grabbed);
 }
 
 // Overriden to return false, we do NOT want to release capture on mouse
@@ -96,4 +118,12 @@ bool MenuHost::ReleaseCaptureOnMouseReleased() {
   return false;
 }
 
+void MenuHost::ReleaseGrab() {
+  WidgetGtk::ReleaseGrab();
+  if (did_pointer_grab_) {
+    did_pointer_grab_ = false;
+    gdk_pointer_ungrab(GDK_CURRENT_TIME);
+  }
+}
+
 }  // namespace views
diff --git a/views/controls/menu/menu_host_gtk.h b/views/controls/menu/menu_host_gtk.h
index 545acb5..6e70bc9 100644
--- a/views/controls/menu/menu_host_gtk.h
+++ b/views/controls/menu/menu_host_gtk.h
@@ -30,12 +30,17 @@ class MenuHost : public WidgetGtk {
  protected:
   virtual RootView* CreateRootView();
 
-  virtual void OnCancelMode();
+  // If the grab breaks we cancel the menu.
+  virtual gboolean OnGrabBrokeEvent(GtkWidget* widget, GdkEvent* event);
+  virtual void OnGrabNotify(GtkWidget* widget, gboolean was_grabbed);
 
   // Overriden to return false, we do NOT want to release capture on mouse
   // release.
   virtual bool ReleaseCaptureOnMouseReleased();
 
+  // Overriden to also release pointer grab.
+  virtual void ReleaseGrab();
+
  private:
   // If true, we've been closed.
   bool closed_;
@@ -43,6 +48,9 @@ class MenuHost : public WidgetGtk {
   // The view we contain.
   SubmenuView* submenu_;
 
+  // Have we done a pointer grab?
+  bool did_pointer_grab_;
+
   DISALLOW_COPY_AND_ASSIGN(MenuHost);
 };
 
diff --git a/views/controls/menu/menu_item_view.cc b/views/controls/menu/menu_item_view.cc
index 6308b52..7a6056f 100644
--- a/views/controls/menu/menu_item_view.cc
+++ b/views/controls/menu/menu_item_view.cc
@@ -53,11 +53,9 @@ bool MenuItemView::allow_task_nesting_during_run_ = false;
 // static
 int MenuItemView::label_start_;
 
-// Margins between the right of the item and the label.
 // static
 int MenuItemView::item_right_margin_;
 
-// Preferred height of menu items. Reset every time a menu is run.
 // static
 int MenuItemView::pref_menu_height_;
 
@@ -189,13 +187,6 @@ void MenuItemView::Paint(gfx::Canvas* canvas) {
   Paint(canvas, false);
 }
 
-gfx::Size MenuItemView::GetPreferredSize() {
-  const gfx::Font& font = MenuConfig::instance().font;
-  return gfx::Size(
-      font.GetStringWidth(title_) + label_start_ + item_right_margin_,
-      font.height() + GetBottomMargin() + GetTopMargin());
-}
-
 MenuController* MenuItemView::GetMenuController() {
   return GetRootMenuItem()->controller_;
 }
diff --git a/views/controls/menu/menu_item_view_gtk.cc b/views/controls/menu/menu_item_view_gtk.cc
index 6f5b8e4..33e5ff4 100644
--- a/views/controls/menu/menu_item_view_gtk.cc
+++ b/views/controls/menu/menu_item_view_gtk.cc
@@ -4,13 +4,97 @@
 
 #include "views/controls/menu/menu_item_view.h"
 
-#include "base/logging.h"
+#include "app/gfx/canvas.h"
+#include "app/gfx/favicon_size.h"
+#include "app/resource_bundle.h"
+#include "grit/app_resources.h"
+#include "views/controls/button/text_button.h"
 #include "views/controls/menu/menu_config.h"
+#include "views/controls/menu/submenu_view.h"
 
 namespace views {
 
+// Background color when the menu item is selected.
+static const SkColor kSelectedBackgroundColor = SkColorSetRGB(246, 249, 253);
+
+gfx::Size MenuItemView::GetPreferredSize() {
+  const gfx::Font& font = MenuConfig::instance().font;
+  // TODO(sky): this is a workaround until I figure out why font.height()
+  // isn't returning the right thing. We really only want to include
+  // kFavIconSize if we're showing icons.
+  int content_height = std::max(kFavIconSize, font.height());
+  return gfx::Size(
+      font.GetStringWidth(title_) + label_start_ + item_right_margin_,
+      content_height + GetBottomMargin() + GetTopMargin());
+}
+
 void MenuItemView::Paint(gfx::Canvas* canvas, bool for_drag) {
-  NOTIMPLEMENTED();
+  const MenuConfig& config = MenuConfig::instance();
+  bool render_selection =
+      (!for_drag && IsSelected() &&
+       parent_menu_item_->GetSubmenu()->GetShowSelection(this));
+
+  int icon_x = config.item_left_margin;
+  int top_margin = GetTopMargin();
+  int bottom_margin = GetBottomMargin();
+  int icon_y = top_margin + (height() - config.item_top_margin -
+                             bottom_margin - config.check_height) / 2;
+  int icon_height = config.check_height;
+  int available_height = height() - top_margin - bottom_margin;
+
+  // Render the background. As MenuScrollViewContainer draws the background, we
+  // only need the background when we want it to look different, as when we're
+  // selected.
+  if (render_selection)
+    canvas->drawColor(kSelectedBackgroundColor, SkXfermode::kSrc_Mode);
+
+  // Render the check.
+  if (type_ == CHECKBOX && GetDelegate()->IsItemChecked(GetCommand())) {
+    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+    SkBitmap* check = rb.GetBitmapNamed(IDR_MENU_CHECK);
+    // Don't use config.check_width here as it's padded to force more padding.
+    gfx::Rect check_bounds(icon_x, icon_y, check->width(), icon_height);
+    AdjustBoundsForRTLUI(&check_bounds);
+    canvas->DrawBitmapInt(*check, check_bounds.x(), check_bounds.y());
+  }
+
+  // Render the foreground.
+  SkColor fg_color =
+      IsEnabled() ? TextButton::kEnabledColor : TextButton::kDisabledColor;
+  int width = this->width() - item_right_margin_ - label_start_;
+  const gfx::Font& font = MenuConfig::instance().font;
+  gfx::Rect text_bounds(label_start_, top_margin +
+                        (available_height - font.height()) / 2, width,
+                        font.height());
+  text_bounds.set_x(MirroredLeftPointForRect(text_bounds));
+  canvas->DrawStringInt(GetTitle(), font, fg_color,
+                        text_bounds.x(), text_bounds.y(), text_bounds.width(),
+                        text_bounds.height(),
+                        GetRootMenuItem()->GetDrawStringFlags());
+
+  // Render the icon.
+  if (icon_.width() > 0) {
+    gfx::Rect icon_bounds(config.item_left_margin,
+                          top_margin + (height() - top_margin -
+                          bottom_margin - icon_.height()) / 2,
+                          icon_.width(),
+                          icon_.height());
+    icon_bounds.set_x(MirroredLeftPointForRect(icon_bounds));
+    canvas->DrawBitmapInt(icon_, icon_bounds.x(), icon_bounds.y());
+  }
+
+  // Render the submenu indicator (arrow).
+  if (HasSubmenu()) {
+    gfx::Rect arrow_bounds(this->width() - item_right_margin_ +
+                           config.label_to_arrow_padding,
+                           top_margin + (available_height -
+                                         config.arrow_width) / 2,
+                           config.arrow_width, height());
+    AdjustBoundsForRTLUI(&arrow_bounds);
+    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+    canvas->DrawBitmapInt(*rb.GetBitmapNamed(IDR_MENU_ARROW),
+                          arrow_bounds.x(), arrow_bounds.y());
+  }
 }
 
 }  // namespace views
diff --git a/views/controls/menu/menu_item_view_win.cc b/views/controls/menu/menu_item_view_win.cc
index 8f3812c..8ec80eb3 100644
--- a/views/controls/menu/menu_item_view_win.cc
+++ b/views/controls/menu/menu_item_view_win.cc
@@ -19,6 +19,13 @@ using gfx::NativeTheme;
 
 namespace views {
 
+gfx::Size MenuItemView::GetPreferredSize() {
+  const gfx::Font& font = MenuConfig::instance().font;
+  return gfx::Size(
+      font.GetStringWidth(title_) + label_start_ + item_right_margin_,
+      font.height() + GetBottomMargin() + GetTopMargin());
+}
+
 void MenuItemView::Paint(gfx::Canvas* canvas, bool for_drag) {
   const MenuConfig& config = MenuConfig::instance();
   bool render_selection =
diff --git a/views/controls/menu/menu_scroll_view_container.cc b/views/controls/menu/menu_scroll_view_container.cc
index 45a4777..b50e926 100644
--- a/views/controls/menu/menu_scroll_view_container.cc
+++ b/views/controls/menu/menu_scroll_view_container.cc
@@ -185,7 +185,8 @@ void MenuScrollViewContainer::Paint(gfx::Canvas* canvas) {
       NativeTheme::MENU, dc, MENU_POPUPBACKGROUND, 0, &bounds);
   canvas->endPlatformPaint();
 #else
-  NOTIMPLEMENTED();
+  // This is the same as COLOR_TOOLBAR.
+  canvas->drawColor(SkColorSetRGB(210, 225, 246), SkXfermode::kSrc_Mode);
 #endif
 }
 
diff --git a/views/widget/widget_gtk.h b/views/widget/widget_gtk.h
index 8f356fb..7de1492 100644
--- a/views/widget/widget_gtk.h
+++ b/views/widget/widget_gtk.h
@@ -156,6 +156,9 @@ class WidgetGtk : public Widget, public MessageLoopForUI::Observer {
 
   void set_mouse_down(bool mouse_down) { is_mouse_down_ = mouse_down; }
 
+  // Do we own the mouse grab?
+  bool has_capture() const { return has_capture_; }
+
   // Returns whether capture should be released on mouse release. The default
   // is true.
   virtual bool ReleaseCaptureOnMouseReleased() { return true; }
@@ -164,7 +167,7 @@ class WidgetGtk : public Widget, public MessageLoopForUI::Observer {
   void DoGrab();
 
   // Releases a grab done by this widget.
-  void ReleaseGrab();
+  virtual void ReleaseGrab();
 
   // Sets the WindowGtk in the userdata section of the widget.
   static void SetWindowForNative(GtkWidget* widget, WindowGtk* window);
diff --git a/views/window/custom_frame_view.cc b/views/window/custom_frame_view.cc
index 8f0999b..3434485 100644
--- a/views/window/custom_frame_view.cc
+++ b/views/window/custom_frame_view.cc
@@ -514,7 +514,6 @@ void CustomFrameView::LayoutWindowControls() {
     pushed_part = IDR_CLOSE_SA_P;
   }
 
-  /*
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
 
   close_button_->SetImage(CustomButton::BS_NORMAL,
@@ -523,7 +522,6 @@ void CustomFrameView::LayoutWindowControls() {
                           rb.GetBitmapNamed(hot_part));
   close_button_->SetImage(CustomButton::BS_PUSHED,
                           rb.GetBitmapNamed(pushed_part));
-  */
 }
 
 void CustomFrameView::LayoutTitleBar() {
diff --git a/views/window/window_gtk.cc b/views/window/window_gtk.cc
index 1643e1b..b21d4dd 100644
--- a/views/window/window_gtk.cc
+++ b/views/window/window_gtk.cc
@@ -115,7 +115,7 @@ void WindowGtk::Show() {
 }
 
 void WindowGtk::HideWindow() {
-  NOTIMPLEMENTED();
+  Hide();
 }
 
 void WindowGtk::PushForceHidden() {