Add 'Revoke File Permissions' button to App Info Dialog

chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_tab.h

Index: chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_tab.h
diff --git a/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_tab.h b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_tab.h
index 3fb042b9363b6c2b6854c87b8c044c6b9ea71feb..82392691e14ef075ffa497059a4506b297ab32d2 100644
--- a/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_tab.h
+++ b/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_tab.h
@@ -7,6 +7,7 @@
 
 #include "chrome/browser/ui/views/apps/app_info_dialog/app_info_tab.h"
 #include "ui/gfx/native_widget_types.h"
+#include "ui/views/controls/button/button.h"
 
 class Profile;
 
@@ -14,13 +15,17 @@ namespace extensions {
 class Extension;
 class PermissionSet;
 }
+namespace ui {
+class Event;
+}
 namespace views {
+class LabelButton;
 class ScrollView;
 }
 
 // The Permissions tab of the app info dialog, which provides insight and
 // control over the app's various permissions.
-class AppInfoPermissionsTab : public AppInfoTab {
+class AppInfoPermissionsTab : public AppInfoTab, views::ButtonListener {
  public:
   AppInfoPermissionsTab(gfx::NativeWindow parent_window,
                         Profile* profile,
@@ -42,6 +47,15 @@ class AppInfoPermissionsTab : public AppInfoTab {
   // Overridden from views::View:
   virtual void Layout() OVERRIDE;
 
+  // Overridden from views::ButtonListener:
+  virtual void ButtonPressed(views::Button* sender,
+                             const ui::Event& event) OVERRIDE;
+
+  // Clears all retained file permissions for this app. Must only be called if
+  // CanRevokeFilePermissions() returns true.
+  virtual void RevokeFilePermissions();

[Matt Giuca, 2014/05/09 05:36:47]

Why are these virtual?

[sashab, 2014/05/11 23:45:21]

I don't think anything anything will extend this class, so there's no need for
them to be virtual. Removed.

[Matt Giuca, 2014/05/12 01:02:50]

You shouldn't make methods virtual just because "something might extend this
class" or even "something does extend this class". A method should only be
virtual if you *need* to override it. Every virtual method increases code
complexity:

1. It's bad for performance; every virtual method call has to do an extra
pointer lookup, and defeats optimizations like inlining. But more importantly,
2. It increases the potential amount of behaviour that can happen when you call
the method. When I call a non-virtual method, I just have to consider what that
method does. When I call a virtual method, I don't really know what I am calling
and I have to consider all of the possible behaviour.

You should treat virtual as a tool you use when you need it, not as a modifier
to use whenever it might make sense (like const).

+  virtual bool CanRevokeFilePermissions() const;
+
   const extensions::PermissionSet* GetRequiredPermissions() const;
   const std::vector<base::string16> GetRequiredPermissionMessages() const;
 
@@ -52,6 +66,7 @@ class AppInfoPermissionsTab : public AppInfoTab {
   const std::vector<base::string16> GetRetainedFilePermissionMessages() const;
 
   views::ScrollView* scroll_view_;
+  views::LabelButton* revoke_file_permissions_button_;
 
   DISALLOW_COPY_AND_ASSIGN(AppInfoPermissionsTab);
 };