Split GetPermissionStatus into GetPermissionStatusForFrame/Worker

content/public/browser/permission_manager.h

Index: content/public/browser/permission_manager.h
diff --git a/content/public/browser/permission_manager.h b/content/public/browser/permission_manager.h
index 54de7a2ad9beb309917fb6da914f392baa663b7c..a5e62b49c9af8a295b78b8373eeb7f9876587cd3 100644
--- a/content/public/browser/permission_manager.h
+++ b/content/public/browser/permission_manager.h
@@ -63,13 +63,18 @@ class CONTENT_EXPORT PermissionManager {
   // 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.