Follow-up on https://bugs.webkit.org/show_bug.cgi?id=35031:

Implements cancelGeolocationPermissionRequestForFrame()
Queues infobars.

BUG=39686,39804
TEST=geolocation_browsertest.cc

Review URL: http://codereview.chromium.org/1573002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43368 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 229 insertions, 44 deletions

diff --git a/chrome/browser/geolocation/geolocation_browsertest.cc b/chrome/browser/geolocation/geolocation_browsertest.cc
index 64ae282..84100e0 100644
--- a/chrome/browser/geolocation/geolocation_browsertest.cc
+++ b/chrome/browser/geolocation/geolocation_browsertest.cc
@@ -32,7 +32,7 @@
 // load and wait one single frame here by calling a javascript function.
 class IFrameLoader : public NotificationObserver {
  public:
-   IFrameLoader(Browser* browser, int iframe_id)
+   IFrameLoader(Browser* browser, int iframe_id, const GURL& url)
       : navigation_completed_(false),
         javascript_completed_(false) {
     NavigationController* controller =
@@ -43,8 +43,9 @@ class IFrameLoader : public NotificationObserver {
                    NotificationService::AllSources());
     std::string script = StringPrintf(
         "window.domAutomationController.setAutomationId(0);"
-        "window.domAutomationController.send(addIFrame(%d));",
-        iframe_id);
+        "window.domAutomationController.send(addIFrame(%d, \"%s\"));",
+        iframe_id,
+        url.spec().c_str());
     browser->GetSelectedTabContents()->render_view_host()->
         ExecuteJavascriptInWebFrame(L"", UTF8ToWide(script));
     ui_test_utils::RunMessageLoop();
@@ -217,10 +218,10 @@ class GeolocationBrowserTest : public InProcessBrowserTest {
     } else if (options == INITIALIZATION_IFRAMES) {
       current_browser_ = browser();
       ui_test_utils::NavigateToURL(current_browser_, current_url_);
-      IFrameLoader iframe0(current_browser_, 0);
+      IFrameLoader iframe0(current_browser_, 0, GURL());
       iframe0_url_ = iframe0.iframe_url();
 
-      IFrameLoader iframe1(current_browser_, 1);
+      IFrameLoader iframe1(current_browser_, 1, GURL());
       iframe1_url_ = iframe1.iframe_url();
     } else {
       current_browser_ = browser();
@@ -275,6 +276,7 @@ class GeolocationBrowserTest : public InProcessBrowserTest {
     else
       infobar_->AsConfirmInfoBarDelegate()->Cancel();
     WaitForJSPrompt();
+    tab_contents->RemoveInfoBar(infobar_);
     LOG(WARNING) << "infobar response set";
     infobar_ = NULL;
     content_settings = tab_contents->geolocation_content_settings();
@@ -537,3 +539,39 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest,
   SetInfobarResponse(iframe1_url_, true);
   CheckGeoposition(cached_position);
 }
+
+
+#if defined(OS_MACOSX)
+// TODO(bulach): investigate why this fails on mac. It may be related to:
+// http://crbug.com/29424
+#define MAYBE_CancelPermissionForFrame DISABLED_CancelPermissionForFrame
+#else
+#define MAYBE_CancelPermissionForFrame CancelPermissionForFrame
+#endif
+
+IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest,
+                       MAYBE_CancelPermissionForFrame) {
+  html_for_tests_ = "files/geolocation/iframes_different_origin.html";
+  Initialize(INITIALIZATION_IFRAMES);
+  LOG(WARNING) << "frames loaded";
+
+  iframe_xpath_ = L"//iframe[@id='iframe_0']";
+  AddGeolocationWatch(true);
+  SetInfobarResponse(iframe0_url_, true);
+  CheckGeoposition(MockLocationProvider::instance_->position_);
+  // Disables further prompts from this iframe.
+  CheckStringValueFromJavascript("false", "geoEnableAlerts(false)");
+
+  // Test second iframe from a different origin with a cached geoposition will
+  // create the infobar.
+  iframe_xpath_ = L"//iframe[@id='iframe_1']";
+  AddGeolocationWatch(true);
+
+  int num_infobars_before_cancel =
+      current_browser_->GetSelectedTabContents()->infobar_delegate_count();
+  // Change the iframe, and ensure the infobar is gone.
+  IFrameLoader change_iframe_1(current_browser_, 1, current_url_);
+  int num_infobars_after_cancel =
+      current_browser_->GetSelectedTabContents()->infobar_delegate_count();
+  EXPECT_EQ(num_infobars_before_cancel, num_infobars_after_cancel + 1);
+}
diff --git a/chrome/browser/geolocation/geolocation_dispatcher_host.cc b/chrome/browser/geolocation/geolocation_dispatcher_host.cc
index fb8f531b..f937476 100644
--- a/chrome/browser/geolocation/geolocation_dispatcher_host.cc
+++ b/chrome/browser/geolocation/geolocation_dispatcher_host.cc
@@ -55,6 +55,8 @@ bool GeolocationDispatcherHost::OnMessageReceived(
                         OnRegisterDispatcher)
     IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_UnregisterDispatcher,
                         OnUnregisterDispatcher)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_CancelPermissionRequest,
