Merged permissions in the App Info dialog into a single list

chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc

Index: chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc
diff --git a/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc
index d7f02218efdf29fa12328a06a903e6287afee595..3aeb601902641463bab229184fc1a3214956afd4 100644
--- a/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc
+++ b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel.cc
@@ -17,6 +17,8 @@
 #include "extensions/common/permissions/api_permission.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/views/border.h"
+#include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
@@ -24,55 +26,60 @@
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/view.h"
 
+namespace {
+
+// IDs for the two bullet column sets.
+static const int kBulletColumnSetId = 1;

[tapted, 2014/10/20 00:08:41]

nit: `static` is redundant. (and `const` also implies internal linkage so the
anon-namespace is also technically redundant, but we tend to keep that).

[sashab, 2014/10/20 00:45:29]

Done.

+static const int kNestedBulletColumnSetId = 2;
+
+// Creates a close button with the given listener that can be placed to
+// the right of a bullet in the permissions list. The button is shifted
+// down by 1px to align with the permission's bullet.
+views::LabelButton* CreateCloseButton(views::ButtonListener* listener) {
+  views::LabelButton* btn = views::BubbleFrameView::CreateCloseButton(listener);
+  btn->SetBorder(views::Border::CreateEmptyBorder(1, 0, 0, 0));
+  return btn;
+}
+
+}  // namespace
+
 AppInfoPermissionsPanel::AppInfoPermissionsPanel(
     Profile* profile,
     const extensions::Extension* app)
     : AppInfoPanel(profile, app),
-      active_permissions_heading_(NULL),
-      active_permissions_list_(NULL),
-      retained_files_heading_(NULL),
-      retained_files_list_(NULL),
+      permissions_list_count_(0),
+      permissions_list_(NULL),
+      permissions_list_layout_(NULL),
       revoke_file_permissions_button_(NULL),
-      retained_devices_heading_(NULL),
-      retained_devices_list_(NULL),
       revoke_device_permissions_button_(NULL) {
-  // Create UI elements.
-  CreateActivePermissionsControl();
-  CreateRetainedFilesControl();
-  CreateRetainedDevicesControl();
-
-  // Layout elements.
   SetLayoutManager(
       new views::BoxLayout(views::BoxLayout::kVertical,
                            0,
                            0,
                            views::kUnrelatedControlVerticalSpacing));
 
-  LayoutActivePermissionsControl();
-  LayoutRetainedFilesControl();
-  LayoutRetainedDevicesControl();
+  CreatePermissionsList();
+  FillPermissionsList();
+  LayoutPermissionsList();
 }
 
 AppInfoPermissionsPanel::~AppInfoPermissionsPanel() {
-  // Destroy view children before their models.
-  RemoveAllChildViews(true);
 }
 
 // Given a list of strings, returns a view containing a list of these strings
 // as bulleted items.
