Make ExtensionInstallPrompt::Prompt ref-counted

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 37 matching lines @@
 @interface ExtensionInstallViewController ()
 - (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;
+- (NSArray*)buildWarnings:(scoped_refptr<ExtensionInstallPrompt::Prompt>)prompt;
 - (void)updateViewFrame:(NSRect)frame;
 @end
 
 @interface DetailToggleHyperlinkButtonCell : HyperlinkButtonCell {
   NSUInteger permissionsDetailIndex_;
   ExtensionInstallPrompt::DetailsType permissionsDetailType_;
   SEL linkClickedAction_;
 }
 
 @property(assign, nonatomic) NSUInteger permissionsDetailIndex;
@@ skipping 104 matching lines @@
 @synthesize okButton = okButton_;
 @synthesize outlineView = outlineView_;
 @synthesize warningsSeparator = warningsSeparator_;
 @synthesize ratingStars = ratingStars_;
 @synthesize ratingCountField = ratingCountField_;
 @synthesize userCountField = userCountField_;
 @synthesize storeLinkButton = storeLinkButton_;
 
 - (id)initWithNavigator:(content::PageNavigator*)navigator
                delegate:(ExtensionInstallPrompt::Delegate*)delegate
-                 prompt:(const ExtensionInstallPrompt::Prompt&)prompt {
+                 prompt:(scoped_refptr<ExtensionInstallPrompt::Prompt>)prompt {
   // We use a different XIB in the case of bundle installs, installs with
   // webstore data, or no permission warnings. These are laid out nicely for
   // the data they display.
   NSString* nibName = nil;
-  if (prompt.type() == ExtensionInstallPrompt::BUNDLE_INSTALL_PROMPT) {
+  if (prompt->type() == ExtensionInstallPrompt::BUNDLE_INSTALL_PROMPT) {
     nibName = @"ExtensionInstallPromptBundle";
-  } else if (prompt.has_webstore_data()) {
+  } else if (prompt->has_webstore_data()) {
     nibName = @"ExtensionInstallPromptWebstoreData";
-  } else if (!prompt.ShouldShowPermissions() &&
-             prompt.GetRetainedFileCount() == 0) {
+  } else if (!prompt->ShouldShowPermissions() &&
+             prompt->GetRetainedFileCount() == 0) {
     nibName = @"ExtensionInstallPromptNoWarnings";
   } else {
     nibName = @"ExtensionInstallPrompt";
   }
 
   if ((self = [super initWithNibName:nibName
                               bundle:base::mac::FrameworkBundle()])) {
     navigator_ = navigator;
     delegate_ = delegate;
-    prompt_.reset(new ExtensionInstallPrompt::Prompt(prompt));
+    prompt_ = prompt;
     warnings_.reset([[self buildWarnings:prompt] retain]);
   }
   return self;
 }
 
 - (IBAction)storeLinkClicked:(id)sender {
   GURL store_url(extension_urls::GetWebstoreItemDetailURLPrefix() +
                  prompt_->extension()->id());
   navigator_->OpenURL(OpenURLParams(
       store_url, Referrer(), NEW_FOREGROUND_TAB, content::PAGE_TRANSITION_LINK,
@@ skipping 326 matching lines @@
   for (id child in [item objectForKey:kChildrenKey])
     [self expandItemAndChildren:child];
 }
 
 - (void)onToggleDetailsLinkClicked:(id)sender {
   size_t index = [sender permissionsDetailIndex];
   ExtensionInstallPrompt::DetailsType type = [sender permissionsDetailType];
   prompt_->SetIsShowingDetails(
       type, index, !prompt_->GetIsShowingDetails(type, index));
 
-  warnings_.reset([[self buildWarnings:*prompt_] retain]);
+  warnings_.reset([[self buildWarnings:prompt_] retain]);
   [outlineView_ reloadData];
 
   for (id item in warnings_.get())
     [self expandItemAndChildren:item];
 }
 
 - (NSDictionary*)buildItemWithTitle:(NSString*)title
                      cellAttributes:(CellAttributes)cellAttributes
                            children:(NSArray*)children {
   if (!children || ([children count] == 0 && cellAttributes & kUseBullet)) {
@@ skipping 21 matching lines @@
   return @{
     kTitleKey : @"",
     kChildrenKey : @[ @{} ],
     kCellAttributesKey : [NSNumber numberWithInt:kUseCustomLinkCell |
                                                  kNoExpandMarker],
     kPermissionsDetailIndex : [NSNumber numberWithUnsignedInteger:index],
     kPermissionsDetailType : [NSNumber numberWithUnsignedInteger:type],
   };
 }
 
-- (NSArray*)buildWarnings:(const ExtensionInstallPrompt::Prompt&)prompt {
+- (NSArray*)buildWarnings:
+                (scoped_refptr<ExtensionInstallPrompt::Prompt>)prompt {

[Scott Hess - ex-Googler, 2014/06/16 19:29:04]

Since you don't actually take a ref of |prompt| in here, a solution which
side-steps my comments below would be to just leave this as a const ref.  Just
because your object is ref-countable doesn't mean that you have to allow
everyone to ref-count it if you know that the original value can't go out of
scope.

[Devlin, 2014/06/17 16:05:26]

Went with this option.  Thanks! :)

   NSMutableArray* warnings = [NSMutableArray array];
   NSString* heading = nil;
 
   ExtensionInstallPrompt::DetailsType type =
       ExtensionInstallPrompt::PERMISSIONS_DETAILS;
-  if (prompt.ShouldShowPermissions()) {
+  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 (prompt->GetPermissionCount() > 0) {
+      for (size_t i = 0; i < prompt->GetPermissionCount(); ++i) {
+        [children
+            addObject:[self buildItemWithTitle:SysUTF16ToNSString(
+                                                   prompt->GetPermission(i))
+                                cellAttributes:kUseBullet
+                                      children:nil]];

[Scott Hess - ex-Googler, 2014/06/16 19:29:04]

You should feel free to use temporaries to tame the wrapping.  I don't think it
will take any more space, but it will be easier to change going forward. 
Likewise in the three cases below.  This could either be something like:
      NSString* title = SysUTF16ToNSString(prompt->GetPermission(i));
      id child = [self buildItemWithTitle:title
                           cellAttributes:kUseBullet
                                 children:nil];
      [children addObject:child];
or:
      NSString* title = SysUTF16ToNSString(prompt->GetPermission(i));
      [children addObject:[self buildItemWithTitle:title
                                    cellAttributes:kUseBullet
                                          children:nil]];
since each one varies a bit, maybe there's one pattern which is more applicable
than the others.

Of course, it would also work to push the SysUTF16ToNSString() conversion into
-buildItemWithTitle:*, since there are no callers which actually have an
NSString to hand.

 
         // 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]];
+        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());
+      heading = SysUTF16ToNSString(prompt->GetPermissionsHeading());
     } else {
       [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]];
   }
 
-  if (prompt.GetRetainedFileCount() > 0) {
+  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:
-            [self buildItemWithTitle:SysUTF16ToNSString(
-                prompt.GetRetainedFile(i))
-                      cellAttributes:kUseBullet
-                            children:nil]];
+    if (prompt->GetIsShowingDetails(type, 0)) {
+      for (size_t i = 0; i < prompt->GetRetainedFileCount(); ++i) {
+        [children
+            addObject:[self buildItemWithTitle:SysUTF16ToNSString(
+                                                   prompt->GetRetainedFile(i))
+                                cellAttributes:kUseBullet
+                                      children:nil]];
       }
     }
 
-    [warnings addObject:
-        [self buildItemWithTitle:SysUTF16ToNSString(
-            prompt.GetRetainedFilesHeading())
-                  cellAttributes:kBoldText | kAutoExpandCell | kNoExpandMarker
-                        children:children]];
+    [warnings
+        addObject:[self
+                      buildItemWithTitle:SysUTF16ToNSString(
+                                             prompt->GetRetainedFilesHeading())
+                          cellAttributes:kBoldText | kAutoExpandCell |
+                                         kNoExpandMarker
+                                children:children]];
 
     // Add a row for the link.
     [warnings addObject:
         [self buildDetailToggleItem:type permissionsDetailIndex:0]];
   }
 
   return warnings;
 }
 
 - (void)updateViewFrame:(NSRect)frame {
@@ skipping 64 matching lines @@
 }
 
 - (void)accessibilityPerformAction:(NSString*)action {
   if ([action isEqualToString:NSAccessibilityPressAction])
     [self handleLinkClicked];
   else
     [super accessibilityPerformAction:action];
 }
 
 @end