Make PermissionManager use ContentSettingsType internally more

chrome/browser/permissions/permission_manager.cc

 // 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.
 
 #include "chrome/browser/permissions/permission_manager.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/callback.h"
@@ skipping 63 matching lines @@
 // ContentSetting.
 void ContentSettingToPermissionStatusCallbackWrapper(
     const base::Callback<void(PermissionStatus)>& callback,
     ContentSetting setting) {
   callback.Run(ContentSettingToPermissionStatus(setting));
 }
 
 // Helper method to convert PermissionType to ContentSettingType.
 ContentSettingsType PermissionTypeToContentSetting(PermissionType permission) {
   switch (permission) {
+    case PermissionType::MIDI:
+      return CONTENT_SETTINGS_TYPE_MIDI;
     case PermissionType::MIDI_SYSEX:
       return CONTENT_SETTINGS_TYPE_MIDI_SYSEX;
     case PermissionType::PUSH_MESSAGING:
       return CONTENT_SETTINGS_TYPE_PUSH_MESSAGING;
     case PermissionType::NOTIFICATIONS:
       return CONTENT_SETTINGS_TYPE_NOTIFICATIONS;
     case PermissionType::GEOLOCATION:
       return CONTENT_SETTINGS_TYPE_GEOLOCATION;
     case PermissionType::PROTECTED_MEDIA_IDENTIFIER:
 #if defined(OS_ANDROID) || defined(OS_CHROMEOS)
       return CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER;
 #else
       NOTIMPLEMENTED();
       break;
 #endif
     case PermissionType::DURABLE_STORAGE:
       return CONTENT_SETTINGS_TYPE_DURABLE_STORAGE;
-    case PermissionType::MIDI:
-      // This will hit the NOTREACHED below.
-      break;
     case PermissionType::AUDIO_CAPTURE:
       return CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC;
     case PermissionType::VIDEO_CAPTURE:
       return CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA;
     case PermissionType::BACKGROUND_SYNC:
       return CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC;
     case PermissionType::FLASH:
       return CONTENT_SETTINGS_TYPE_PLUGINS;
     case PermissionType::NUM:
       // This will hit the NOTREACHED below.
       break;
   }
 
   NOTREACHED() << "Unknown content setting for permission "
                << static_cast<int>(permission);
   return CONTENT_SETTINGS_TYPE_DEFAULT;
 }
 
 // Returns whether the permission has a constant PermissionStatus value (i.e.
 // always approved or always denied)
