Issue 2858028: [Mac] Base implementation of extension infobars on the mac....

Issue 2858028: [Mac] Base implementation of extension infobars on the mac.... (Closed)

Created:
10 years, 6 months ago by Bons

Modified:
10 years, 6 months ago

Reviewers:
Mark Mentovai

CC:
chromium-reviews, jam+cc_chromium.org, ben+cc_chromium.org, John Grabowski, Erik does not do reviews, Aaron Boodman, pam+watch_chromium.org, brettw-cc_chromium.org, darin-cc_chromium.org

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

[Mac] Base implementation of extension infobars on the mac. BUG=43168 TEST=install an extension that has an infobar. see if one shows up. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51031

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 14

Patch Set 3 : '' #

Patch Set 4 : '' #

Created: 10 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+172 lines, -38 lines)			Patch
M	chrome/browser/cocoa/extension_view_mac.mm	View	1 2 3	2 chunks	+2 lines, -2 lines	0 comments	Download
A	chrome/browser/cocoa/extensions/extension_infobar_controller.h	View	2 3	1 chunk	+22 lines, -0 lines	0 comments	Download
A	chrome/browser/cocoa/extensions/extension_infobar_controller.mm	View	2	1 chunk	+117 lines, -0 lines	0 comments	Download
M	chrome/browser/cocoa/extensions/extension_popup_controller.h	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/cocoa/extensions/extension_popup_controller.mm	View	1 2 3	4 chunks	+5 lines, -10 lines	0 comments	Download
M	chrome/browser/cocoa/infobar_controller.h	View	1 2 3	4 chunks	+7 lines, -3 lines	0 comments	Download
M	chrome/browser/cocoa/infobar_controller.mm	View	1 2 3	4 chunks	+10 lines, -15 lines	0 comments	Download
M	chrome/browser/extensions/extension_infobar_delegate.cc	View	1 2 3	1 chunk	+0 lines, -7 lines	0 comments	Download
M	chrome/browser/renderer_host/render_widget_host_view_mac.mm	View	1 2 3	1 chunk	+6 lines, -0 lines	0 comments	Download
M	chrome/chrome_browser.gypi	View	1 2 3	1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

Mark Mentovai

I’ll take another look when there are comments. http://codereview.chromium.org/2858028/diff/5001/6002 File chrome/browser/cocoa/extensions/extension_infobar_controller.h (right): http://codereview.chromium.org/2858028/diff/5001/6002#newcode4 chrome/browser/cocoa/extensions/extension_infobar_controller.h:4: #include ...

10 years, 6 months ago (2010-06-25 21:38:12 UTC) #1

Bons

10 years, 6 months ago (2010-06-25 22:45:09 UTC) #2

http://codereview.chromium.org/2858028/diff/5001/6002
File chrome/browser/cocoa/extensions/extension_infobar_controller.h (right):

http://codereview.chromium.org/2858028/diff/5001/6002#newcode4
chrome/browser/cocoa/extensions/extension_infobar_controller.h:4: 
On 2010/06/25 21:38:12, Mark Mentovai wrote:
> #include guards.

Done.

http://codereview.chromium.org/2858028/diff/5001/6003
File chrome/browser/cocoa/extensions/extension_infobar_controller.mm (right):

http://codereview.chromium.org/2858028/diff/5001/6003#newcode41
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:41: -
(void)addAdditionalControls {
On 2010/06/25 21:38:12, Mark Mentovai wrote:
> This file could use some comments around the trickier bits.

Done.

http://codereview.chromium.org/2858028/diff/5001/6003#newcode102
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:102:
ExtensionInfoBarController* controller = [[ExtensionInfoBarController alloc]
On 2010/06/25 21:38:12, Mark Mentovai wrote:
> Since you gotta break up lines anyway, I find this more readable:
> 
>   ExtensionInfoBarController* controller =
>       [[ExtensionInfoBarController alloc] initWithDelegate:this
>                                                     window:window];

Done.

http://codereview.chromium.org/2858028/diff/5001/6003#newcode105
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:105: return new
InfoBar(controller);
On 2010/06/25 21:38:12, Mark Mentovai wrote:
> InfoBar doesn’t own its controller, right? Who’s responsible for deleting the
> ExtensionInfoBarController?

The InfoBarController class. See cocoa/infobar.h

http://codereview.chromium.org/2858028/diff/5001/6006
File chrome/browser/cocoa/infobar_controller.h (right):

http://codereview.chromium.org/2858028/diff/5001/6006#newcode76
chrome/browser/cocoa/infobar_controller.h:76: // Removes the ok and cancel
buttons and resizes the textfield to use the
On 2010/06/25 21:38:12, Mark Mentovai wrote:
> OK, Cancel (caps)

Done.

http://codereview.chromium.org/2858028/diff/5001/6007
File chrome/browser/cocoa/infobar_controller.mm (right):

http://codereview.chromium.org/2858028/diff/5001/6007#newcode216
chrome/browser/cocoa/infobar_controller.mm:216: // Remove the ok and cancel
buttons, since they are not needed.
On 2010/06/25 21:38:12, Mark Mentovai wrote:
> The comment in the .h said this too?

Done.

http://codereview.chromium.org/2858028/diff/5001/6010
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/2858028/diff/5001/6010#newcode730
chrome/chrome_browser.gypi:730:
'browser/cocoa/extensions/extension_infobar_controller.h',
On 2010/06/25 21:38:12, Mark Mentovai wrote:
> Sort!

Done.

Expand Messages | Collapse Messages