Audit the last usage of Geolocation and Notification permissions.

content/public/browser/content_browser_client.h

Index: content/public/browser/content_browser_client.h
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index 356c724e59da6db092aa0055e5fa68f3d94432e0..88f66cedabf0cc696d91290e0f3983e24de9f05b 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -474,6 +474,12 @@ class CONTENT_EXPORT ContentBrowserClient {
       base::Callback<void(bool)> result_callback,
       base::Closure* cancel_callback);
 
+  // Informs the content settings host map that a permission has been used.

[Bernhard Bauer, 2014/07/14 10:50:33]

Informs whom? Content settings host map (actually host content settings map) is
not a thing that content/ knows about.

[Daniel Nishi, 2014/07/14 17:12:54]

Removed reference to chrome concept.

+  virtual void UseContentSettingPermission(WebContents* web_contents,

[Bernhard Bauer, 2014/07/14 10:50:33]

I would probably call this "DidUse..." or something. Right now it reads like an
imperative.

[Daniel Nishi, 2014/07/14 17:12:54]

Done.

+                                           const GURL& primary_url,
+                                           const GURL& secondary_url,
+                                           const std::string& setting_type);

[Bernhard Bauer, 2014/07/14 10:50:33]

This is somewhat weird as a content embedder interface. What is the setting
passed here? (Looking at the whole picture, I know that it will be the string
"geolocation", but that's a content setting type name defined in chrome/. The
fact that you need to repeat the constant inside of GeolocationDispatcherHost
should be a bit of a red flag).

We also don't actually use this for arbitrary content setting types. TBH, I
would just make this a specific method for geolocation.

You could also move this to WebContentsObserver. The only reason why other
methods around here that take a WebContents are not on WebContentsObserver is
because there is one particular object that needs to handle them (to decide
about a permission), whereas this is just notifying whoever is interested.

[Daniel Nishi, 2014/07/14 17:12:54]

I'm not sure if WebContentsObserver is the right landing place since the header
implies it is for classes interested in page load events, which a permission use
doesn't seem to be.

It is a good point that it is only used for geolocation. I've changed the method
to be specific.

+
   // Returns true if the given page is allowed to open a window of the given
   // type. If true is returned, |no_javascript_access| will indicate whether
   // the window that is created should be scriptable/in the same process.