Replace PermissionType in chrome/ with ContentSettingsType

chrome/browser/permissions/permission_manager.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_PERMISSIONS_PERMISSION_MANAGER_H_
 #define CHROME_BROWSER_PERMISSIONS_PERMISSION_MANAGER_H_
 
 #include <unordered_map>
 
 #include "base/callback_forward.h"
@@ skipping 15 matching lines @@
 
 class PermissionManager : public KeyedService,
                           public content::PermissionManager,
                           public content_settings::Observer {
  public:
   static PermissionManager* Get(Profile* profile);
 
   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,
       content::RenderFrameHost* render_frame_host,
       const GURL& requesting_origin,
       bool user_gesture,
       const base::Callback<void(blink::mojom::PermissionStatus)>& callback)
       override;
   int RequestPermissions(
       const std::vector<content::PermissionType>& permissions,
@@ skipping 18 matching lines @@
       content::PermissionType permission,
       const GURL& requesting_origin,
       const GURL& embedding_origin,
       const base::Callback<void(blink::mojom::PermissionStatus)>& callback)
       override;
   void UnsubscribePermissionStatusChange(int subscription_id) override;
 
   // 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;
 
   class PendingRequest;
   using PendingRequestsMap = IDMap<std::unique_ptr<PendingRequest>>;
 
   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
   // identified by its |permission_id|. If the PendingRequest contains more than
   // one permission, it will wait for the remaining permissions to be resolved.
   // When all the permissions have been resolved, the PendingRequest's callback
   // is run.
   void OnPermissionsRequestResponseStatus(
       int request_id,
       int permission_id,
       blink::mojom::PermissionStatus status);
 
   // content_settings::Observer implementation.
   void OnContentSettingChanged(const ContentSettingsPattern& primary_pattern,
                                const ContentSettingsPattern& secondary_pattern,
                                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_;
 
   DISALLOW_COPY_AND_ASSIGN(PermissionManager);
 };
 
 #endif // CHROME_BROWSER_PERMISSIONS_PERMISSION_MANAGER_H_