+                        OnCancelPermissionRequest)
     IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_RequestPermission,
                         OnRequestPermission)
     IPC_MESSAGE_HANDLER(ViewHostMsg_Geolocation_StartUpdating,
@@ -114,6 +116,14 @@ void GeolocationDispatcherHost::OnRequestPermission(
       requesting_frame);
 }
 
+void GeolocationDispatcherHost::OnCancelPermissionRequest(
+    int render_view_id, int bridge_id, const GURL& requesting_frame) {
+  LOG(INFO) << "cancel permission request";
+  geolocation_permission_context_->CancelGeolocationPermissionRequest(
+      resource_message_filter_process_id_, render_view_id, bridge_id,
+      requesting_frame);
+}
+
 void GeolocationDispatcherHost::OnStartUpdating(
     int render_view_id, int bridge_id, const GURL& requesting_frame,
     bool enable_high_accuracy) {
@@ -121,12 +131,10 @@ void GeolocationDispatcherHost::OnStartUpdating(
   // optimize the no-location-available case and reduce latency in the success
   // case (location lookup happens in parallel with the permission request).
   LOG(INFO) << "start updating" << render_view_id;
-  if (!location_arbitrator_) {
-    location_arbitrator_ =
-        geolocation_permission_context_->StartUpdatingRequested(
-            resource_message_filter_process_id_, render_view_id, bridge_id,
-            requesting_frame);
-  }
+  location_arbitrator_ =
+      geolocation_permission_context_->StartUpdatingRequested(
+          resource_message_filter_process_id_, render_view_id, bridge_id,
+          requesting_frame);
   DCHECK(location_arbitrator_);
   location_arbitrator_->AddObserver(
       this, GeolocationArbitrator::UpdateOptions(enable_high_accuracy));
@@ -134,10 +142,11 @@ void GeolocationDispatcherHost::OnStartUpdating(
 
 void GeolocationDispatcherHost::OnStopUpdating(
     int render_view_id, int bridge_id) {
+  // TODO(joth): Balance calls to RemoveObserver here with AddObserver above.
+  // http://crbug.com/40103
   LOG(INFO) << "stop updating" << render_view_id;
-  if (location_arbitrator_)
-    location_arbitrator_->RemoveObserver(this);
-  location_arbitrator_ = NULL;
+  geolocation_permission_context_->StopUpdatingRequested(
+      resource_message_filter_process_id_, render_view_id, bridge_id);
 }
 
 void GeolocationDispatcherHost::OnSuspend(
diff --git a/chrome/browser/geolocation/geolocation_dispatcher_host.h b/chrome/browser/geolocation/geolocation_dispatcher_host.h
index 982e693..8d1e0d9 100644
--- a/chrome/browser/geolocation/geolocation_dispatcher_host.h
+++ b/chrome/browser/geolocation/geolocation_dispatcher_host.h
@@ -45,6 +45,8 @@ class GeolocationDispatcherHost
   void OnUnregisterDispatcher(int render_view_id);
   void OnRequestPermission(
       int render_view_id, int bridge_id, const GURL& requesting_frame);
+  void OnCancelPermissionRequest(
+      int render_view_id, int bridge_id, const GURL& requesting_frame);
   void OnStartUpdating(
       int render_view_id, int bridge_id, const GURL& requesting_frame,
       bool enable_high_accuracy);
diff --git a/chrome/browser/geolocation/geolocation_permission_context.cc b/chrome/browser/geolocation/geolocation_permission_context.cc
index d78b5a7..6c66cdd 100644
--- a/chrome/browser/geolocation/geolocation_permission_context.cc
+++ b/chrome/browser/geolocation/geolocation_permission_context.cc
@@ -27,11 +27,6 @@
 
 namespace {
 
-const FilePath::CharType kGeolocationPermissionPath[] =
-    FILE_PATH_LITERAL("Geolocation");
-
-const wchar_t kAllowedDictionaryKey[] = L"allowed";
-
 // This is the delegate used to display the confirmation info bar.
 class GeolocationConfirmInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
@@ -49,7 +44,11 @@ class GeolocationConfirmInfoBarDelegate : public ConfirmInfoBarDelegate {
   }
 
   // ConfirmInfoBarDelegate
-  virtual void InfoBarClosed() { delete this; }
+  virtual void InfoBarClosed() {
+    context_->OnInfoBarClosed(tab_contents_, render_process_id_,
+                              render_view_id_, bridge_id_);
+    delete this;
+  }
   virtual Type GetInfoBarType() { return INFO_TYPE; }
   virtual bool Accept() { return SetPermission(true); }
   virtual bool Cancel() { return SetPermission(false); }
@@ -106,6 +105,27 @@ class GeolocationConfirmInfoBarDelegate : public ConfirmInfoBarDelegate {
 
 }  // namespace
 
+struct GeolocationPermissionContext::PendingInfoBarRequest {
+  int render_process_id;
+  int render_view_id;
+  int bridge_id;
+  GURL requesting_frame;
+  // If non-NULL, it's the current geolocation infobar for this tab.
+  InfoBarDelegate* infobar_delegate;
+
+  bool IsForTab(int p_render_process_id, int p_render_view_id) const {
+    return render_process_id == p_render_process_id &&
+           render_view_id == p_render_view_id;
+  }
+
+  bool Equals(int p_render_process_id,
+              int p_render_view_id,
+              int p_bridge_id) const {
+    return IsForTab(p_render_process_id, p_render_view_id) &&
+           bridge_id == p_bridge_id;
+  }
+};
+
 GeolocationPermissionContext::GeolocationPermissionContext(
     Profile* profile)
     : profile_(profile) {
@@ -150,11 +170,17 @@ void GeolocationPermissionContext::RequestGeolocationPermission(
     NotifyPermissionSet(render_process_id, render_view_id, bridge_id,
                         requesting_frame, true);
   } else { // setting == ask. Prompt the user.
-    RequestPermissionFromUI(render_process_id, render_view_id, bridge_id,
-                            requesting_frame);
+    RequestPermissionFromUI(tab_contents, render_process_id, render_view_id,
+                            bridge_id, requesting_frame);
   }
 }
 
+void GeolocationPermissionContext::CancelGeolocationPermissionRequest(
+    int render_process_id, int render_view_id, int bridge_id,
+    const GURL& requesting_frame) {
+  CancelPendingInfoBar(render_process_id, render_view_id, bridge_id);
+}
+
 void GeolocationPermissionContext::SetPermission(
     int render_process_id, int render_view_id, int bridge_id,
     const GURL& requesting_frame, bool allowed) {
@@ -189,24 +215,38 @@ GeolocationArbitrator* GeolocationPermissionContext::StartUpdatingRequested(
   return arbitrator;
 }
 
-void GeolocationPermissionContext::RequestPermissionFromUI(
-    int render_process_id, int render_view_id, int bridge_id,
-    const GURL& requesting_frame) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+void GeolocationPermissionContext::StopUpdatingRequested(
+    int render_process_id, int render_view_id, int bridge_id) {
+  CancelPendingInfoBar(render_process_id, render_view_id, bridge_id);
+}
 
-  TabContents* tab_contents =
-      tab_util::GetTabContentsByID(render_process_id, render_view_id);
-  if (!tab_contents) {
-    // The tab may have gone away, or the request may not be from a tab at all.
-    LOG(WARNING) << "Attempt to use geolocation tabless renderer: "
-        << render_process_id << "," << render_view_id << "," << bridge_id
-        << " (geolocation is not supported in extensions)";
-    NotifyPermissionSet(render_process_id, render_view_id, bridge_id,
-                        requesting_frame, false);
-    return;
+void GeolocationPermissionContext::OnInfoBarClosed(
+    TabContents* tab_contents, int render_process_id, int render_view_id,
+    int bridge_id) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
+       i != pending_infobar_requests_.end(); ++i) {
+    if (i->Equals(render_process_id, render_view_id, bridge_id)) {
+      pending_infobar_requests_.erase(i);
+      break;
+    }
   }
-  tab_contents->AddInfoBar(new GeolocationConfirmInfoBarDelegate(tab_contents,
-      this, render_process_id, render_view_id, bridge_id, requesting_frame));
+  // Now process the queued infobars, if any.
+  ShowQueuedInfoBar(tab_contents, render_process_id, render_view_id);
+}
+
+void GeolocationPermissionContext::RequestPermissionFromUI(
+    TabContents* tab_contents, int render_process_id, int render_view_id,
+    int bridge_id, const GURL& requesting_frame) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  PendingInfoBarRequest pending_infobar_request;
+  pending_infobar_request.render_process_id = render_process_id;
+  pending_infobar_request.render_view_id = render_view_id;
+  pending_infobar_request.bridge_id = bridge_id;
+  pending_infobar_request.requesting_frame = requesting_frame;
+  pending_infobar_request.infobar_delegate = NULL;
+  pending_infobar_requests_.push_back(pending_infobar_request);
+  ShowQueuedInfoBar(tab_contents, render_process_id, render_view_id);
 }
 
 void GeolocationPermissionContext::NotifyPermissionSet(
@@ -239,3 +279,49 @@ void GeolocationPermissionContext::NotifyArbitratorPermissionGranted(
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
   GeolocationArbitrator::GetInstance()->OnPermissionGranted(requesting_frame);
 }
+
+void GeolocationPermissionContext::ShowQueuedInfoBar(
+    TabContents* tab_contents, int render_process_id, int render_view_id) {
+  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
+       i != pending_infobar_requests_.end(); ++i) {
+    if (i->IsForTab(render_process_id, render_view_id)) {
+      // Check if already displayed.
+      if (i->infobar_delegate)
+        break;
+      i->infobar_delegate =
+          new GeolocationConfirmInfoBarDelegate(
+              tab_contents, this, render_process_id, render_view_id,
+              i->bridge_id, i->requesting_frame);
+      tab_contents->AddInfoBar(i->infobar_delegate);
+      break;
+    }
+  }
+}
+
+void GeolocationPermissionContext::CancelPendingInfoBar(
+    int render_process_id, int render_view_id, int bridge_id) {
+  if (!ChromeThread::CurrentlyOn(ChromeThread::UI)) {
+    ChromeThread::PostTask(
+        ChromeThread::UI, FROM_HERE,
+        NewRunnableMethod(this,
+            &GeolocationPermissionContext::CancelPendingInfoBar,
+            render_process_id, render_view_id, bridge_id));
+    return;
+  }
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
+       i != pending_infobar_requests_.end(); ++i) {
+    if (i->Equals(render_process_id, render_view_id, bridge_id)) {
+      TabContents* tab_contents =
+          tab_util::GetTabContentsByID(render_process_id, render_view_id);
+      if (tab_contents && i->infobar_delegate) {
+        // Removing an infobar will remove it from the pending vector.
+        tab_contents->RemoveInfoBar(i->infobar_delegate);
+      } else {
+        // Remove it directly from the pending vector.
+        pending_infobar_requests_.erase(i);
+      }
+      break;
+    }
+  }
+}
diff --git a/chrome/browser/geolocation/geolocation_permission_context.h b/chrome/browser/geolocation/geolocation_permission_context.h
index a7c31fa..d8b5fa3 100644
--- a/chrome/browser/geolocation/geolocation_permission_context.h
+++ b/chrome/browser/geolocation/geolocation_permission_context.h
@@ -13,8 +13,10 @@
 class GeolocationDispatcherHost;
 class GURL;
 class GeolocationArbitrator;
+class InfoBarDelegate;
 class Profile;
 class RenderViewHost;
+class TabContents;
 
 // GeolocationPermissionContext manages Geolocation permissions flow,
 // creating UI elements to ask the user for permission when necessary.
@@ -31,13 +33,18 @@ class GeolocationPermissionContext
       int render_process_id, int render_view_id, int bridge_id,
       const GURL& requesting_frame);
 
+  // The render is cancelling a pending permission request.
+  void CancelGeolocationPermissionRequest(
+      int render_process_id, int render_view_id, int bridge_id,
+      const GURL& requesting_frame);
+
   // Called once the user sets the geolocation permission.
   // It'll notify the render via ViewMsg_Geolocation_PermissionSet.
   void SetPermission(
       int render_process_id, int render_view_id, int bridge_id,
       const GURL& requesting_frame, bool allowed);
 
-  // Called when a new render view wants to start receiving location updates.
+  // Called when a geolocation object wants to start receiving location updates.
   // Returns the location arbitrator that the caller (namely, the dispatcher
   // host) will use to receive these updates. The arbitrator is ref counted.
   // This also applies global policy around which location providers may be
@@ -47,14 +54,28 @@ class GeolocationPermissionContext
       int render_process_id, int render_view_id, int bridge_id,
       const GURL& requesting_frame);
 
+  // Called when a geolocation object has stopped. Because we are
+  // short-circuiting permission request (see StartUpdatingRequested above), we
+  // cancel any pending permission in here, since WebKit doesn't know about the
+  // pending permission request and will never call
+  // CancelGeolocationPermissionRequest().
+  void StopUpdatingRequested(
+      int render_process_id, int render_view_id, int bridge_id);
+
+  // Called by the InfoBarDelegate to notify it's closed.
+  void OnInfoBarClosed(
+      TabContents* tab_contents, int render_process_id, int render_view_id,
+      int bridge_id);
+
  private:
+  struct PendingInfoBarRequest;
   friend class base::RefCountedThreadSafe<GeolocationPermissionContext>;
   virtual ~GeolocationPermissionContext();
 
-  // Triggers the associated UI element to request permission.
+  // Triggers (or queues) the associated UI element to request permission.
   void RequestPermissionFromUI(
-      int render_process_id, int render_view_id, int bridge_id,
-      const GURL& requesting_frame);
+      TabContents* tab_contents, int render_process_id, int render_view_id,
+      int bridge_id, const GURL& requesting_frame);
 
   // Notifies whether or not the corresponding render is allowed to use
   // geolocation.
