Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4)

Issue 7746012: Fixes the touch build from patch @ 98179 (Closed)

Created:
9 years, 4 months ago by sky
Modified:
9 years, 4 months ago
CC:
chromium-reviews, dhollowa
Visibility:
Public.

Description

Fixes the touch build from patch @ 98179. I missed a touch class using MenuItemView. BUG=57890 TEST=thorougly test all menus in chrome: bookmarks, wrench menu, context menu on page, extension menus... TBR=ben@chromium.org R=ben@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98183

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -9 lines) Patch
M views/controls/menu/native_menu_x.h View 3 chunks +5 lines, -2 lines 0 comments Download
M views/controls/menu/native_menu_x.cc View 3 chunks +11 lines, -7 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
sky
9 years, 4 months ago (2011-08-25 04:17:10 UTC) #1
Ben Goodger (Google)
9 years, 4 months ago (2011-08-25 15:26:39 UTC) #2
LGTM

On Wed, Aug 24, 2011 at 9:17 PM, <sky@chromium.org> wrote:

> Reviewers: Ben Goodger (Google),
>
> Description:
> Fixes the touch build from patch @ 98179. I missed a touch class using
> MenuItemView.
>
> BUG=57890
> TEST=thorougly test all menus in chrome: bookmarks, wrench menu,
> context menu on page, extension menus...
> TBR=ben@chromium.org
> R=ben@chromium.org
>
> Please review this at
http://codereview.chromium.**org/7746012/<http://codereview.chromium.org/7746...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M views/controls/menu/native_**menu_x.h
>  M views/controls/menu/native_**menu_x.cc
>
>
> Index: views/controls/menu/native_**menu_x.cc
> diff --git a/views/controls/menu/native_**menu_x.cc
> b/views/controls/menu/native_**menu_x.cc
> index eb960422d0a9cf4ba07f2340c8b429**948a75ca94..**
> b07389e4a3ffb56f117a6db941e1a6**d5dd10b335 100644
> --- a/views/controls/menu/native_**menu_x.cc
> +++ b/views/controls/menu/native_**menu_x.cc
> @@ -9,13 +9,15 @@
>  #include "ui/gfx/canvas_skia.h"
>  #include "ui/gfx/skia_util.h"
>  #include "views/controls/menu/menu_2.h"
> +#include "views/controls/menu/menu_**runner.h"
>  #include "views/controls/menu/submenu_**view.h"
>
>  namespace views {
>
>  NativeMenuX::NativeMenuX(**Menu2* menu)
>     : model_(menu->model()),
> -      ALLOW_THIS_IN_INITIALIZER_**LIST(root_(new MenuItemView(this))) {
> +      ALLOW_THIS_IN_INITIALIZER_**LIST(root_(new MenuItemView(this))),
> +      menu_runner_(new MenuRunner(root_)) {
>  }
>
>  NativeMenuX::~NativeMenuX() {
> @@ -23,10 +25,13 @@ NativeMenuX::~NativeMenuX() {
>
>  // MenuWrapper implementation:
>  void NativeMenuX::RunMenuAt(const gfx::Point& point, int alignment) {
> +  // TODO: this should really return the value from MenuRunner.
>   UpdateStates();
> -  root_->RunMenuAt(NULL, NULL, gfx::Rect(point, gfx::Size()),
> -      alignment == Menu2::ALIGN_TOPLEFT ? MenuItemView::TOPLEFT :
> -      MenuItemView::TOPRIGHT, true);
> +  if (menu_runner_->RunMenuAt(NULL, NULL, gfx::Rect(point, gfx::Size()),
> +          alignment == Menu2::ALIGN_TOPLEFT ? MenuItemView::TOPLEFT :
> +          MenuItemView::TOPRIGHT, MenuRunner::HAS_MNEMONICS) ==
> +      MenuRunner::MENU_DELETED)
> +    return;
>  }
>
>  void NativeMenuX::CancelMenu() {
> @@ -34,10 +39,9 @@ void NativeMenuX::CancelMenu() {
>  }
>
>  void NativeMenuX::Rebuild() {
> -  if (SubmenuView* submenu = root_->GetSubmenu()) {
> +  if (SubmenuView* submenu = root_->GetSubmenu())
>     submenu->RemoveAllChildViews(**true);
> -  }
> -  AddMenuItemsFromModel(root_.**get(), model_);
> +  AddMenuItemsFromModel(root_, model_);
>  }
>
>  void NativeMenuX::UpdateStates() {
> Index: views/controls/menu/native_**menu_x.h
> diff --git a/views/controls/menu/native_**menu_x.h
> b/views/controls/menu/native_**menu_x.h
> index 2621fb35569bfda330562bf04cb1c5**4e5257d158..**
> 34ae8944e7c6042efd8c55eed13503**25ac46940d 100644
> --- a/views/controls/menu/native_**menu_x.h
> +++ b/views/controls/menu/native_**menu_x.h
> @@ -1,4 +1,4 @@
> -// Copyright (c) 2010 The Chromium Authors. All rights reserved.
> +// Copyright (c) 2011 The Chromium Authors. All rights reserved.
>  // Use of this source code is governed by a BSD-style license that can be
>  // found in the LICENSE file.
>
> @@ -16,6 +16,8 @@ class MenuModel;
>
>  namespace views {
>
> +class MenuRunner;
> +
>  // A non-GTK implementation of MenuWrapper, used currently for touchui.
>  class NativeMenuX : public MenuWrapper,
>                     public MenuDelegate {
> @@ -46,7 +48,8 @@ class NativeMenuX : public MenuWrapper,
>
>   // The attached model and delegate. Does not assume ownership.
>   ui::MenuModel* model_;
> -  scoped_ptr<MenuItemView> root_;
> +  MenuItemView* root_;
> +  scoped_ptr<MenuRunner> menu_runner_;
>
>   DISALLOW_COPY_AND_ASSIGN(**NativeMenuX);
>  };
>
>
>

Powered by Google App Engine
This is Rietveld 408576698