Add Mac interface for installing bundles of extensions.

Add Mac interface for installing bundles of extensions.

BUG=112096
TEST=chrome --apps-gallery-url="http://www.corp.google.com"
Then verify the UI when installing the bundle at http://www.corp.google.com/~jstritar/bundle.html.

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

[jstritar, 2012-02-26 22:15:43]

Hey, can you guys take a look?

@rsesek: chrome/browser/ui/cocoa
@yoz: chrome/browser/extensions
@estade: chrome/browser/ui/gtk/

[Robert Sesek, 2012-02-27 18:59:20]

Can you please attach screen shots of what this looks like? Also, have you
tested in different languages like Russian, German, and Italian?

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm
(left):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:43: //
amount the control's height had to be adjusted.
Why remove this comment?

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:264: 
nit: remove

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_bridge.mm
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_bridge.mm:85: 
nit: remove

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h:24: }
 // namespace extensions
nit: no end namespace comments for forward declares

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:367:
int installedListHeight =
For the code you're adding, use NSInteger instead of plain int. You can leave
existing uses, though.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:399:
BOOL hidden = heading.empty();
C++ bool instead of ObjC bool.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:408:
NSMutableString* joinedItems = [NSMutableString string];
This code is very similar with that of the other file. Does it make sense to
generalize into a function?

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/tool...
File chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm (right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/tool...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm:714: -
(NSPoint)wrenchButtonBubblePoint {
Remove? Not called and not declared in the .h

[Yoyo Zhou, 2012-02-28 00:42:38]

LGTM for chrome/browser/extensions

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/extensions/bu...
File chrome/browser/extensions/bundle_installer.cc (left):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/extensions/bu...
chrome/browser/extensions/bundle_installer.cc:169: // so we always show the same
message.
Why remove this comment?

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/extensions/bu...
File chrome/browser/extensions/bundle_installer.cc (right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/extensions/bu...
chrome/browser/extensions/bundle_installer.cc:196: #if defined(TOOLKIT_VIEWS)
I trust you know what to do with the merge conflict between your 2 CLs here.

[Evan Stade, 2012-02-28 01:01:51]

gtk lgtm

[jstritar, 2012-03-05 18:05:08]

Can you take another look?

I tested on a couple languages including an RTL. (new strings not translated yet
though).

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/extensions/bu...
File chrome/browser/extensions/bundle_installer.cc (left):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/extensions/bu...
chrome/browser/extensions/bundle_installer.cc:169: // so we always show the same
message.
Re-added.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/extensions/bu...
File chrome/browser/extensions/bundle_installer.cc (right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/extensions/bu...
chrome/browser/extensions/bundle_installer.cc:196: #if defined(TOOLKIT_VIEWS)
The other CL landed, so I removed this entire block.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm
(left):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:43: //
amount the control's height had to be adjusted.
Updated the comment.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:264: 
On 2012/02/27 18:59:20, rsesek wrote:
> nit: remove

Done.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_bridge.mm
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_bridge.mm:85: 
On 2012/02/27 18:59:20, rsesek wrote:
> nit: remove

Done.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.h:24: }
 // namespace extensions
On 2012/02/27 18:59:20, rsesek wrote:
> nit: no end namespace comments for forward declares

Done.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:367:
int installedListHeight =
On 2012/02/27 18:59:20, rsesek wrote:
> For the code you're adding, use NSInteger instead of plain int. You can leave
> existing uses, though.

Done.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:399:
BOOL hidden = heading.empty();
On 2012/02/27 18:59:20, rsesek wrote:
> C++ bool instead of ObjC bool.

Done.

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:408:
NSMutableString* joinedItems = [NSMutableString string];
On 2012/02/27 18:59:20, rsesek wrote:
> This code is very similar with that of the other file. Does it make sense to
> generalize into a function?

These classes use slightly different approaches for laying out the controls,
which is kind of silly. How about I reconcile these in a follow up CL?

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/tool...
File chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm (right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/tool...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm:714: -
(NSPoint)wrenchButtonBubblePoint {
On 2012/02/27 18:59:20, rsesek wrote:
> Remove? Not called and not declared in the .h

Done.

[Robert Sesek, 2012-03-05 18:30:13]

lgtm with nits

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm
(right):

http://codereview.chromium.org/9460045/diff/2002/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:408:
NSMutableString* joinedItems = [NSMutableString string];
On 2012/03/05 18:05:08, jstritar wrote:
> On 2012/02/27 18:59:20, rsesek wrote:
> > This code is very similar with that of the other file. Does it make sense to
> > generalize into a function?
> 
> These classes use slightly different approaches for laying out the controls,
> which is kind of silly. How about I reconcile these in a follow up CL?

SGTM

http://codereview.chromium.org/9460045/diff/15001/chrome/browser/ui/cocoa/ext...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm
(right):

http://codereview.chromium.org/9460045/diff/15001/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:382:

nit: remove blank line

http://codereview.chromium.org/9460045/diff/15001/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:395:
- (int)addExtensionList:(NSTextField*)headingMsg
(NSInteger)

http://codereview.chromium.org/9460045/diff/15001/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:432:
BOOL is_first_visible = YES;
naming: isFirstVisible

[jstritar, 2012-03-05 19:11:59]

Also converted a couple more ints to NSInteger.

http://codereview.chromium.org/9460045/diff/15001/chrome/browser/ui/cocoa/ext...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm
(right):

http://codereview.chromium.org/9460045/diff/15001/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:382:

On 2012/03/05 18:30:13, rsesek wrote:
> nit: remove blank line

Done.

http://codereview.chromium.org/9460045/diff/15001/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:395:
- (int)addExtensionList:(NSTextField*)headingMsg
On 2012/03/05 18:30:13, rsesek wrote:
> (NSInteger)

Done.

http://codereview.chromium.org/9460045/diff/15001/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:432:
BOOL is_first_visible = YES;
On 2012/03/05 18:30:13, rsesek wrote:
> naming: isFirstVisible

Done.

[commit-bot: I haz the power, 2012-03-05 19:12:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jstritar@chromium.org/9460045/18003

[commit-bot: I haz the power, 2012-03-05 20:59:34]

Change committed as 124994

[Nico, 2012-03-09 02:20:54]

It looks like the ExtensionInstalledBubbleBundle.xib file contains a
GTMUILocalizerAndLayoutTweaker and a ChromeUILocalizer, but all strings are
inserted at runtime, so there's nothing to localize. If you don't plan on adding
localized strings right to the xib file soon, please remove these two objects
since they cause build warning:

[4947/6288] ACTION browser: Process_xibs_for_localization
app/nibs/ExtensionInstalledBubbleBundle.xib:0: warning: generate_localizer
didn't find any resource strings