Issue 14150005: views: Finally get rid of the deprecated Menu2 API.

Issue 14150005: views: Finally get rid of the deprecated Menu2 API. (Closed)

Created:
7 years, 8 months ago by tfarina

Modified:
7 years, 8 months ago

Reviewers:
msw, sky

CC:
chromium-reviews, Ben Goodger (Google), msw

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

views: Finally get rid of the deprecated Menu2 API. BUG=None TEST=None R=sky@chromium.org,msw@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194356

Patch Set 5 : rm comment #

Total comments: 2

Created: 7 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+75 lines, -288 lines)			Patch
M	chrome/browser/ui/views/frame/system_menu_insertion_delegate_win.h	View	1 2	1 chunk	+7 lines, -4 lines	0 comments	Download
M	chrome/browser/ui/views/frame/system_menu_insertion_delegate_win.cc	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
D	ui/views/controls/menu/menu_2.h	View		1 chunk	+0 lines, -96 lines	0 comments	Download
D	ui/views/controls/menu/menu_2.cc	View		1 chunk	+0 lines, -61 lines	0 comments	Download
A	ui/views/controls/menu/menu_insertion_delegate.h	View		1 chunk	+21 lines, -0 lines	0 comments	Download
D	ui/views/controls/menu/menu_wrapper.h	View		1 chunk	+0 lines, -82 lines	0 comments	Download
M	ui/views/controls/menu/native_menu_win.h	View	1 2 3 4	1 chunk	+34 lines, -17 lines	0 comments	Download
M	ui/views/controls/menu/native_menu_win.cc	View		9 chunks	+9 lines, -19 lines	2 comments	Download
M	ui/views/views.gyp	View	1 2 3 4	4 chunks	+3 lines, -8 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

msw

I actually don't know anything about Menu2. I'll defer to Scott (and maybe Ben?). I ...

7 years, 8 months ago (2013-04-15 18:05:07 UTC) #2

sky

LGTM https://codereview.chromium.org/14150005/diff/7002/ui/views/controls/menu/native_menu_win.h File ui/views/controls/menu/native_menu_win.h (right): https://codereview.chromium.org/14150005/diff/7002/ui/views/controls/menu/native_menu_win.h#newcode30 ui/views/controls/menu/native_menu_win.h:30: // TODO(beng): rename to MenuWin once the old ...

7 years, 8 months ago (2013-04-15 21:00:33 UTC) #3

tfarina

https://codereview.chromium.org/14150005/diff/7002/ui/views/controls/menu/native_menu_win.h File ui/views/controls/menu/native_menu_win.h (right): https://codereview.chromium.org/14150005/diff/7002/ui/views/controls/menu/native_menu_win.h#newcode30 ui/views/controls/menu/native_menu_win.h:30: // TODO(beng): rename to MenuWin once the old class ...

7 years, 8 months ago (2013-04-15 22:23:11 UTC) #4

tfarina

Committed patchset #5 manually as r194356 (presubmit successful).

7 years, 8 months ago (2013-04-16 13:11:45 UTC) #5

tfarina

https://codereview.chromium.org/14150005/diff/16001/ui/views/controls/menu/native_menu_win.cc File ui/views/controls/menu/native_menu_win.cc (right): https://codereview.chromium.org/14150005/diff/16001/ui/views/controls/menu/native_menu_win.cc#newcode630 ui/views/controls/menu/native_menu_win.cc:630: new NativeMenuWin(model_->GetSubmenuModelAt(model_index), NULL)); Mike, I think we need to ...

7 years, 8 months ago (2013-04-18 19:01:29 UTC) #6

tfarina

7 years, 8 months ago (2013-04-18 21:29:50 UTC) #7

Message was sent while issue was closed.

https://codereview.chromium.org/14150005/diff/16001/ui/views/controls/menu/na...
File ui/views/controls/menu/native_menu_win.cc (right):

https://codereview.chromium.org/14150005/diff/16001/ui/views/controls/menu/na...
ui/views/controls/menu/native_menu_win.cc:630: new
NativeMenuWin(model_->GetSubmenuModelAt(model_index), NULL));
On 2013/04/18 19:01:29, tfarina wrote:
> Mike, I think we need to pass |this| here to avoid the crash.
> 
> What you think?
Sorry, I'm wrong, the second parameter of NativeMenuWin is a HWND. This probably
needs to be |system_menu_for_|. Does that sounds right?

Expand Messages | Collapse Messages