Add InfoBarIdentifier to GroupedPermissionInfoBarDelegate

chrome/browser/permissions/grouped_permission_infobar_delegate_android.h

Index: chrome/browser/permissions/grouped_permission_infobar_delegate_android.h
diff --git a/chrome/browser/permissions/grouped_permission_infobar_delegate.h b/chrome/browser/permissions/grouped_permission_infobar_delegate_android.h
similarity index 61%
rename from chrome/browser/permissions/grouped_permission_infobar_delegate.h
rename to chrome/browser/permissions/grouped_permission_infobar_delegate_android.h
index 70dc48d5dbf3c514d40cca8242c8411a79097512..3635096819a5b5c367ffdb9a56990ee0b914f305 100644
--- a/chrome/browser/permissions/grouped_permission_infobar_delegate.h
+++ b/chrome/browser/permissions/grouped_permission_infobar_delegate_android.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_PERMISSIONS_GROUPED_PERMISSION_INFOBAR_DELEGATE_H_
-#define CHROME_BROWSER_PERMISSIONS_GROUPED_PERMISSION_INFOBAR_DELEGATE_H_
+#ifndef CHROME_BROWSER_PERMISSIONS_GROUPED_PERMISSION_INFOBAR_DELEGATE_ANDROID_H_
+#define CHROME_BROWSER_PERMISSIONS_GROUPED_PERMISSION_INFOBAR_DELEGATE_ANDROID_H_
 
 #include <memory>
 
@@ -14,49 +14,51 @@
 class GURL;
 class InfoBarService;
 
-namespace infobars {
-class InfoBarManager;
-}
-
-// Configures an InfoBar to display a group of permission requests, each of
+// An InfoBar that displays a group of permission requests, each of
 // which can be allowed or blocked independently.

[Peter Kasting, 2016/09/23 00:42:06]

Nit: Wrap at 80 columns

[lshang, 2016/09/27 01:50:42]

Done.

 // TODO(tsergeant): Expand this class so it can be used without subclassing.
 class GroupedPermissionInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
-  // Implementation is in platform-specific infobar file.
-  static std::unique_ptr<infobars::InfoBar> CreateInfoBar(
-      infobars::InfoBarManager* infobar_manager,
-      std::unique_ptr<GroupedPermissionInfoBarDelegate> delegate);
+  ~GroupedPermissionInfoBarDelegate() override;
 
-  GroupedPermissionInfoBarDelegate(
+  static infobars::InfoBar* Create(
+      InfoBarService* infobar_service,
       const GURL& requesting_origin,
       const std::vector<ContentSettingsType>& types);
-  ~GroupedPermissionInfoBarDelegate() override;
 
-  bool ShouldShowPersistenceToggle() const;
   bool persist() const { return persist_; }
   void set_persist(bool persist) { persist_ = persist; }
+  size_t permission_count() const { return types_.size(); }
 
-  base::string16 GetMessageText() const override;
+  // Returns true if the infobar should display a toggle to allow users to
+  // opt-out of persisting their accept/deny decision.
+  bool ShouldShowPersistenceToggle() const;
+
+  ContentSettingsType GetContentSettingType(size_t position) const;
+  int GetIconIdForPermission(size_t position) const;
 
-  int GetPermissionCount() const;
-  ContentSettingsType GetContentSettingType(int index) const;
-  int GetIconIdForPermission(int index) const;
-  // Message text to display for an individual permission at |index|.
-  base::string16 GetMessageTextFragment(int index) const;
+  // Message text to display for an individual permission at |position|.

[Peter Kasting, 2016/09/23 00:42:06]

Nit: Probably no need for this comment or the next one, as their meanings are
basically clear from their names, but if you do keep these comments, or add them
for the things above, use parallel structure for all comments, e.g. "Toggle
Allow/Block for an individual permission at |position|."

[lshang, 2016/09/27 01:50:42]

Done.

+  base::string16 GetMessageTextFragment(size_t position) const;
 
-  void ToggleAccept(int position, bool new_value);
+  // Toggle Allow/Block for each permission type.
+  void ToggleAccept(size_t position, bool new_value);

[Peter Kasting, 2016/09/23 00:42:05]

Nit: The function name here says "accept" and the comment says "allow/block". 
Use consistent language everywhere.

[lshang, 2016/09/27 01:50:43]

Done.

+
+  // ConfirmInfoBarDelegate:
+  base::string16 GetMessageText() const override;
 
  protected:
-  bool GetAcceptState(int position);
+  bool GetAcceptState(size_t position);
 
  private:
-  // ConfirmInfoBarDelegate:
-  base::string16 GetButtonLabel(InfoBarButton button) const override;
-  int GetButtons() const override;
+  GroupedPermissionInfoBarDelegate(
+      const GURL& requesting_origin,
+      const std::vector<ContentSettingsType>& types);
 
-  // InfoBarDelegate:
+  // ConfirmInfoBarDelegate:
+  InfoBarIdentifier GetIdentifier() const override;
   Type GetInfoBarType() const override;
+  int GetButtons() const override;
+  base::string16 GetButtonLabel(InfoBarButton button) const override;
 
   const GURL requesting_origin_;
   const std::vector<ContentSettingsType> types_;
@@ -66,4 +68,4 @@ class GroupedPermissionInfoBarDelegate : public ConfirmInfoBarDelegate {
   DISALLOW_COPY_AND_ASSIGN(GroupedPermissionInfoBarDelegate);
 };
 
-#endif  // CHROME_BROWSER_PERMISSIONS_GROUPED_PERMISSION_INFOBAR_DELEGATE_H_
+#endif  // CHROME_BROWSER_PERMISSIONS_GROUPED_PERMISSION_INFOBAR_DELEGATE_ANDROID_H_