Refactor ChromeGeolocationPermissionContext

chrome/browser/geolocation/chrome_geolocation_permission_context.h

Index: chrome/browser/geolocation/chrome_geolocation_permission_context.h
diff --git a/chrome/browser/geolocation/chrome_geolocation_permission_context.h b/chrome/browser/geolocation/chrome_geolocation_permission_context.h
index d0f35cee28235a3ea5a9c586051caddf5035a98e..48149c3c3053a15950506f2dc4947ca65cfabe2c 100644
--- a/chrome/browser/geolocation/chrome_geolocation_permission_context.h
+++ b/chrome/browser/geolocation/chrome_geolocation_permission_context.h
@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_GEOLOCATION_CHROME_GEOLOCATION_PERMISSION_CONTEXT_H_
 #define CHROME_BROWSER_GEOLOCATION_CHROME_GEOLOCATION_PERMISSION_CONTEXT_H_
 
+#include <string>
+
 #include "base/memory/scoped_ptr.h"
 #include "content/public/browser/geolocation_permission_context.h"
 
@@ -18,7 +20,7 @@ class Profile;
 class ChromeGeolocationPermissionContext
     : public content::GeolocationPermissionContext {
  public:
-  explicit ChromeGeolocationPermissionContext(Profile* profile);
+  static ChromeGeolocationPermissionContext* Create(Profile *profile);

[bulach, 2012/10/17 12:59:33]

nit: s/Profile *p/Profile* p/

however, I just chatted with jknotten, and it feels strange to have this factory
method here, since this class can be derived. our suggestion would be:

- add another file, chrome_geolocation_permission_context_factory.h / .cc
- move this method there
- in the implementation, #include both the android and the regular files
- add #ifdefs in the implementation to create the right one..

[Ramya, 2012/10/17 17:38:43]

Agreed. Will make the change in the http://codereview.chromium.org/11188020/
which introduces android implementations.

 
   static void RegisterUserPrefs(PrefService *user_prefs);
 
@@ -35,17 +37,37 @@ class ChromeGeolocationPermissionContext
       int bridge_id,
       const GURL& requesting_frame) OVERRIDE;
 
- private:
+ protected:
+  explicit ChromeGeolocationPermissionContext(Profile* profile);
   virtual ~ChromeGeolocationPermissionContext();
 
-  // Removes any pending InfoBar request.
-  void CancelPendingInfoBarRequest(int render_process_id,
-                                   int render_view_id,
-                                   int bridge_id);
+  // Decide whether the geolocation permission should be granted.
+  // Calls PermissionDecided if permission can be decided non-interactively,
+  // or NotifyPermissionSet if permission decided by presenting an
+  // infobar to the user. Called on the UI thread.
+  virtual void DecidePermission(int render_process_id,
+                                int render_view_id,
+                                int bridge_id,
+                                const GURL& requesting_frame,
+                                const GURL& embedder,
+                                base::Callback<void(bool)> callback);
+
+  // Called when permission is granted without interactively asking
+  // the user. Can be overridden to introduce additional UI flow.
+  // Should ultimately ensure that NotifyPermissionSet is called.
+  // Called on the UI thread.
+  virtual void PermissionDecided(int render_process_id,
+                                 int render_view_id,
+                                 int bridge_id,
+                                 const GURL& requesting_frame,
+                                 const GURL& embedder,
+                                 base::Callback<void(bool)> callback,
+                                 bool allowed);
 
   // Notifies whether or not the corresponding bridge is allowed to use
   // geolocation via
   // GeolocationPermissionContext::SetGeolocationPermissionResponse().
+  // Called on the UI thread.
   void NotifyPermissionSet(int render_process_id,
                            int render_view_id,
                            int bridge_id,
@@ -53,6 +75,22 @@ class ChromeGeolocationPermissionContext
                            base::Callback<void(bool)> callback,
                            bool allowed);
 
+  // Return an instance of the infobar queue controller, creating it
+  // if necessary.
+  GeolocationInfoBarQueueController* QueueController();
+
+  // Create an InfoBarQueueController. overriden in derived classes to provide
+  // additional UI flow.  Called on the UI thread.
+  virtual GeolocationInfoBarQueueController* CreateQueueController();
+
+  Profile* profile() const { return profile_; }

[bulach, 2012/10/17 12:59:33]

nit: I think this method and QueueController() should go up in 44, i.e., all non
virtuals before the virtual methods..

also, please add a similar header as 27 before the virtuals:
  // GeolocationPermissionContext implementation:

[Ramya, 2012/10/17 17:38:43]

Done.

+
+ private:
+  // Removes any pending InfoBar request.
+  void CancelPendingInfoBarRequest(int render_process_id,
+                                   int render_view_id,
+                                   int bridge_id);
+
   // This must only be accessed from the UI thread.
   Profile* const profile_;