Update extension install prompt to reflect withheld permissions

chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm

Index: chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
diff --git a/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
index df708813c6d13897c38c52112c02c69dcc13e9be..3e96e1ebed1c5b9a532af17660ffaa124d63d747 100644
--- a/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
+++ b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
@@ -56,6 +56,9 @@ typedef NSUInteger CellAttributes;
 - (NSDictionary*)buildDetailToggleItem:(size_t)type
                  permissionsDetailIndex:(size_t)index;
 - (NSArray*)buildWarnings:(const ExtensionInstallPrompt::Prompt&)prompt;
+- (NSString*)appendPermissionsToPrompt:(const ExtensionInstallPrompt::Prompt&)

[Alexei Svitkine (slow), 2014/09/15 20:48:16]

Can you add a comment documenting this function? I know this file has bad
precedent for it and doesn't look like it has the comments, but since this
function has a non-trivial amount of params and a return value, it's better to
describe how it works. Put the comment above this function declaration.

[gpdavis, 2014/09/16 19:01:47]

Done.

+             prompt withType:(ExtensionInstallPrompt::PermissionsType)type

[Alexei Svitkine (slow), 2014/09/15 20:48:16]

I don't think git cl format works well for ObjC.

How about this formatting:

- (NSString*)
appendPermissionsToPrompt:(const ExtensionInstallPrompt::Prompt&)prompt
                 withType:(ExtensionInstallPrompt::PermissionsType)type
                 children:(NSMutableArray*)children;

(That way, all the :'s are aligned and you only wrap at the return value, which
is sometimes done in C++ too).

[gpdavis, 2014/09/16 19:01:47]

Done.

+                              children:(NSMutableArray*)children;
 - (void)updateViewFrame:(NSRect)frame;
 @end
 