@@ -65,9 +86,19 @@ class GeolocationPermissionContext
   // Calls GeolocationArbitrator::OnPermissionGranted.
   void NotifyArbitratorPermissionGranted(const GURL& requesting_frame);
 
+  // Shows an infobar if there's any pending for this tab.
+  void ShowQueuedInfoBar(TabContents* tab_contents, int render_process_id,
+                         int render_view_id);
+
+  void CancelPendingInfoBar(
+      int render_process_id, int render_view_id, int bridge_id);
+
   // This should only be accessed from the UI thread.
   Profile* const profile_;
 
+  typedef std::vector<PendingInfoBarRequest> PendingInfoBarRequests;
+  PendingInfoBarRequests pending_infobar_requests_;
+
   DISALLOW_COPY_AND_ASSIGN(GeolocationPermissionContext);
 };
 
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index 67fc00f..b112e01 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -2264,6 +2264,13 @@ IPC_BEGIN_MESSAGES(ViewHost)
                        int /* bridge_id */,
                        GURL /* GURL of the frame requesting geolocation */)
 
+  // The |render_view_id| and |bridge_id| representing |GURL| is cancelling its
+  // previous permission request to access geolocation position.
+  IPC_MESSAGE_CONTROL3(ViewHostMsg_Geolocation_CancelPermissionRequest,
+                       int /* render_view_id */,
+                       int /* bridge_id */,
+                       GURL /* GURL of the frame */)
+
   // The |render_view_id| and |bridge_id| requests Geolocation service to start
   // updating.
   // This is an asynchronous call, and the browser process may eventually reply
