Add GTK interface for installing bundles of extensions.

Add GTK interface for installing bundles of extensions.

BUG=112096
TEST=*InstallBundle*

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

[jstritar, 2012-02-23 19:57:32]

Could you all take a look?

@mihaip: extension code, general review
@rsesek: ui/cocoa OWNERS
@estade: ui/gtk OWNERS

Thanks!

[Robert Sesek, 2012-02-23 20:12:32]

FYI

https://chromiumcodereview.appspot.com/9456019/diff/5001/chrome/browser/exten...
File chrome/browser/extensions/extension_install_ui.h (right):

https://chromiumcodereview.appspot.com/9456019/diff/5001/chrome/browser/exten...
chrome/browser/extensions/extension_install_ui.h:69: const SkBitmap& icon()
const { return icon_; }
If this class is used exclusively for displaying UI elements, you may want to
consider using gfx::Image here.

https://chromiumcodereview.appspot.com/9456019/diff/7001/chrome/browser/ui/co...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm
(right):

https://chromiumcodereview.appspot.com/9456019/diff/7001/chrome/browser/ui/co...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:151:
[titleField_ setStringValue:base::SysUTF16ToNSString(
Can you re-wrap this line?

[Evan Stade, 2012-02-23 22:13:27]

lgtm with nits

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/gtk/extens...
File chrome/browser/ui/gtk/extensions/bundle_installed_bubble_gtk.cc (right):

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/gtk/extens...
chrome/browser/ui/gtk/extensions/bundle_installed_bubble_gtk.cc:29: const int
kHorizontalColumnSpacing = 10;
docs

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/gtk/extens...
chrome/browser/ui/gtk/extensions/bundle_installed_bubble_gtk.cc:78: 
extra newline

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/gtk/extens...
chrome/browser/ui/gtk/extensions/bundle_installed_bubble_gtk.cc:94:
BubbleGtk::ARROW_LOCATION_TOP_RIGHT :
either indent this 4 more spaces or do

!base::i18n::IsRTL() ? BubbleGtk::ARROW_LOCATION_TOP_RIGHT :
                       BubbleGtk::ARROW_LOCATION_TOP_LEFT;

(annoying BubbleGtk doesn't do this automatically)

[Mihai Parparita -not on Chrome, 2012-02-24 01:42:02]

LGTM

https://chromiumcodereview.appspot.com/9456019/diff/7001/chrome/browser/exten...
File chrome/browser/extensions/extension_install_ui.cc (right):

https://chromiumcodereview.appspot.com/9456019/diff/7001/chrome/browser/exten...
chrome/browser/extensions/extension_install_ui.cc:57: 0,
Since there's enough of these 0 "gaps" in these tables, can you add comments
saying what each one corresponds to and/or why a value is not needed?

[jstritar, 2012-02-24 18:21:45]

http://codereview.chromium.org/9456019/diff/5001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_install_ui.h (right):

http://codereview.chromium.org/9456019/diff/5001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_install_ui.h:69: const SkBitmap& icon()
const { return icon_; }
On 2012/02/23 20:12:32, rsesek wrote:
> If this class is used exclusively for displaying UI elements, you may want to
> consider using gfx::Image here.

Done.

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_install_ui.cc (right):

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_install_ui.cc:57: 0,
On 2012/02/24 01:42:03, Mihai Parparita wrote:
> Since there's enough of these 0 "gaps" in these tables, can you add comments
> saying what each one corresponds to and/or why a value is not needed?

Done.

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

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:151:
[titleField_ setStringValue:base::SysUTF16ToNSString(
On 2012/02/23 20:12:32, rsesek wrote:
> Can you re-wrap this line?

Done.

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/gtk/extens...
File chrome/browser/ui/gtk/extensions/bundle_installed_bubble_gtk.cc (right):

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/gtk/extens...
chrome/browser/ui/gtk/extensions/bundle_installed_bubble_gtk.cc:29: const int
kHorizontalColumnSpacing = 10;
On 2012/02/23 22:13:27, Evan Stade wrote:
> docs

Done.

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/gtk/extens...
chrome/browser/ui/gtk/extensions/bundle_installed_bubble_gtk.cc:78: 
On 2012/02/23 22:13:27, Evan Stade wrote:
> extra newline

Done.

http://codereview.chromium.org/9456019/diff/7001/chrome/browser/ui/gtk/extens...
chrome/browser/ui/gtk/extensions/bundle_installed_bubble_gtk.cc:94:
BubbleGtk::ARROW_LOCATION_TOP_RIGHT :
On 2012/02/23 22:13:27, Evan Stade wrote:
> either indent this 4 more spaces or do
> 
> !base::i18n::IsRTL() ? BubbleGtk::ARROW_LOCATION_TOP_RIGHT :
>                        BubbleGtk::ARROW_LOCATION_TOP_LEFT;
> 
> (annoying BubbleGtk doesn't do this automatically)

Done.

[Robert Sesek, 2012-02-24 19:10:19]

lgtm

http://codereview.chromium.org/9456019/diff/8001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_install_dialog.cc (right):

http://codereview.chromium.org/9456019/diff/8001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_install_dialog.cc:124:
filled_out_prompt.set_icon(gfx::Image(new SkBitmap(*icon)));
FYI: https://chromiumcodereview.appspot.com/9455062/

Don't worry about being blocked on this. I'll fix up all callsites after it
lands.

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

http://codereview.chromium.org/9456019/diff/8001/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:166:
[iconView_ setImage:image];
prompt_->icon()->ToNSImage()

[jstritar, 2012-02-24 20:03:36]

http://codereview.chromium.org/9456019/diff/8001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_install_dialog.cc (right):

http://codereview.chromium.org/9456019/diff/8001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_install_dialog.cc:124:
filled_out_prompt.set_icon(gfx::Image(new SkBitmap(*icon)));
Nice!

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

http://codereview.chromium.org/9456019/diff/8001/chrome/browser/ui/cocoa/exte...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:166:
[iconView_ setImage:image];
On 2012/02/24 19:10:19, rsesek wrote:
> prompt_->icon()->ToNSImage()

Done.

[commit-bot: I haz the power, 2012-02-24 20:04:16]

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

[commit-bot: I haz the power, 2012-02-24 20:04:27]

Presubmit check for 9456019-3025 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
chrome/browser/ui/views/extensions/extension_install_dialog_view.cc

Presubmit checks took 1.8s to calculate.

[jstritar, 2012-02-24 20:08:13]

Hi Scott, can you take a look at the chrome/browser/ui/views changes for the
OWNERS check?

[sky, 2012-02-24 21:05:00]

LGTM

[commit-bot: I haz the power, 2012-02-24 21:05:53]

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

[commit-bot: I haz the power, 2012-02-24 23:06:54]

Change committed as 123574

[commit-bot: I haz the power, 2012-02-25 14:20:47]

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

[commit-bot: I haz the power, 2012-02-25 16:56:35]

Change committed as 123658