Split GetPermissionStatus into GetPermissionStatusForFrame/Worker

content/public/browser/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 CONTENT_PUBLIC_BROWSER_PERMISSION_MANAGER_H_
 #define CONTENT_PUBLIC_BROWSER_PERMISSION_MANAGER_H_
 
 #include "content/common/content_export.h"
 #include "third_party/WebKit/public/platform/modules/permissions/permission_status.mojom.h"
 
@@ skipping 45 matching lines @@
       const GURL& requesting_origin,
       bool user_gesture,
       const base::Callback<void(
           const std::vector<blink::mojom::PermissionStatus>&)>& callback) = 0;
 
   // Cancels a previous permission request specified by |request_id|. Cancelling
   // an already cancelled request or providing the |request_id|
   // kNoPendingOperation is a no-op.
   virtual void CancelPermissionRequest(int request_id) = 0;
 
-  // Returns the permission status of a given requesting_origin/embedding_origin
-  // tuple. This is not taking a RenderFrameHost because the call might happen
-  // outside of a frame context.
-  virtual blink::mojom::PermissionStatus GetPermissionStatus(
-      PermissionType permission,
-      const GURL& requesting_origin,
-      const GURL& embedding_origin) = 0;
+  // Returns the PermissionStatus for a specific RenderFrameHost.
+  virtual blink::mojom::PermissionStatus GetPermissionStatusForFrame(

[mlamouri (slow - plz ping), 2017/06/08 13:35:52]

Can we drop "ForFrame" and "ForWorkers" and instead mention that when the caller
has a RFH available, it should pass the RFH? As pointed in some comments in the
CL, the calls can happen for a frame too when the initial caller may or may not
be from a frame.

[raymes, 2017/06/12 03:24:17]

Hmm could you explain what you mean a bit more by:
Which calls are you specifically referring to? 

We could keep it as GetPermissionStatusForFrame/GetPermissionStatus (I think
this is what you're hinting at?). The downside in my mind is that I think there
is a good chance of people misusing it by not passing a frame and then getting
the wrong value. I believe the only time a frame can't be passed in is when the
call is for a worker.

The other advantage of a ForWorker function is that we can impose specific
restrictions that apply in the contexts of workers.

[mlamouri (slow - plz ping), 2017/06/15 13:02:08]

I think johnme@ mentioned one place.
Most users that care about ForFrame/ForWorkers would be coming from the renderer
process/Blink and they would most likely use the permission service that does
that routing automatically. Actually, only push, notifications and background
sync are using content::PermissionManager::GetPermissionStatus() directly (if we
ignore tests and all).

My worry here is that ForWorker will be used in cases where things might or
might not come from a worker, making the naming moot and creating more confusing
than it solves. Though, that's a naming issue so feel free to ignore.

+      content::PermissionType permission,
+      content::RenderFrameHost* render_frame_host,
+      const GURL& requesting_origin) = 0;
+
+  // Returns the PermissionStatus for a worker. This should not be used for
+  // determining the permission status for a request coming from a
+  // RenderFrameHost.
+  virtual blink::mojom::PermissionStatus GetPermissionStatusForWorker(
+      content::PermissionType permission,
+      const GURL& requesting_origin) = 0;

[mlamouri (slow - plz ping), 2017/06/08 13:35:52]

No strong opinion on this but maybe we can keep the requesting/embedding? In
other words, your change would be about adding a new method that take a RFH and
you would only have to change the places that need to be changed. WDYT?

[raymes, 2017/06/12 03:24:17]

(Hopefully addressed in the above comment)

 
   // Sets the permission back to its default for the requesting_origin/
   // embedding_origin tuple.
   virtual void ResetPermission(PermissionType permission,
                                const GURL& requesting_origin,
                                const GURL& embedding_origin) = 0;
 
   // Runs the given |callback| whenever the |permission| associated with the
   // pair { requesting_origin, embedding_origin } changes.
   // Returns the subscription_id to be used to unsubscribe. Can be
   // kNoPendingOperation if the subscribe was not successful.
   virtual int SubscribePermissionStatusChange(
       PermissionType permission,
       const GURL& requesting_origin,
       const GURL& embedding_origin,
       const base::Callback<void(blink::mojom::PermissionStatus)>& callback) = 0;
 
   // Unregisters from permission status change notifications.
   // The |subscription_id| must match the value returned by the
   // SubscribePermissionStatusChange call. Unsubscribing
   // an already unsubscribed |subscription_id| or providing the
   // |subscription_id| kNoPendingOperation is a no-op.
   virtual void UnsubscribePermissionStatusChange(int subscription_id) = 0;
 };
 
 }  // namespace content
 
 #endif // CONTENT_PUBLIC_BROWSER_PERMISSION_MANAGER_H_