Replace PermissionType in chrome/ with ContentSettingsType

chrome/browser/permissions/permission_manager.h

Index: chrome/browser/permissions/permission_manager.h
diff --git a/chrome/browser/permissions/permission_manager.h b/chrome/browser/permissions/permission_manager.h
index bc7e0406f875793d7595f14bc956f43f5f0a0806..3488dbc3e92c1709b1d22c1cae1f4f573f826234 100644
--- a/chrome/browser/permissions/permission_manager.h
+++ b/chrome/browser/permissions/permission_manager.h
@@ -33,6 +33,17 @@ class PermissionManager : public KeyedService,
   explicit PermissionManager(Profile* profile);
   ~PermissionManager() override;
 
+  int RequestPermission(
+      ContentSettingsType permission,
+      content::RenderFrameHost* render_frame_host,
+      const GURL& requesting_origin,
+      bool user_gesture,
+      const base::Callback<void(blink::mojom::PermissionStatus)>& callback);
+  blink::mojom::PermissionStatus GetPermissionStatus(

[raymes, 2017/02/07 04:46:15]

Naming is tricky here (for both these functions). GetContentSetting could be
somewhat confusing for this function.

I think given the tradeoffs here, and after looking at the style guide, we may
want to just keep these named the same way. It's hard to get the usage wrong
because the functions should do exactly the same thing, just with different
types.

We should comment this file appropriately, explaining which versions are for
what purposes. We should also put a comment somewhere explaining more generally
the idea that ContentSettingsType/ContentSetting should be used in chrome and
PermissionType/PermissionStatus should be used in content.

[Timothy Loh, 2017/02/08 04:01:06]

Once we change it to return ContentSetting, I think GetContentSetting will work
as a name.
I added a comment here and a comment above the ContentSettingsType enum. I'm not
sure if the latter will be too useful because people might not see it though.

[raymes, 2017/02/09 00:39:56]

I think we should keep it as GetPermissionStatus (or something similar).
GetContentSetting could be confused as returning the underlying user's decision,
which may or may not be the same result as GetPermissionStatus. 

+      ContentSettingsType permission,
+      const GURL& requesting_origin,
+      const GURL& embedding_origin);
+
   // content::PermissionManager implementation.
   int RequestPermission(
       content::PermissionType permission,
@@ -71,7 +82,7 @@ class PermissionManager : public KeyedService,
   // TODO(raymes): Rather than exposing this, expose a denial reason from
   // GetPermissionStatus so that callers can determine whether a permission is
   // denied due to the kill switch.
-  bool IsPermissionKillSwitchOn(content::PermissionType permission);
+  bool IsPermissionKillSwitchOn(ContentSettingsType);
 
  private:
   friend class GeolocationPermissionContextTests;
@@ -82,7 +93,7 @@ class PermissionManager : public KeyedService,
   struct Subscription;
   using SubscriptionsMap = IDMap<std::unique_ptr<Subscription>>;
 
-  PermissionContextBase* GetPermissionContext(content::PermissionType type);
+  PermissionContextBase* GetPermissionContext(ContentSettingsType type);
 
   // Called when a permission was decided for a given PendingRequest. The
   // PendingRequest is identified by its |request_id| and the permission is
@@ -101,17 +112,13 @@ class PermissionManager : public KeyedService,
                                ContentSettingsType content_type,
                                std::string resource_identifier) override;
 
-  ContentSetting GetPermissionStatusInternal(content::PermissionType permission,
-                                             const GURL& requesting_origin,
-                                             const GURL& embedding_origin);
-
   Profile* profile_;
   PendingRequestsMap pending_requests_;
   SubscriptionsMap subscriptions_;
 
-  std::unordered_map<content::PermissionType,
+  std::unordered_map<ContentSettingsType,
                      std::unique_ptr<PermissionContextBase>,
-                     PermissionTypeHash>
+                     ContentSettingsTypeHash>
       permission_contexts_;
 
   base::WeakPtrFactory<PermissionManager> weak_ptr_factory_;