Issue 7045023: Simplify ProfileMenuButton.

Issue 7045023: Simplify ProfileMenuButton. (Closed)

Created:
9 years, 7 months ago by Nico

Modified:
9 years, 7 months ago

Reviewers:
Ben Goodger (Google), sky, mirandac%chromium.org

CC:
chromium-reviews, Miranda Callahan

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

Visibility:
Public.

Description

Simplify ProfileMenuButton. Currently, its click callback was on the frame, which then called a method on the button again. Instead, let the button be its own listener. No intended functionality change. BUG=83169 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=85869

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Total comments: 2

Patch Set 4 : . #

Total comments: 1

Patch Set 5 : . #

Created: 9 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+19 lines, -46 lines)			Patch
M	chrome/browser/ui/views/frame/glass_browser_frame_view.h	View	1 2	3 chunks	+1 line, -6 lines	0 comments	Download
M	chrome/browser/ui/views/frame/glass_browser_frame_view.cc	View		2 chunks	+2 lines, -8 lines	0 comments	Download
M	chrome/browser/ui/views/frame/opaque_browser_frame_view.h	View	1 2	3 chunks	+0 lines, -5 lines	0 comments	Download
M	chrome/browser/ui/views/frame/opaque_browser_frame_view.cc	View		2 chunks	+2 lines, -9 lines	0 comments	Download
M	chrome/browser/ui/views/profile_menu_button.h	View	1 2	4 chunks	+6 lines, -6 lines	0 comments	Download
M	chrome/browser/ui/views/profile_menu_button.cc	View	1 2 3 4	4 chunks	+8 lines, -12 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

mirandac%chromium.org

Far better, LGTM w/comment nit (though I'm not an OWNER here...). http://codereview.chromium.org/7045023/diff/3001/chrome/browser/ui/views/profile_menu_button.cc File chrome/browser/ui/views/profile_menu_button.cc (right): ...

9 years, 7 months ago (2011-05-19 04:05:18 UTC) #2

Nico

http://codereview.chromium.org/7045023/diff/3001/chrome/browser/ui/views/profile_menu_button.cc File chrome/browser/ui/views/profile_menu_button.cc (right): http://codereview.chromium.org/7045023/diff/3001/chrome/browser/ui/views/profile_menu_button.cc#newcode76 chrome/browser/ui/views/profile_menu_button.cc:76: // OpaqueBrowserFrameView, views::ViewMenuDelegate overrides: On 2011/05/19 04:05:18, mirandac%chromium.org wrote: ...

9 years, 7 months ago (2011-05-19 04:13:25 UTC) #3

Ben Goodger (Google)

9 years, 7 months ago (2011-05-19 04:16:21 UTC) #4

LGTM

http://codereview.chromium.org/7045023/diff/2007/chrome/browser/ui/views/prof...
File chrome/browser/ui/views/profile_menu_button.cc (right):

http://codereview.chromium.org/7045023/diff/2007/chrome/browser/ui/views/prof...
chrome/browser/ui/views/profile_menu_button.cc:78: gfx::Point menu_point(pt.x(),
nit: single line

Expand Messages | Collapse Messages