Issue 2730015: Mac/clang: Uncontentious fixes.

Issue 2730015: Mac/clang: Uncontentious fixes. (Closed)

Created:
10 years, 6 months ago by Nico

Modified:
10 years, 6 months ago

Reviewers:
Mark Mentovai

CC:
chromium-reviews, Evan Martin

Visibility:
Public.

Description

Mac/clang: Uncontentious fixes. * Remove unused variables * Make types in h and cc files agree * Use subclasses if we call subclass methods * Fix one real bug (`if (a); a->foo()`) * Fix forward declarations to be correct * Don't mark some definitions with "extern" Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49570

Patch Set 1 #

Total comments: 2

Patch Set 2 : '' #

Total comments: 3

Created: 10 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+28 lines, -44 lines)			Patch
M	base/crypto/signature_creator_mac.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/app_controller_mac.mm	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/authorization_util.mm	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	chrome/browser/cocoa/bookmark_bar_folder_controller.mm	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/browser/cocoa/bookmark_button.h	View		1 chunk	+1 line, -1 line	1 comment	Download
M	chrome/browser/cocoa/bookmark_editor_base_controller.mm	View		1 chunk	+0 lines, -2 lines	0 comments	Download
M	chrome/browser/cocoa/browser_window_controller.mm	View		1 chunk	+7 lines, -6 lines	1 comment	Download
M	chrome/browser/cocoa/cookie_prompt_window_controller.mm	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/browser/cocoa/custom_home_pages_model.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/download_started_animation_mac.mm	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/browser/cocoa/extensions/browser_action_button.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/extensions/browser_action_button.mm	View		2 chunks	+1 line, -3 lines	0 comments	Download
M	chrome/browser/cocoa/extensions/browser_actions_controller.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/extensions/browser_actions_controller.mm	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/menu_controller.mm	View		1 chunk	+1 line, -1 line	1 comment	Download
M	chrome/browser/cocoa/preferences_window_controller.mm	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/shell_dialogs_mac.mm	View		3 chunks	+4 lines, -3 lines	0 comments	Download
M	chrome/browser/cocoa/tab_view.mm	View		5 chunks	+0 lines, -10 lines	0 comments	Download
M	chrome/browser/cocoa/url_drop_target.mm	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/web_drag_source.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/web_drop_target.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/renderer_host/render_widget_host_view_mac.h	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	skia/ext/skia_utils_mac.h	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

Nico

I didn't touch 'extern const NSString* foo = @"foo";' yet 'cause that needs more, possibly ...

10 years, 6 months ago (2010-06-11 18:24:28 UTC) #1

Mark Mentovai

10 years, 6 months ago (2010-06-11 18:47:35 UTC) #2

LGTM. Good stuff.

Make sure it’s been tested with the current toolchain too.

http://codereview.chromium.org/2730015/diff/2001/3005
File chrome/browser/cocoa/bookmark_button.h (right):

http://codereview.chromium.org/2730015/diff/2001/3005#newcode12
chrome/browser/cocoa/bookmark_button.h:12: struct BookmarkDragData;
Oh, clang considers class/structness? How very MSVC++-esque.

http://codereview.chromium.org/2730015/diff/2001/3007
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/2730015/diff/2001/3007#newcode1481
chrome/browser/cocoa/browser_window_controller.mm:1481: // TODO(pinkerton):
figure out page-up, http://crbug.com/16305
Wow.

http://codereview.chromium.org/2730015/diff/2001/3015
File chrome/browser/cocoa/menu_controller.mm (right):

http://codereview.chromium.org/2730015/diff/2001/3015#newcode144
chrome/browser/cocoa/menu_controller.mm:144: if (model)
ha!

Expand Messages | Collapse Messages