Cocoa: Show OAuth issues in extension install dialog

Cocoa: Show OAuth issues in extension install dialog

This is the Cocoa version of the following two CLs:
https://chromiumcodereview.appspot.com/10696042
https://chromiumcodereview.appspot.com/10824054

Currently the extension install dialog shows permission warnings inside a multiline text field.

OAuth issues are different in that they are a list of issues, each of which can be expadned to show more details.

To support this I replaced the multiline text field with an NSOutlineView.

Screenshots:
  - OAuth issues + Warnings: http://i.imgur.com/Hbag3.png
  - Inline install + Warnings: http://i.imgur.com/gpLI9.png
  - Inline + No Warnings (outline view hidden): http://i.imgur.com/se9tH.png
  - Bundle install + Warnings: http://i.imgur.com/5H3Rm.png

XIB changes: Replaced warning and subtitle text field with outline view. Updated relevant outlets.

BUG=30206
TEST=
  1. Test OAuth issues. Run chrome with --demand-user-scope-approval. Drag/drop oauth2.crx from estade. Verify that warnings and oauth issues are displayed. Verify that expanding a oauth issue grows the dialog.
  2. Test Inline install + Warning. Navigate to https://www.google.com/chrome/intl/en/p/google-plus.html and click install. Verify that warnings are displayed.
  3. Test Inline Install + No Warnings. Navigate to http://chrome.angrybirds.com/ and click install. Verify that the outline view is hidden and that the dialog is correctly resized.
  4. Test Bundle Install. Run chrome with  --apps-gallery-url="http://www.corp.google.com/" Navigate to http://www.corp.google.com/~jstritar/bundle.html. Verify that warnings are shown.

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

[Evan Stade, 2012-08-03 18:07:20]

changes to non-mac things LGTM. +sky for ui/ OWNERS stamp.

https://chromiumcodereview.appspot.com/10829170/diff/11/chrome/browser/ui/vie...
File chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
(right):

https://chromiumcodereview.appspot.com/10829170/diff/11/chrome/browser/ui/vie...
chrome/browser/ui/views/extensions/extension_install_dialog_view.cc:380:
IDS_EXTENSION_PERMISSION_LINE, prompt.GetPermission(i));
you can use PrepareForDisplay

[sail, 2012-08-03 19:10:39]

Rebased and fixed GTK build.

https://chromiumcodereview.appspot.com/10829170/diff/11/chrome/browser/ui/vie...
File chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
(right):

https://chromiumcodereview.appspot.com/10829170/diff/11/chrome/browser/ui/vie...
chrome/browser/ui/views/extensions/extension_install_dialog_view.cc:380:
IDS_EXTENSION_PERMISSION_LINE, prompt.GetPermission(i));
On 2012/08/03 18:07:20, Evan Stade wrote:
> you can use PrepareForDisplay

Done.

[sky, 2012-08-03 22:22:40]

views LGTM

[Robert Sesek, 2012-08-06 21:31:48]

Overall pretty good. We've started to prefer NSWidth/Height over direct struct
access, so mostly comments about that.

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h
(right):

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h:50:
BOOL isComputingRowHeight;
nit: trailing _

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm
(right):

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:79:
height += [outlineView rectOfRow:i].size.height;
nit: NSHeight

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:88:
CGFloat offset = desiredHeight - frame.size.height;
nit: NSHeight

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:104:
NSDictionary* BuildItem(NSString* title, BOOL isGroupItem, NSArray* children) {
I think this may be easier to read inline as an ObjC method because of the named
parameters…

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:113:
canExpand = false;
NO

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:162:
rect.size.width = std::min(frame.size.width, frame.size.height) * 0.38;
Use NSWidth/Height in here

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:401:
[[self window] setFrame:NSMakeRect(currentRect.origin.x,
Why not just mutate currentRect and pass that?

https://chromiumcodereview.appspot.com/10829170/diff/1010/chrome/browser/ui/c...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:472:
row:row].size.width;
NSWidth

[sail, 2012-08-09 02:00:29]

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h
(right):

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h:50:
BOOL isComputingRowHeight;
On 2012/08/06 21:31:48, rsesek wrote:
> nit: trailing _

Done.

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
File chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm
(right):

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:79:
height += [outlineView rectOfRow:i].size.height;
On 2012/08/06 21:31:48, rsesek wrote:
> nit: NSHeight

Done.

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:88:
CGFloat offset = desiredHeight - frame.size.height;
On 2012/08/06 21:31:48, rsesek wrote:
> nit: NSHeight

Done.

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:104:
NSDictionary* BuildItem(NSString* title, BOOL isGroupItem, NSArray* children) {
On 2012/08/06 21:31:48, rsesek wrote:
> I think this may be easier to read inline as an ObjC method because of the
named
> parameters…

Done.

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:113:
canExpand = false;
On 2012/08/06 21:31:48, rsesek wrote:
> NO

Done.

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:162:
rect.size.width = std::min(frame.size.width, frame.size.height) * 0.38;
On 2012/08/06 21:31:48, rsesek wrote:
> Use NSWidth/Height in here

Done.

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:401:
[[self window] setFrame:NSMakeRect(currentRect.origin.x,
On 2012/08/06 21:31:48, rsesek wrote:
> Why not just mutate currentRect and pass that?

Done.
Good idea.

http://codereview.chromium.org/10829170/diff/1010/chrome/browser/ui/cocoa/ext...
chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm:472:
row:row].size.width;
On 2012/08/06 21:31:48, rsesek wrote:
> NSWidth

Done.

[Robert Sesek, 2012-08-09 14:48:16]

LGTM

[sky, 2012-08-09 16:15:26]

LGTM

[Evan Stade, 2012-08-10 02:47:42]

p.s., your screenshots make me think we probably do one day want the oauth stuff
for inline installs (although probably not bundles). However this is not urgent
right now.

[commit-bot: I haz the power, 2012-08-10 23:16:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/10829170/13004

[commit-bot: I haz the power, 2012-08-11 03:57:58]

Change committed as 151187