-// The PermissionTypes for which true is returned should be exactly those which
-// return nullptr in PermissionManager::GetPermissionContext since they don't
-// have a context.
-bool IsConstantPermission(PermissionType type) {
+// The ContentSettingsTypes for which true is returned will also return nullptr
+// in PermissionManager::GetPermissionContext since they don't have a context.
+bool IsConstantPermission(ContentSettingsType type) {
   switch (type) {
-    case PermissionType::MIDI:
+    case CONTENT_SETTINGS_TYPE_MIDI:
       return true;
     default:
       return false;
   }

[raymes, 2017/02/23 05:37:08]

nit: I wonder if we should just make this:
return type == CONTENT_SETTINGS_TYPE_MIDI;

[Timothy Loh, 2017/02/23 06:17:10]

Done.

 }
 
 void PermissionRequestResponseCallbackWrapper(
     const base::Callback<void(PermissionStatus)>& callback,
     const std::vector<PermissionStatus>& vector) {
   DCHECK_EQ(vector.size(), 1ul);
   callback.Run(vector[0]);
 }
 
 // Function used for handling permission types which do not change their
 // value i.e. they are always approved or always denied etc.
 // CONTENT_SETTING_DEFAULT is returned if the permission needs further handling.
 // This function should only be called when IsConstantPermission has returned
 // true for the PermissionType.
 blink::mojom::PermissionStatus GetPermissionStatusForConstantPermission(
-    PermissionType type) {
+    ContentSettingsType type) {
   DCHECK(IsConstantPermission(type));
   switch (type) {
-    case PermissionType::MIDI:
+    case CONTENT_SETTINGS_TYPE_MIDI:
       return PermissionStatus::GRANTED;
     default:
       return PermissionStatus::DENIED;
   }
 }
 
 }  // anonymous namespace
 
 class PermissionManager::PendingRequest {
  public:
-  PendingRequest(content::RenderFrameHost* render_frame_host,
-                 const std::vector<PermissionType> permissions,
-                 const base::Callback<
-                     void(const std::vector<PermissionStatus>&)>& callback)
+  PendingRequest(
+      content::RenderFrameHost* render_frame_host,
+      const std::vector<ContentSettingsType>& permissions,
+      const base::Callback<void(const std::vector<PermissionStatus>&)>&
+          callback)
       : render_process_id_(render_frame_host->GetProcess()->GetID()),
         render_frame_id_(render_frame_host->GetRoutingID()),
         callback_(callback),
         permissions_(permissions),
         results_(permissions.size(), PermissionStatus::DENIED),
         remaining_results_(permissions.size()) {}
 
   void SetPermissionStatus(int permission_id, PermissionStatus status) {
     DCHECK(!IsComplete());
 
     results_[permission_id] = status;
     --remaining_results_;
   }
 
   bool IsComplete() const {
     return remaining_results_ == 0;
   }
 
   int render_process_id() const { return render_process_id_; }
   int render_frame_id() const { return render_frame_id_; }
 
   const base::Callback<void(const std::vector<PermissionStatus>&)>
   callback() const {
     return callback_;
   }
 
-  std::vector<PermissionType> permissions() const {
+  std::vector<ContentSettingsType> permissions() const {
     return permissions_;
   }
 
   std::vector<PermissionStatus> results() const {
     return results_;
   }
 
  private:
   int render_process_id_;
   int render_frame_id_;
   const base::Callback<void(const std::vector<PermissionStatus>&)> callback_;
-  std::vector<PermissionType> permissions_;
+  std::vector<ContentSettingsType> permissions_;
   std::vector<PermissionStatus> results_;
   size_t remaining_results_;
 };
 
 struct PermissionManager::Subscription {
-  PermissionType permission;
+  ContentSettingsType permission;
   GURL requesting_origin;
   GURL embedding_origin;
   base::Callback<void(PermissionStatus)> callback;
   PermissionStatus current_value;
 };
 
 // static
 PermissionManager* PermissionManager::Get(Profile* profile) {
   return PermissionManagerFactory::GetForProfile(profile);
 }
@@ skipping 41 matching lines @@
     HostContentSettingsMapFactory::GetForProfile(profile_)
         ->RemoveObserver(this);
 }
 
 int PermissionManager::RequestPermission(
     ContentSettingsType content_settings_type,
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
     bool user_gesture,
     const base::Callback<void(PermissionStatus)>& callback) {
-  // TODO(timloh): We should be operating on ContentSettingsType instead of
-  // converting these back to PermissionType.
-  PermissionType permission_type;
-  bool success = PermissionUtil::GetPermissionType(content_settings_type,
-                                                   &permission_type);
-  DCHECK(success);
   return RequestPermissions(
-      std::vector<PermissionType>(1, permission_type), render_frame_host,
-      requesting_origin, user_gesture,
+      std::vector<ContentSettingsType>(1, content_settings_type),
+      render_frame_host, requesting_origin, user_gesture,
       base::Bind(&PermissionRequestResponseCallbackWrapper, callback));
 }
 
 PermissionStatus PermissionManager::GetPermissionStatus(
     ContentSettingsType permission,
     const GURL& requesting_origin,
     const GURL& embedding_origin) {
+  if (IsConstantPermission(permission))
+    return GetPermissionStatusForConstantPermission(permission);
   PermissionContextBase* context = GetPermissionContext(permission);
   return ContentSettingToPermissionStatus(
       context->GetPermissionStatus(requesting_origin.GetOrigin(),
                                    embedding_origin.GetOrigin())
       .content_setting);
 }
 
 int PermissionManager::RequestPermission(
     PermissionType permission,
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
     bool user_gesture,
     const base::Callback<void(PermissionStatus)>& callback) {
+  ContentSettingsType content_settings_type =
+      PermissionTypeToContentSetting(permission);
   return RequestPermissions(
-      std::vector<PermissionType>(1, permission),
-      render_frame_host,
-      requesting_origin,
-      user_gesture,
+      std::vector<ContentSettingsType>(1, content_settings_type),
+      render_frame_host, requesting_origin, user_gesture,
       base::Bind(&PermissionRequestResponseCallbackWrapper, callback));
 }
 
 int PermissionManager::RequestPermissions(
     const std::vector<PermissionType>& permissions,
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin,
     bool user_gesture,
     const base::Callback<void(
         const std::vector<PermissionStatus>&)>& callback) {
+  std::vector<ContentSettingsType> content_settings_types;
+  std::transform(permissions.begin(), permissions.end(),
+                 back_inserter(content_settings_types),
+                 PermissionTypeToContentSetting);
+  return RequestPermissions(content_settings_types, render_frame_host,
+                            requesting_origin, user_gesture, callback);
+}
+
+int PermissionManager::RequestPermissions(
+    const std::vector<ContentSettingsType>& permissions,
+    content::RenderFrameHost* render_frame_host,
+    const GURL& requesting_origin,
+    bool user_gesture,
+    const base::Callback<void(const std::vector<PermissionStatus>&)>&
+        callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (permissions.empty()) {
     callback.Run(std::vector<PermissionStatus>());
     return kNoPendingOperation;
   }
 
   content::WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(render_frame_host);
   GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();
 
   int request_id = pending_requests_.Add(base::MakeUnique<PendingRequest>(
       render_frame_host, permissions, callback));
 
   const PermissionRequestID request(render_frame_host, request_id);
 
   for (size_t i = 0; i < permissions.size(); ++i) {
-    const PermissionType permission = permissions[i];
+    const ContentSettingsType permission = permissions[i];
 
-    if (IsConstantPermission(permission) ||
-        !GetPermissionContext(PermissionTypeToContentSetting(permission))) {
+    if (IsConstantPermission(permission) || !GetPermissionContext(permission)) {
       // Track permission request usages even for constant permissions.
       PermissionUmaUtil::PermissionRequested(permission, requesting_origin,
                                              embedding_origin, profile_);
       OnPermissionsRequestResponseStatus(
           request_id, i, GetPermissionStatusForConstantPermission(permission));
       continue;
     }
 
-    PermissionContextBase* context =
-        GetPermissionContext(PermissionTypeToContentSetting(permission));
+    PermissionContextBase* context = GetPermissionContext(permission);
     context->RequestPermission(
         web_contents, request, requesting_origin, user_gesture,
         base::Bind(&ContentSettingToPermissionStatusCallbackWrapper,
             base::Bind(&PermissionManager::OnPermissionsRequestResponseStatus,
                 weak_ptr_factory_.GetWeakPtr(), request_id, i)));
   }
 
   // The request might have been resolved already.
   if (!pending_requests_.Lookup(request_id))
     return kNoPendingOperation;
 
   return request_id;
 }

[raymes, 2017/02/23 05:37:08]

nit: I think the order of these functions is all out of order with the header,
but we should at least try to make new ones fit in place so perhaps move this
under GetPermissionContext below?

[Timothy Loh, 2017/02/23 06:17:10]

As discussed, I've made this new function public and moved it up next to the
RequestPermission(ContentSettingsType, ...) function.

 
 PermissionContextBase* PermissionManager::GetPermissionContext(
     ContentSettingsType type) {
   const auto& it = permission_contexts_.find(type);
   return it == permission_contexts_.end() ? nullptr : it->second.get();
 }
 
 void PermissionManager::OnPermissionsRequestResponseStatus(
     int request_id,
     int permission_id,
@@ skipping 14 matching lines @@
   if (!pending_request)
     return;
 
   content::WebContents* web_contents = tab_util::GetWebContentsByFrameID(
       pending_request->render_process_id(), pending_request->render_frame_id());
   DCHECK(web_contents);
 
   const PermissionRequestID request(pending_request->render_process_id(),
                                     pending_request->render_frame_id(),
                                     request_id);
-  for (PermissionType permission : pending_request->permissions()) {
-    PermissionContextBase* context =
-        GetPermissionContext(PermissionTypeToContentSetting(permission));
+  for (ContentSettingsType permission : pending_request->permissions()) {
+    PermissionContextBase* context = GetPermissionContext(permission);
     if (!context)
       continue;
     context->CancelPermissionRequest(web_contents, request);
   }
   pending_requests_.Remove(request_id);
 }
 
 void PermissionManager::ResetPermission(PermissionType permission,
                                         const GURL& requesting_origin,
                                         const GURL& embedding_origin) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   PermissionContextBase* context =
       GetPermissionContext(PermissionTypeToContentSetting(permission));
   if (!context)
     return;
 
   context->ResetPermission(requesting_origin.GetOrigin(),
                            embedding_origin.GetOrigin());
 }
 
 PermissionStatus PermissionManager::GetPermissionStatus(
     PermissionType permission,
     const GURL& requesting_origin,
     const GURL& embedding_origin) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (IsConstantPermission(permission))
-    return GetPermissionStatusForConstantPermission(permission);
   return GetPermissionStatus(PermissionTypeToContentSetting(permission),
                              requesting_origin, embedding_origin);
 }
 
 int PermissionManager::SubscribePermissionStatusChange(
     PermissionType permission,
     const GURL& requesting_origin,
     const GURL& embedding_origin,
     const base::Callback<void(PermissionStatus)>& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (subscriptions_.IsEmpty())
     HostContentSettingsMapFactory::GetForProfile(profile_)->AddObserver(this);
 
+  ContentSettingsType content_type = PermissionTypeToContentSetting(permission);
   auto subscription = base::MakeUnique<Subscription>();
-  subscription->permission = permission;
+  subscription->permission = content_type;
   subscription->requesting_origin = requesting_origin;
   subscription->embedding_origin = embedding_origin;
   subscription->callback = callback;
 
   subscription->current_value =
-      GetPermissionStatus(permission, requesting_origin, embedding_origin);
+      GetPermissionStatus(content_type, requesting_origin, embedding_origin);
 
   return subscriptions_.Add(std::move(subscription));
 }
 
 void PermissionManager::UnsubscribePermissionStatusChange(int subscription_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // Whether |subscription_id| is known will be checked by the Remove() call.
   subscriptions_.Remove(subscription_id);
 
   if (subscriptions_.IsEmpty())
@@ skipping 13 matching lines @@
     ContentSettingsType content_type,
     std::string resource_identifier) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   std::list<base::Closure> callbacks;
 
   for (SubscriptionsMap::iterator iter(&subscriptions_);
        !iter.IsAtEnd(); iter.Advance()) {
     Subscription* subscription = iter.GetCurrentValue();
     if (IsConstantPermission(subscription->permission))
       continue;
-    if (PermissionTypeToContentSetting(subscription->permission) !=
-        content_type) {
+    if (subscription->permission != content_type)
       continue;
-    }
 
     if (primary_pattern.IsValid() &&
         !primary_pattern.Matches(subscription->requesting_origin))
       continue;
     if (secondary_pattern.IsValid() &&
         !secondary_pattern.Matches(subscription->embedding_origin))
       continue;
 
     PermissionStatus new_value = GetPermissionStatus(
         subscription->permission, subscription->requesting_origin,
         subscription->embedding_origin);
     if (subscription->current_value == new_value)
       continue;
 
     subscription->current_value = new_value;
 
     // Add the callback to |callbacks| which will be run after the loop to
     // prevent re-entrance issues.
     callbacks.push_back(base::Bind(subscription->callback, new_value));
   }
 
   for (const auto& callback : callbacks)
     callback.Run();
 }