Update UI for WebStore bundle installs.

Update UI for WebStore bundle installs.

This CL adds extension icons in the install dialog as well as the "was installed" bubble.

Followup to https://codereview.chromium.org/855513002/

XIB changes in ExtensionInstallPromptBundle.xib:
* convert the "bundle items" NSTextField (filled with a multiline string) to an NSView (filled with dynamically created per-item subviews)
* add an NSImageView in the top right to hold the bundle item

XIB changes in ExtensionInstalledBubbleBundle.xib:
* convert the "installed" and "failed" NSTextFields (filled with multiline strings) to NSViews (filled with dynamically created per-item subviews)

BUG=448724
Committed: https://crrev.com/d9e1d9dc1f30cba6b7a86d8d578dbd2ed36db808
Cr-Commit-Position: refs/heads/master@{#326769}

[Marc Treib, 2015-04-21 12:10:30]

treib@chromium.org changed reviewers:
+ asvitkine@chromium.org, finnur@chromium.org

[Marc Treib, 2015-04-21 12:10:30]

asvitkine@chromium.org: Please review changes in c/a/nibs and
c/b/ui/cocoa/extensions.

finnur@chromium.org: Please review changes in c/b/extensions and
c/b/ui/views/extensions.

Screenshots (also on the bug):
Win:
https://chromium.googlecode.com/issues/attachment?aid=4487240002000&name=WinB...
Mac:
https://chromium.googlecode.com/issues/attachment?aid=4487240002001&name=MacB...

(No "before" screenshots, because "before" was an unfinished mess :)

[Finnur, 2015-04-21 13:36:51]

LGTM, one nit.

https://codereview.chromium.org/816223008/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/extension_install_dialog_view.h (right):

https://codereview.chromium.org/816223008/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/extension_install_dialog_view.h:154: explicit
IconedView(views::View* view, const gfx::ImageSkia& image);
nit: Don't need explicit.

[Marc Treib, 2015-04-21 13:46:45]

https://codereview.chromium.org/816223008/diff/100001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/extension_install_dialog_view.h (right):

https://codereview.chromium.org/816223008/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/extension_install_dialog_view.h:154: explicit
IconedView(views::View* view, const gfx::ImageSkia& image);
On 2015/04/21 13:36:51, Finnur wrote:
> nit: Don't need explicit.

Right - thanks, done!

[Alexei Svitkine (slow), 2015-04-21 15:14:45]

Please add a "XIB changes" section to the CL description, per the guidelines
here:
https://www.chromium.org/developers/design-documents/mac-xib-files

[Marc Treib, 2015-04-21 15:31:50]

On 2015/04/21 15:14:45, Alexei Svitkine wrote:
> Please add a "XIB changes" section to the CL description, per the guidelines
> here:
> https://www.chromium.org/developers/design-documents/mac-xib-files

Done!

[Alexei Svitkine (slow), 2015-04-21 16:40:53]

Please also add a TEST= line with instructions as to how someone from QA team
can verify that this change is working correctly.

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
(right):

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm:108:
CGFloat ComputeDesiredControlHeight(NSControl* control) {
Add a comment above.

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm:143: if
(requiredHeight > desiredHeight) {
Nit: No {}'s

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm
(right):

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:484:
// Go over the items backwards, since Cocoa coords go from the bottom up.
This code appears to be the same as in extension_install_view_controller.mm.

Please find a way to share the code, instead of repeating it.

[Marc Treib, 2015-04-22 08:09:16]

Comments addressed.

Unfortunately, there's no good way to test this yet, because the corresponding
Web Store parts aren't in place yet. Once they are, we'll update the test
scripts of our QA folks to include a bundle installation.

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
(right):

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm:108:
CGFloat ComputeDesiredControlHeight(NSControl* control) {
On 2015/04/21 16:40:52, Alexei Svitkine wrote:
> Add a comment above.

Done.

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm:143: if
(requiredHeight > desiredHeight) {
On 2015/04/21 16:40:52, Alexei Svitkine wrote:
> Nit: No {}'s

Done.

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm
(right):

https://codereview.chromium.org/816223008/diff/120001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/extension_installed_bubble_controller.mm:484:
// Go over the items backwards, since Cocoa coords go from the bottom up.
On 2015/04/21 16:40:52, Alexei Svitkine wrote:
> This code appears to be the same as in extension_install_view_controller.mm.
> 
> Please find a way to share the code, instead of repeating it.

Done. (Added new bundle_util.h/mm since there didn't seem to be a place for it
otherwise.)

[Alexei Svitkine (slow), 2015-04-22 16:52:27]

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/bundle_util.h (right):

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.h:13: // to |itemsField|.
Document the return value.

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.h:15: NSView* itemsField, const
extensions::BundleInstaller::ItemList& items);
Nit: 1 param per line if first param matches.

Also, output parameters should be last.

Finally, this should use C++ naming convention (hacker style) since it's not an
objective C method.

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/bundle_util.mm (right):

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.mm:14: const CGFloat
kExtensionIconSize = 32;
Put in an anon namespace.

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.mm:17: NSView* itemsField, const
extensions::BundleInstaller::ItemList& items) {
1 param per line.

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.mm:18: const CGFloat titleWidth =
[itemsField frame].size.width - kExtensionIconSize;
C++ naming convention in free standing functions (title_width) - fix throughout.

[Marc Treib, 2015-04-23 09:47:07]

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/bundle_util.h (right):

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.h:13: // to |itemsField|.
On 2015/04/22 16:52:27, Alexei Svitkine wrote:
> Document the return value.

Done.

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.h:15: NSView* itemsField, const
extensions::BundleInstaller::ItemList& items);
On 2015/04/22 16:52:27, Alexei Svitkine wrote:
> Nit: 1 param per line if first param matches.

Okay. The style guide allows either, though.

> Also, output parameters should be last.

Done.

> Finally, this should use C++ naming convention (hacker style) since it's not
an
> objective C method.

Done.

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/bundle_util.mm (right):

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.mm:14: const CGFloat
kExtensionIconSize = 32;
On 2015/04/22 16:52:27, Alexei Svitkine wrote:
> Put in an anon namespace.

Done. FTR though, it's not necessary here, since file-scope constants have
internal linkage anyway.

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.mm:17: NSView* itemsField, const
extensions::BundleInstaller::ItemList& items) {
On 2015/04/22 16:52:27, Alexei Svitkine wrote:
> 1 param per line.

Done.

https://codereview.chromium.org/816223008/diff/140001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/bundle_util.mm:18: const CGFloat titleWidth =
[itemsField frame].size.width - kExtensionIconSize;
On 2015/04/22 16:52:27, Alexei Svitkine wrote:
> C++ naming convention in free standing functions (title_width) - fix
throughout.

Done.

[Alexei Svitkine (slow), 2015-04-23 16:47:39]

lgtm % comment

https://codereview.chromium.org/816223008/diff/160001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
(right):

https://codereview.chromium.org/816223008/diff/160001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm:138:
void OffsetViewVerticallyToFitContent(NSView* view, CGFloat* totalOffset) {
Nit: Since this is a C++ function, use hacker_style within it. Please update 
OffsetControlVerticallyToFitContent() as well which was not using the right
convention.

[Marc Treib, 2015-04-24 09:06:05]

https://codereview.chromium.org/816223008/diff/160001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
(right):

https://codereview.chromium.org/816223008/diff/160001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm:138:
void OffsetViewVerticallyToFitContent(NSView* view, CGFloat* totalOffset) {
On 2015/04/23 16:47:39, Alexei Svitkine wrote:
> Nit: Since this is a C++ function, use hacker_style within it. Please update 
> OffsetControlVerticallyToFitContent() as well which was not using the right
> convention.

Done, also for the other functions below.

[Marc Treib, 2015-04-24 09:06:11]

The CQ bit was checked by treib@chromium.org

[Marc Treib, 2015-04-24 09:06:12]

The patchset sent to the CQ was uploaded after l-g-t-m from finnur@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/816223008/#ps180001
(title: "style fix")

[commit-bot: I haz the power, 2015-04-24 09:06:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/816223008/180001

[commit-bot: I haz the power, 2015-04-24 11:17:17]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-04-24 11:18:21]

Patchset 10 (id:??) landed as
https://crrev.com/d9e1d9dc1f30cba6b7a86d8d578dbd2ed36db808
Cr-Commit-Position: refs/heads/master@{#326769}