@@ -592,37 +595,30 @@ bool HasAttribute(id item, CellAttributesMask attributeMask) {
 - (NSArray*)buildWarnings:(const ExtensionInstallPrompt::Prompt&)prompt {
   NSMutableArray* warnings = [NSMutableArray array];
   NSString* heading = nil;
+  NSString* withheldHeading = nil;
 
   ExtensionInstallPrompt::DetailsType type =
       ExtensionInstallPrompt::PERMISSIONS_DETAILS;
+  bool hasPermissions = prompt.GetPermissionCount(
+      ExtensionInstallPrompt::PermissionsType::ALL_PERMISSIONS);
+  CellAttributes warningCellAttributes =
+      kBoldText | kAutoExpandCell | kNoExpandMarker;
   if (prompt.ShouldShowPermissions()) {
     NSMutableArray* children = [NSMutableArray array];
-    if (prompt.GetPermissionCount() > 0) {
-      for (size_t i = 0; i < prompt.GetPermissionCount(); ++i) {
-        [children addObject:
-            [self buildItemWithTitle:SysUTF16ToNSString(prompt.GetPermission(i))
-                      cellAttributes:kUseBullet
-                            children:nil]];
-
-        // If there are additional details, add them below this item.
-        if (!prompt.GetPermissionsDetails(i).empty()) {
-          if (prompt.GetIsShowingDetails(
-              ExtensionInstallPrompt::PERMISSIONS_DETAILS, i)) {
-            [children addObject:
-                [self buildItemWithTitle:SysUTF16ToNSString(
-                    prompt.GetPermissionsDetails(i))
-                          cellAttributes:kNoExpandMarker
-                                children:nil]];
-          }
-
-          // Add a row for the link.
-          [children addObject:
-              [self buildDetailToggleItem:type permissionsDetailIndex:i]];
-        }
-      }
-
-      heading = SysUTF16ToNSString(prompt.GetPermissionsHeading());
-    } else {
+    NSMutableArray* withheldChildren = [NSMutableArray array];
+
+    heading =
+        [self appendPermissionsToPrompt:prompt
+                               withType:ExtensionInstallPrompt::
+                                            PermissionsType::REGULAR_PERMISSIONS
+                               children:children];
+    withheldHeading = [self

[Alexei Svitkine (slow), 2014/09/15 20:48:16]

Nit: I'd wrap the same way as the line above.

[gpdavis, 2014/09/16 19:01:47]

Unfortunately this one is 1 character too long to be formatted the same way :( 
That must be why git cl format did it this way.  Are we okay with wrapping
before :: ?  Like:

ExtensionInstallPrompt::PermissionsType
    ::WITHHELD_PERMISSIONS

Unfortunately it doesn't fit with the colons after PermissionsType either.

+        appendPermissionsToPrompt:prompt
+                         withType:ExtensionInstallPrompt::PermissionsType::
+                                      WITHHELD_PERMISSIONS
+                         children:withheldChildren];
+
+    if (!hasPermissions) {
       [children addObject:
           [self buildItemWithTitle:
               l10n_util::GetNSString(IDS_EXTENSION_NO_SPECIAL_PERMISSIONS)
@@ -631,10 +627,16 @@ bool HasAttribute(id item, CellAttributesMask attributeMask) {
       heading = @"";
     }
 
-    [warnings addObject:[self
-        buildItemWithTitle:heading
-            cellAttributes:kBoldText | kAutoExpandCell | kNoExpandMarker
-                  children:children]];
+    [warnings addObject:[self buildItemWithTitle:heading
+                                  cellAttributes:warningCellAttributes
+                                        children:children]];
+
+    // Add withheld permissions to the prompt if they exist.
+    if (withheldHeading) {
+      [warnings addObject:[self buildItemWithTitle:withheldHeading
+                                    cellAttributes:warningCellAttributes
+                                          children:withheldChildren]];
+    }
   }
 
   if (prompt.GetRetainedFileCount() > 0) {
@@ -652,11 +654,11 @@ bool HasAttribute(id item, CellAttributesMask attributeMask) {
       }
     }
 
-    [warnings addObject:
-        [self buildItemWithTitle:SysUTF16ToNSString(
-            prompt.GetRetainedFilesHeading())
-                  cellAttributes:kBoldText | kAutoExpandCell | kNoExpandMarker
-                        children:children]];
+    [warnings
+        addObject:[self buildItemWithTitle:SysUTF16ToNSString(
+                                               prompt.GetRetainedFilesHeading())
+                            cellAttributes:warningCellAttributes
+                                  children:children]];
 
     // Add a row for the link.
     [warnings addObject:
@@ -666,6 +668,41 @@ bool HasAttribute(id item, CellAttributesMask attributeMask) {
   return warnings;
 }
 
+- (NSString*)appendPermissionsToPrompt:(const ExtensionInstallPrompt::Prompt&)
+             prompt withType:(ExtensionInstallPrompt::PermissionsType)type

[Alexei Svitkine (slow), 2014/09/15 20:48:16]

Use same wrapping I suggested above.

[gpdavis, 2014/09/16 19:01:47]

Done.

+                              children:(NSMutableArray*)children {
+  size_t permissionsCount = prompt.GetPermissionCount(type);
+  if (permissionsCount == 0)
+    return NULL;
+
+  for (size_t i = 0; i < permissionsCount; ++i) {
+    NSDictionary* item = [self

[Alexei Svitkine (slow), 2014/09/15 20:48:16]

Nit: I'd wrap after the = if it still lets you align :'s.

[gpdavis, 2014/09/16 19:01:47]

Not without wrapping the line, unfortunately (it's 82 characters long).  Should
I keep it the way it is, or format it differently?

+        buildItemWithTitle:SysUTF16ToNSString(prompt.GetPermission(i, type))
+            cellAttributes:kUseBullet
+                  children:nil];
+    [children addObject:item];
+
+    // If there are additional details, add them below this item.
+    if (!prompt.GetPermissionsDetails(i, type).empty()) {
+      if (prompt.GetIsShowingDetails(
+              ExtensionInstallPrompt::PERMISSIONS_DETAILS, i)) {
+        item =
+            [self buildItemWithTitle:SysUTF16ToNSString(
+                                         prompt.GetPermissionsDetails(i, type))
+                      cellAttributes:kNoExpandMarker
+                            children:nil];
+        [children addObject:item];
+      }
+
+      // Add a row for the link.
+      [children
+          addObject:[self buildDetailToggleItem:type permissionsDetailIndex:i]];

[Alexei Svitkine (slow), 2014/09/15 20:48:16]

Nit: I'd wrap after the first : here instead.

[gpdavis, 2014/09/16 19:01:47]

Done.

+    }
+  }
+
+  return SysUTF16ToNSString(prompt.GetPermissionsHeading(type));
+}
+
 - (void)updateViewFrame:(NSRect)frame {
   NSWindow* window = [[self view] window];
   [window setFrame:[window frameRectForContentRect:frame] display:YES];