[Mac] Finish up extension infobar UI implementation.

[Mac] Finish up extension infobar UI implementation.
o One issue under consideration is whether the HTML content of the infobar can take up the entire width. Right now on Windows it does not but with this implementation (and on Linux) it does.

BUG=43168
TEST=ExtensionApiTest.Infobars

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52182

[Mark Mentovai - out til August, 2010-07-12 18:45:59]

LG with these changes.

http://codereview.chromium.org/2973003/diff/5001/6001
File chrome/browser/browser_resources.grd (right):

http://codereview.chromium.org/2973003/diff/5001/6001#newcode37
chrome/browser/browser_resources.grd:37: <include
name="IDR_EXTENSIONS_INFOBAR_CSS_MAC"
file="resources\extensions_infobar_mac.css" flattenhtml="true" type="BINDATA" />
Let’s use IDR_EXTENSIONS_INFOBAR_CSS and make the value dependent on <if
expr="os == 'darwin'"> .

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

http://codereview.chromium.org/2973003/diff/5001/6003#newcode280
chrome/browser/cocoa/extensions/extension_action_context_menu.mm:280:
if([menuItem isEqualTo: inspectorItem_.get()]) {
No space after colon.

http://codereview.chromium.org/2973003/diff/5001/6003#newcode281
chrome/browser/cocoa/extensions/extension_action_context_menu.mm:281: return
(action_ && action_->HasPopup(CurrentTabId()));
Outer parentheses unnecessary.

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

http://codereview.chromium.org/2973003/diff/5001/6005#newcode39
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:39: // and.
width and what?

http://codereview.chromium.org/2973003/diff/5001/6005#newcode45
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:45: // A helper
class to bridge the asynchronous Skia bitmap loading mechanism to
Anonymous namespace for this class?

http://codereview.chromium.org/2973003/diff/5001/6005#newcode50
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:50:
InfobarBridge(ExtensionInfoBarController* owner) : owner_(owner),
explicit?

http://codereview.chromium.org/2973003/diff/5001/6005#newcode52
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:52: delegate_ =
[owner delegate]->AsExtensionInfoBarDelegate();
You could have done delegate_([owner delegate]->AsExtensionInfoBarDeleagate())
in the initializer list too. It doesn’t really make too much of a difference
here in this case. Sometimes the compiler can generate better or more-correct
code when you use the initializer list, but here it shouldn’t matter at all.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode57
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:57:
~InfobarBridge() {
I suspect that this is virtual because some superclass declares a virtual
destructor. You can mark this one as virtual to make it explicit.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode73
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:73:
OnImageLoaded(NULL, icon_resource, 0);  // |image|, |index|.
The in-line comment on this line sucks. Fortunately, you can just get rid of it,
because apparently it just means to tell you what the two zero parameters are
about, and you can look a few lines down to get the same information.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode96
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:96: int
image_size = Extension::EXTENSION_ICON_BITTY;
const?

http://codereview.chromium.org/2973003/diff/5001/6005#newcode176
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:176:
floor(NSMidY([infoBarView_ frame]) - (kButtonHeightPx / 2.0)) +
std::floor is better because it adapts its return type to its argument.

http://codereview.chromium.org/2973003/diff/5001/6006
File chrome/browser/extensions/extension_host.cc (right):

http://codereview.chromium.org/2973003/diff/5001/6006#newcode340
chrome/browser/extensions/extension_host.cc:340: #if defined(OS_MACOSX)
As discussed.

[Andrew Bonventre (Bons), 2010-07-12 21:39:47]

Done and enabled the API test.

http://codereview.chromium.org/2973003/diff/5001/6001
File chrome/browser/browser_resources.grd (right):

http://codereview.chromium.org/2973003/diff/5001/6001#newcode37
chrome/browser/browser_resources.grd:37: <include
name="IDR_EXTENSIONS_INFOBAR_CSS_MAC"
file="resources\extensions_infobar_mac.css" flattenhtml="true" type="BINDATA" />
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> Let’s use IDR_EXTENSIONS_INFOBAR_CSS and make the value dependent on <if
> expr="os == 'darwin'"> .

Done.

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

http://codereview.chromium.org/2973003/diff/5001/6003#newcode280
chrome/browser/cocoa/extensions/extension_action_context_menu.mm:280:
if([menuItem isEqualTo: inspectorItem_.get()]) {
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> No space after colon.

Done.

http://codereview.chromium.org/2973003/diff/5001/6003#newcode281
chrome/browser/cocoa/extensions/extension_action_context_menu.mm:281: return
(action_ && action_->HasPopup(CurrentTabId()));
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> Outer parentheses unnecessary.

Done.

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

http://codereview.chromium.org/2973003/diff/5001/6005#newcode39
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:39: // and.
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> width and what?

Done.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode45
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:45: // A helper
class to bridge the asynchronous Skia bitmap loading mechanism to
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> Anonymous namespace for this class?

Can't. Needed outside of class (instance var).

http://codereview.chromium.org/2973003/diff/5001/6005#newcode50
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:50:
InfobarBridge(ExtensionInfoBarController* owner) : owner_(owner),
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> explicit?

Done.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode52
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:52: delegate_ =
[owner delegate]->AsExtensionInfoBarDelegate();
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> You could have done delegate_([owner delegate]->AsExtensionInfoBarDeleagate())
> in the initializer list too. It doesn’t really make too much of a difference
> here in this case. Sometimes the compiler can generate better or more-correct
> code when you use the initializer list, but here it shouldn’t matter at all.

Done.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode57
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:57:
~InfobarBridge() {
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> I suspect that this is virtual because some superclass declares a virtual
> destructor. You can mark this one as virtual to make it explicit.

Done.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode73
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:73:
OnImageLoaded(NULL, icon_resource, 0);  // |image|, |index|.
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> The in-line comment on this line sucks. Fortunately, you can just get rid of
it,
> because apparently it just means to tell you what the two zero parameters are
> about, and you can look a few lines down to get the same information.

Done.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode96
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:96: int
image_size = Extension::EXTENSION_ICON_BITTY;
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> const?

Done.

http://codereview.chromium.org/2973003/diff/5001/6005#newcode176
chrome/browser/cocoa/extensions/extension_infobar_controller.mm:176:
floor(NSMidY([infoBarView_ frame]) - (kButtonHeightPx / 2.0)) +
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> std::floor is better because it adapts its return type to its argument.

Done.

http://codereview.chromium.org/2973003/diff/5001/6006
File chrome/browser/extensions/extension_host.cc (right):

http://codereview.chromium.org/2973003/diff/5001/6006#newcode340
chrome/browser/extensions/extension_host.cc:340: #if defined(OS_MACOSX)
On 2010/07/12 18:45:59, Mark Mentovai wrote:
> As discussed.

Done.

[Mark Mentovai - out til August, 2010-07-12 21:49:30]

LG™