-views::View* AppInfoPermissionsPanel::CreateBulletedListView(
-    const std::vector<base::string16>& messages,
-    bool allow_multiline,
-    gfx::ElideBehavior elide_behavior) {
+void AppInfoPermissionsPanel::CreatePermissionsList() {
   const int kSpacingBetweenBulletAndStartOfText = 5;
+  const int kSpacingBetweenTextAndRevokeButton = 5;
+  const int kIndentationBeforeNestedBullet = 13;
 
-  views::View* list_view = new views::View();
-  views::GridLayout* layout = new views::GridLayout(list_view);
-  list_view->SetLayoutManager(layout);
+  permissions_list_ = new views::View();
+  permissions_list_layout_ = new views::GridLayout(permissions_list_);
+  permissions_list_->SetLayoutManager(permissions_list_layout_);
 
-  // Create 2 columns: one for the bullet, one for the bullet text.
-  static const int kColumnSetId = 1;
-  views::ColumnSet* column_set = layout->AddColumnSet(kColumnSetId);
+  // Create 3 columns: the bullet, the bullet text, and the revoke button.
+  views::ColumnSet* column_set =
+      permissions_list_layout_->AddColumnSet(kBulletColumnSetId);
   column_set->AddColumn(views::GridLayout::FILL,
                         views::GridLayout::LEADING,
                         0,
@@ -82,146 +89,133 @@ views::View* AppInfoPermissionsPanel::CreateBulletedListView(
   column_set->AddPaddingColumn(0, kSpacingBetweenBulletAndStartOfText);
   column_set->AddColumn(views::GridLayout::FILL,
                         views::GridLayout::LEADING,
-                        1,
+                        1 /* stretch to fill space */,
+                        views::GridLayout::USE_PREF,
+                        0,
+                        0);
+  column_set->AddPaddingColumn(0, kSpacingBetweenTextAndRevokeButton);
+  column_set->AddColumn(views::GridLayout::FILL,
+                        views::GridLayout::LEADING,
+                        0,
                         views::GridLayout::USE_PREF,
                         0,
                         0);
 
-  for (std::vector<base::string16>::const_iterator it = messages.begin();
-       it != messages.end();
-       ++it) {
-    views::Label* permission_label = new views::Label(*it);
-    permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-
-    if (allow_multiline)
-      permission_label->SetMultiLine(true);
-    else
-      permission_label->SetElideBehavior(elide_behavior);
-
-    // Extract only the bullet from the IDS_EXTENSION_PERMISSION_LINE text.
-    views::Label* bullet_label = new views::Label(l10n_util::GetStringFUTF16(
-        IDS_EXTENSION_PERMISSION_LINE, base::string16()));
+  views::ColumnSet* nested_column_set =
+      permissions_list_layout_->AddColumnSet(kNestedBulletColumnSetId);
+  nested_column_set->AddPaddingColumn(0, kIndentationBeforeNestedBullet);
+  nested_column_set->AddColumn(views::GridLayout::FILL,
+                               views::GridLayout::LEADING,
+                               0,
+                               views::GridLayout::USE_PREF,
+                               0,
+                               0);
+  nested_column_set->AddPaddingColumn(0, kSpacingBetweenBulletAndStartOfText);
+  nested_column_set->AddColumn(views::GridLayout::FILL,
+                               views::GridLayout::LEADING,
+                               1 /* stretch to fill space */,
+                               views::GridLayout::USE_PREF,
+                               0,
+                               0);
+  nested_column_set->AddPaddingColumn(0, kSpacingBetweenTextAndRevokeButton);
+  nested_column_set->AddColumn(views::GridLayout::FILL,
+                               views::GridLayout::LEADING,
+                               0,
+                               views::GridLayout::USE_PREF,
+                               0,
+                               0);
+}
 
-    // Add a padding row before every item except the first
-    if (it != messages.begin())
-      layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+void AppInfoPermissionsPanel::FillPermissionsList() {
+  // Add regular permission messages.
+  for (const auto& message : GetActivePermissionMessages()) {
+    AddPermissionBullet(false, message, true, gfx::NO_ELIDE, NULL);
+  }
 
-    layout->StartRow(1, kColumnSetId);
-    layout->AddView(bullet_label);
-    layout->AddView(permission_label);
+  // TODO(sashab): Add host permission messages, if the app has any.
+
+  // Add USB devices, if the app has any.
+  if (GetRetainedDeviceCount() > 0) {
+    revoke_device_permissions_button_ = CreateCloseButton(this);
+    AddPermissionBullet(false,
+                        GetRetainedDeviceHeading(),
+                        true,
+                        gfx::NO_ELIDE,
+                        revoke_device_permissions_button_);
+    for (const auto& retained_device : GetRetainedDevices()) {
+      AddPermissionBullet(true, retained_device, false, gfx::ELIDE_TAIL, NULL);
+    }
   }
 
-  return list_view;
+  // Add retained files, if the app has any.
+  if (GetRetainedFileCount() > 0) {
+    revoke_file_permissions_button_ = CreateCloseButton(this);
+    AddPermissionBullet(false,
+                        GetRetainedFileHeading(),
+                        true,
+                        gfx::NO_ELIDE,
+                        revoke_file_permissions_button_);
+    for (const auto& retained_file_path : GetRetainedFilePaths()) {
+      AddPermissionBullet(
+          true, retained_file_path, false, gfx::ELIDE_MIDDLE, NULL);
+    }
+  }
 }
 
-void AppInfoPermissionsPanel::CreateActivePermissionsControl() {
-  active_permissions_heading_ = CreateHeading(
+void AppInfoPermissionsPanel::LayoutPermissionsList() {
+  views::View* vertical_stack = CreateVerticalStack();
+
+  views::View* permissions_heading = CreateHeading(
       l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_APP_PERMISSIONS_TITLE));
-  std::vector<base::string16> permission_strings =
-      GetActivePermissionMessages();
-  if (permission_strings.empty()) {
+  vertical_stack->AddChildView(permissions_heading);
+
+  if (permissions_list_count_ == 0) {
     views::Label* no_permissions_text =
         new views::Label(l10n_util::GetStringUTF16(
             app_->is_extension()
                 ? IDS_APPLICATION_INFO_EXTENSION_NO_PERMISSIONS_TEXT
                 : IDS_APPLICATION_INFO_APP_NO_PERMISSIONS_TEXT));
     no_permissions_text->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    active_permissions_list_ = no_permissions_text;
+    vertical_stack->AddChildView(no_permissions_text);
   } else {
-    active_permissions_list_ =
-        CreateBulletedListView(permission_strings, true, gfx::NO_ELIDE);
-  }
-}
-
-void AppInfoPermissionsPanel::CreateRetainedFilesControl() {
-  const std::vector<base::string16> retained_file_permission_messages =
-      GetRetainedFilePaths();
-
-  if (!retained_file_permission_messages.empty()) {
-    revoke_file_permissions_button_ = new views::LabelButton(
-        this,
-        l10n_util::GetStringUTF16(
-            IDS_APPLICATION_INFO_REVOKE_RETAINED_FILE_PERMISSIONS_BUTTON_TEXT));
-    revoke_file_permissions_button_->SetStyle(views::Button::STYLE_BUTTON);
-
-    retained_files_heading_ = CreateHeading(l10n_util::GetStringUTF16(
-        IDS_APPLICATION_INFO_RETAINED_FILE_PERMISSIONS_TEXT));
-    retained_files_list_ = CreateBulletedListView(
-        retained_file_permission_messages, false, gfx::ELIDE_MIDDLE);
+    vertical_stack->AddChildView(permissions_list_);
   }
+  AddChildView(vertical_stack);
 }
 
-void AppInfoPermissionsPanel::CreateRetainedDevicesControl() {
-  const std::vector<base::string16> retained_device_permission_messages =
-      GetRetainedDevices();
-
-  if (!retained_device_permission_messages.empty()) {
-    revoke_device_permissions_button_ = new views::LabelButton(
-        this,
-        l10n_util::GetStringUTF16(
-            IDS_APPLICATION_INFO_REVOKE_DEVICE_PERMISSIONS_BUTTON_TEXT));
-    revoke_device_permissions_button_->SetStyle(views::Button::STYLE_BUTTON);
-
-    retained_devices_heading_ = CreateHeading(l10n_util::GetStringUTF16(
-        IDS_APPLICATION_INFO_RETAINED_DEVICE_PERMISSIONS_TEXT));
-    retained_devices_list_ = CreateBulletedListView(
-        retained_device_permission_messages, false, gfx::ELIDE_TAIL);
-  }
-}
-
-void AppInfoPermissionsPanel::LayoutActivePermissionsControl() {
-  if (active_permissions_list_) {
-    views::View* vertical_stack = CreateVerticalStack();
-    vertical_stack->AddChildView(active_permissions_heading_);
-    vertical_stack->AddChildView(active_permissions_list_);
-    AddChildView(vertical_stack);
-  }
-}
-
-void AppInfoPermissionsPanel::LayoutRetainedFilesControl() {
-  if (retained_files_list_) {
-    DCHECK(retained_files_heading_);
-    DCHECK(revoke_file_permissions_button_);
-
-    // Add a sub-view so the revoke button is right-aligned.
-    views::View* right_aligned_button = new views::View();
-    views::BoxLayout* right_aligned_horizontal_layout =
-        new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
-    right_aligned_horizontal_layout->set_main_axis_alignment(
-        views::BoxLayout::MAIN_AXIS_ALIGNMENT_END);
-    right_aligned_button->SetLayoutManager(right_aligned_horizontal_layout);
-    right_aligned_button->AddChildView(revoke_file_permissions_button_);
-
-    views::View* vertical_stack = CreateVerticalStack();
-    vertical_stack->AddChildView(retained_files_heading_);
-    vertical_stack->AddChildView(retained_files_list_);
-    vertical_stack->AddChildView(right_aligned_button);
-
-    AddChildView(vertical_stack);
-  }
-}
-
-void AppInfoPermissionsPanel::LayoutRetainedDevicesControl() {
-  if (retained_devices_list_) {
-    DCHECK(retained_devices_heading_);
-    DCHECK(revoke_device_permissions_button_);
-
-    // Add a sub-view so the revoke button is right-aligned.
-    views::View* right_aligned_button = new views::View();
-    views::BoxLayout* right_aligned_horizontal_layout =
-        new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
-    right_aligned_horizontal_layout->set_main_axis_alignment(
-        views::BoxLayout::MAIN_AXIS_ALIGNMENT_END);
-    right_aligned_button->SetLayoutManager(right_aligned_horizontal_layout);
-    right_aligned_button->AddChildView(revoke_device_permissions_button_);
-
-    views::View* vertical_stack = CreateVerticalStack();
-    vertical_stack->AddChildView(retained_devices_heading_);
-    vertical_stack->AddChildView(retained_devices_list_);
-    vertical_stack->AddChildView(right_aligned_button);
-
-    AddChildView(vertical_stack);
-  }
+void AppInfoPermissionsPanel::AddPermissionBullet(
+    bool nested_bullet,
+    const base::string16& message,
+    bool allow_multiline,
+    gfx::ElideBehavior elide_behavior,
+    views::LabelButton* revoke_button) {
+  // Add a padding row before every item except the first.
+  if (permissions_list_count_ != 0)
+    permissions_list_layout_->AddPaddingRow(
+        0, views::kRelatedControlVerticalSpacing);
+
+  // Extract only the bullet from the IDS_EXTENSION_PERMISSION_LINE text.
+  views::Label* bullet_label = new views::Label(l10n_util::GetStringFUTF16(
+      IDS_EXTENSION_PERMISSION_LINE, base::string16()));
+
+  views::Label* permission_label = new views::Label(message);
+  permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  if (allow_multiline)
+    permission_label->SetMultiLine(true);
+  else
+    permission_label->SetElideBehavior(elide_behavior);
+
+  permissions_list_layout_->StartRow(
+      1, nested_bullet ? kNestedBulletColumnSetId : kBulletColumnSetId);
+  permissions_list_layout_->AddView(bullet_label);
+  permissions_list_layout_->AddView(permission_label);
+
+  if (revoke_button != NULL)
+    permissions_list_layout_->AddView(revoke_button);
+  else
+    permissions_list_layout_->SkipColumns(1);
+
+  permissions_list_count_ += 1;
 }
 
 void AppInfoPermissionsPanel::ButtonPressed(views::Button* sender,
@@ -235,11 +229,41 @@ void AppInfoPermissionsPanel::ButtonPressed(views::Button* sender,
   }
 }
 
+int AppInfoPermissionsPanel::GetActivePermissionMessagesCount() const {
+  return GetActivePermissionMessages().size();
+}
+
 const std::vector<base::string16>
 AppInfoPermissionsPanel::GetActivePermissionMessages() const {
   return app_->permissions_data()->GetPermissionMessageStrings();
 }
 
+int AppInfoPermissionsPanel::GetRetainedFileCount() const {
+  if (app_->permissions_data()->HasAPIPermission(
+          extensions::APIPermission::kFileSystem)) {
+    return apps::SavedFilesService::Get(profile_)
+        ->GetAllFileEntries(app_->id())
+        .size();
+  }
+  return 0;
+}
+
+base::string16 AppInfoPermissionsPanel::GetRetainedFileHeading() const {
+  const int kRetainedFilesMessageIDs[6] = {
+      IDS_APPLICATION_INFO_RETAINED_FILES_DEFAULT,
+      IDS_APPLICATION_INFO_RETAINED_FILE_SINGULAR,
+      IDS_APPLICATION_INFO_RETAINED_FILES_ZERO,
+      IDS_APPLICATION_INFO_RETAINED_FILES_TWO,
+      IDS_APPLICATION_INFO_RETAINED_FILES_FEW,
+      IDS_APPLICATION_INFO_RETAINED_FILES_MANY,
+  };
+  std::vector<int> message_ids(
+      kRetainedFilesMessageIDs,
+      kRetainedFilesMessageIDs + arraysize(kRetainedFilesMessageIDs));
+
+  return l10n_util::GetPluralStringFUTF16(message_ids, GetRetainedFileCount());
+}
+
 const std::vector<base::string16>
 AppInfoPermissionsPanel::GetRetainedFilePaths() const {
   std::vector<base::string16> retained_file_paths;
@@ -264,6 +288,29 @@ void AppInfoPermissionsPanel::RevokeFilePermissions() {
   Close();
 }
 
+int AppInfoPermissionsPanel::GetRetainedDeviceCount() const {
+  return extensions::DevicePermissionsManager::Get(profile_)
+      ->GetPermissionMessageStrings(app_->id())
+      .size();
+}
+
+base::string16 AppInfoPermissionsPanel::GetRetainedDeviceHeading() const {
+  const int kRetainedDevicesMessageIDs[6] = {
+      IDS_APPLICATION_INFO_RETAINED_DEVICES_DEFAULT,
+      IDS_APPLICATION_INFO_RETAINED_DEVICE_SINGULAR,
+      IDS_APPLICATION_INFO_RETAINED_DEVICES_ZERO,
+      IDS_APPLICATION_INFO_RETAINED_DEVICES_TWO,
+      IDS_APPLICATION_INFO_RETAINED_DEVICES_FEW,
+      IDS_APPLICATION_INFO_RETAINED_DEVICES_MANY,
+  };
+  std::vector<int> message_ids(
+      kRetainedDevicesMessageIDs,
+      kRetainedDevicesMessageIDs + arraysize(kRetainedDevicesMessageIDs));
+
+  return l10n_util::GetPluralStringFUTF16(message_ids,
+                                          GetRetainedDeviceCount());
+}
+
 const std::vector<base::string16> AppInfoPermissionsPanel::GetRetainedDevices()
     const {
   return extensions::DevicePermissionsManager::Get(profile_)