diff --git a/chrome/renderer/geolocation_dispatcher.cc b/chrome/renderer/geolocation_dispatcher.cc
index 0a543daf6..37732f9 100644
--- a/chrome/renderer/geolocation_dispatcher.cc
+++ b/chrome/renderer/geolocation_dispatcher.cc
@@ -43,6 +43,12 @@ void GeolocationDispatcher::requestPermissionForFrame(
       render_view_->routing_id(), bridge_id, GURL(url)));
 }
 
+void GeolocationDispatcher::cancelPermissionRequestForFrame(
+    int bridge_id, const WebKit::WebURL& url) {
+  render_view_->Send(new ViewHostMsg_Geolocation_CancelPermissionRequest(
+      render_view_->routing_id(), bridge_id, GURL(url)));
+}
+
 void GeolocationDispatcher::startUpdating(
     int bridge_id, const WebKit::WebURL& url, bool enableHighAccuracy) {
   render_view_->Send(new ViewHostMsg_Geolocation_StartUpdating(
diff --git a/chrome/renderer/geolocation_dispatcher.h b/chrome/renderer/geolocation_dispatcher.h
index 8955dfa..6467574 100644
--- a/chrome/renderer/geolocation_dispatcher.h
+++ b/chrome/renderer/geolocation_dispatcher.h
@@ -29,6 +29,8 @@ class GeolocationDispatcher : public WebKit::WebGeolocationService {
 
   // WebKit::WebGeolocationService.
   void requestPermissionForFrame(int bridge_id, const WebKit::WebURL& url);
+  void cancelPermissionRequestForFrame(
+      int bridge_id, const WebKit::WebURL& url);
   void startUpdating(
       int bridge_id, const WebKit::WebURL& url, bool enableHighAccuracy);
   void stopUpdating(int bridge_id);
diff --git a/chrome/test/data/geolocation/iframes_different_origin.html b/chrome/test/data/geolocation/iframes_different_origin.html
index 80d147c..bace494 100755
--- a/chrome/test/data/geolocation/iframes_different_origin.html
+++ b/chrome/test/data/geolocation/iframes_different_origin.html
@@ -7,10 +7,14 @@ function getIFrameSrc(iframe_id) {
   var url = iframe_hosts[iframe_id] + ':' + port + path + '/simple.html';
   return url;
 }
-function addIFrame(iframe_id) {
+function addIFrame(iframe_id, iframe_src) {
   var id = 'iframe_' + iframe_id;
   var iframe = document.getElementById(id);
-  iframe.src = getIFrameSrc(iframe_id);
+  if (iframe_src) {
+    iframe.src = iframe_src;
+  } else {
+    iframe.src = getIFrameSrc(iframe_id);
+  }
   return "" + iframe_id;
 }
 </script>