Clean up the PermissionInfoBarDelegate hierarchy.

chrome/browser/permissions/permission_infobar_delegate.h

Index: chrome/browser/permissions/permission_infobar_delegate.h
diff --git a/chrome/browser/permissions/permission_infobar_delegate.h b/chrome/browser/permissions/permission_infobar_delegate.h
index 53dd55aa18248006a181ac614a222328a10293c8..2b8527b3008763aa51530decf9e3b34f28785e7a 100644
--- a/chrome/browser/permissions/permission_infobar_delegate.h
+++ b/chrome/browser/permissions/permission_infobar_delegate.h
@@ -12,69 +12,44 @@
 #include "chrome/browser/permissions/permission_util.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "components/infobars/core/confirm_infobar_delegate.h"
-#include "content/public/browser/permission_type.h"
-
-class NavigationDetails;
-class Profile;
-
-// Base class for permission infobars, it implements the default behavior
-// so that the accept/deny buttons grant/deny the relevant permission.
-// A basic implementor only needs to implement the methods that
-// provide an icon and a message text to the infobar.
-class PermissionInfobarDelegate : public ConfirmInfoBarDelegate {
+#include "url/gurl.h"
 
+// Base class for permission infobars.

[raymes, 2016/08/17 03:20:43]

nit: fill 80chars or new paragraph

[dominickn, 2016/08/17 03:29:38]

Done.

+// Implementors should inherit from SinglePermissionInfoBarDelegate or
+// GroupedPermissionInfobarDelegate.
+class PermissionInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
   using PermissionSetCallback = base::Callback<void(bool, PermissionAction)>;
-
-  ~PermissionInfobarDelegate() override;
-  ContentSettingsType content_setting() const { return content_settings_type_; }
+  ~PermissionInfoBarDelegate() override;
+  virtual ContentSettingsType GetContentSettingType(int index) const = 0;

[raymes, 2016/08/17 03:20:43]

It seems like we only ever need the first content setting when accessing it
through this interface? Why not just keep the old  API which has the non-virtual
getter and the member?

[dominickn, 2016/08/17 03:29:38]

The GroupedPermissionInfobarDelegate uses the index access, but the old API
doesn't make much sense for Grouped. I concluded that having the virtual API
with DCHECKs was the nicest way to implement this method (we want it on
PermissionInfoBarDelegate because it's accessed via PermissionInfoBar).

If we want to keep the old API we'll need to either change some of the
type-casting (it doesn't make sense to get a single content setting back from
the GroupedInfoBarDelegate) and move it to SinglePermissionInfoBarDelegate.

[raymes, 2016/08/17 03:44:41]

Hmm it's unclear whether the one callsite which currently uses this works
properly for grouped infobars? There are also other similar functions in the
grouped infobar delegate.

 
   // Returns true if the infobar should display a toggle to allow users to
   // opt-out of persisting their accept/deny decision.
-  bool ShouldShowPersistenceToggle() const;
+  virtual bool ShouldShowPersistenceToggle() const;
 
   // Sets whether or not a decided permission should be persisted to content
   // settings.
   void set_persist(bool persist) { persist_ = persist; }
 
-  // ConfirmInfoBarDelegate:
-  base::string16 GetMessageText() const override;
-
  protected:
-  PermissionInfobarDelegate(const GURL& requesting_origin,
-                            content::PermissionType permission_type,
-                            ContentSettingsType content_settings_type,
-                            bool user_gesture,
-                            Profile* profile,
-                            const PermissionSetCallback& callback);
+  explicit PermissionInfoBarDelegate(const GURL& requesting_origin);
 
-  virtual int GetMessageResourceId() const = 0;
+  const GURL& requesting_origin() const { return requesting_origin_; }
+  bool persist() const { return persist_; }
 
  private:
   // ConfirmInfoBarDelegate:
   Type GetInfoBarType() const override;
-  void InfoBarDismissed() override;
-  PermissionInfobarDelegate* AsPermissionInfobarDelegate() override;
-  base::string16 GetButtonLabel(InfoBarButton button) const override;
-  bool Accept() override;
-  bool Cancel() override;
-
-  void SetPermission(bool update_content_setting, PermissionAction decision);
+  PermissionInfoBarDelegate* AsPermissionInfoBarDelegate() override;
 
   GURL requesting_origin_;
-  content::PermissionType permission_type_;
-  ContentSettingsType content_settings_type_;
-  Profile* const profile_;
-  const PermissionSetCallback callback_;
-  bool action_taken_;
-  bool user_gesture_;
   bool persist_;
 
-  DISALLOW_COPY_AND_ASSIGN(PermissionInfobarDelegate);
+  DISALLOW_COPY_AND_ASSIGN(PermissionInfoBarDelegate);
 };
 
 // Implemented in platform-specific code.
 std::unique_ptr<infobars::InfoBar> CreatePermissionInfoBar(
-    std::unique_ptr<PermissionInfobarDelegate> delegate);
+    std::unique_ptr<PermissionInfoBarDelegate> delegate);
 
 #endif  // CHROME_BROWSER_PERMISSIONS_PERMISSION_INFOBAR_DELEGATE_H_