Update extension install prompt to reflect withheld permissions

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/extensions/extension_install_view_controller.h"
 
 #include "base/auto_reset.h"
 #include "base/i18n/rtl.h"
 #include "base/mac/bundle_locations.h"
 #include "base/mac/mac_util.h"
@@ skipping 38 matching lines @@
 - (BOOL)isBundleInstall;
 - (BOOL)hasWebstoreData;
 - (void)appendRatingStar:(const gfx::ImageSkia*)skiaImage;
 - (void)onOutlineViewRowCountDidChange;
 - (NSDictionary*)buildItemWithTitle:(NSString*)title
                      cellAttributes:(CellAttributes)cellAttributes
                            children:(NSArray*)children;
 - (NSDictionary*)buildDetailToggleItem:(size_t)type
                  permissionsDetailIndex:(size_t)index;
 - (NSArray*)buildWarnings:(const ExtensionInstallPrompt::Prompt&)prompt;
+- (bool)appendPermissions:(const ExtensionInstallPrompt::Prompt&)prompt

[Alexei Svitkine (slow), 2014/09/12 19:32:23]

How about the following name, which more closely follows Cocoa permissions:

-appendPermissionsToPrompt:
withType:
heading:
children:

[gpdavis, 2014/09/12 21:56:09]

Done.

+                     type:(ExtensionInstallPrompt::PermissionsType)type
+                  heading:(NSString**)heading
+                 children:(NSMutableArray*)children;
 - (void)updateViewFrame:(NSRect)frame;
 @end
 
 @interface DetailToggleHyperlinkButtonCell : HyperlinkButtonCell {
   NSUInteger permissionsDetailIndex_;
   ExtensionInstallPrompt::DetailsType permissionsDetailType_;
   SEL linkClickedAction_;
 }
 
 @property(assign, nonatomic) NSUInteger permissionsDetailIndex;
@@ skipping 516 matching lines @@
     kCellAttributesKey : [NSNumber numberWithInt:kUseCustomLinkCell |
                                                  kNoExpandMarker],
     kPermissionsDetailIndex : [NSNumber numberWithUnsignedInteger:index],
     kPermissionsDetailType : [NSNumber numberWithUnsignedInteger:type],
   };
 }
 
 - (NSArray*)buildWarnings:(const ExtensionInstallPrompt::Prompt&)prompt {
   NSMutableArray* warnings = [NSMutableArray array];
   NSString* heading = nil;
+  NSString* withheld_heading = nil;

[Alexei Svitkine (slow), 2014/09/12 19:32:24]

Using camelCase in ObjC methods. Here and in the code below.

[gpdavis, 2014/09/12 21:56:09]

Done.

 
   ExtensionInstallPrompt::DetailsType type =
       ExtensionInstallPrompt::PERMISSIONS_DETAILS;
+  bool has_permissions = prompt.GetPermissionCount(
+      ExtensionInstallPrompt::PermissionsType::ALL_PERMISSIONS);
   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]];
+    NSMutableArray* withheld_children = [NSMutableArray array];
 
-        // 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]];
-          }
+    [self appendPermissions:prompt
+                       type:ExtensionInstallPrompt::PermissionsType::
+                                REGULAR_PERMISSIONS
+                    heading:&heading
+                   children:children];
+    [self appendPermissions:prompt
+                       type:ExtensionInstallPrompt::PermissionsType::
+                                WITHHELD_PERMISSIONS
+                    heading:&withheld_heading
+                   children:withheld_children];
 
-          // Add a row for the link.
-          [children addObject:
-              [self buildDetailToggleItem:type permissionsDetailIndex:i]];
-        }
-      }
-
-      heading = SysUTF16ToNSString(prompt.GetPermissionsHeading());
-    } else {
+    if (!has_permissions) {
       [children addObject:
           [self buildItemWithTitle:
               l10n_util::GetNSString(IDS_EXTENSION_NO_SPECIAL_PERMISSIONS)
                     cellAttributes:kUseBullet
                           children:nil]];
       heading = @"";
     }
 
     [warnings addObject:[self
         buildItemWithTitle:heading
             cellAttributes:kBoldText | kAutoExpandCell | kNoExpandMarker
                   children:children]];
+
+    // Add withheld permissions to the prompt if they exist.
+    if (withheld_heading) {
+      [warnings addObject:[self buildItemWithTitle:withheld_heading
+                                    cellAttributes:kBoldText | kAutoExpandCell |

[Alexei Svitkine (slow), 2014/09/12 19:32:24]

Nit: Make a local var for attributes to avoid this wrapping.

[gpdavis, 2014/09/12 21:56:09]

Done.

+                                                   kNoExpandMarker
+                                          children:withheld_children]];
+    }
   }
 
   if (prompt.GetRetainedFileCount() > 0) {
     type = ExtensionInstallPrompt::RETAINED_FILES_DETAILS;
 
     NSMutableArray* children = [NSMutableArray array];
 
     if (prompt.GetIsShowingDetails(type, 0)) {
       for (size_t i = 0; i < prompt.GetRetainedFileCount(); ++i) {
         [children addObject:
@@ skipping 11 matching lines @@
                         children:children]];
 
     // Add a row for the link.
     [warnings addObject:
         [self buildDetailToggleItem:type permissionsDetailIndex:0]];
   }
 
   return warnings;
 }
 
+- (bool)appendPermissions:(const ExtensionInstallPrompt::Prompt&)prompt
+                     type:(ExtensionInstallPrompt::PermissionsType)type
+                  heading:(NSString**)heading
+                 children:(NSMutableArray*)children {
+  size_t permissions_count = prompt.GetPermissionCount(type);
+  if (permissions_count > 0) {

[Alexei Svitkine (slow), 2014/09/12 19:32:24]

Prefer an early return here.

[gpdavis, 2014/09/12 21:56:09]

Done.

+    for (size_t i = 0; i < permissions_count; ++i) {
+      [children
+          addObject:[self buildItemWithTitle:SysUTF16ToNSString(
+                                                 prompt.GetPermission(i, type))
+                              cellAttributes:kUseBullet
+                                    children:nil]];
+
+      // If there are additional details, add them below this item.
+      if (!prompt.GetPermissionsDetails(i, type).empty()) {
+        if (prompt.GetIsShowingDetails(
+                ExtensionInstallPrompt::PERMISSIONS_DETAILS, i)) {
+          [children
+              addObject:[self
+                            buildItemWithTitle:SysUTF16ToNSString(
+                                                   prompt.GetPermissionsDetails(
+                                                       i, type))
+                                cellAttributes:kNoExpandMarker
+                                      children:nil]];
+        }
+
+        // Add a row for the link.
+        [children addObject:[self buildDetailToggleItem:type
+                                 permissionsDetailIndex:i]];
+      }
+    }
+
+    *heading = SysUTF16ToNSString(prompt.GetPermissionsHeading(type));
+    return true;
+  }
+  return false;
+}
+
 - (void)updateViewFrame:(NSRect)frame {
   NSWindow* window = [[self view] window];
   [window setFrame:[window frameRectForContentRect:frame] display:YES];
   [[self view] setFrame:frame];
 }
 
 @end
 
 
 @implementation DetailToggleHyperlinkButtonCell
@@ skipping 55 matching lines @@
 }
 
 - (void)accessibilityPerformAction:(NSString*)action {
   if ([action isEqualToString:NSAccessibilityPressAction])
     [self handleLinkClicked];
   else
     [super accessibilityPerformAction:action];
 }
 
 @end