Don't show queued infobars on tab shutdown.

BUG=69965
TEST=GeolocationBrowserTest.TabDestroyed,GeolocationPermissionContextTests.TabDestroyed

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

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

chrome/browser/geolocation/geolocation_browsertest.cc

@@ -234,11 +234,6 @@ 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, GURL());
-      iframe0_url_ = iframe0.iframe_url();
-
-      IFrameLoader iframe1(current_browser_, 1, GURL());
-      iframe1_url_ = iframe1.iframe_url();
     } else {
       current_browser_ = browser();
       ui_test_utils::NavigateToURL(current_browser_, current_url_);
@@ -252,6 +247,16 @@ class GeolocationBrowserTest : public InProcessBrowserTest {
     return true;
   }
 
+  void LoadIFrames(int number_iframes) {
+      // Limit to 3 iframes.
+      DCHECK(0 < number_iframes && number_iframes <= 3);
+      iframe_urls_.resize(number_iframes);
+      for (int i = 0; i < number_iframes; ++i) {
+        IFrameLoader loader(current_browser_, i, GURL());
+        iframe_urls_[i] = loader.iframe_url();
+      }
+  }
+
   void AddGeolocationWatch(bool wait_for_infobar) {
     GeolocationNotificationObserver notification_observer(wait_for_infobar);
     notification_observer.AddWatchAndWaitForNotification(
@@ -351,10 +356,8 @@ class GeolocationBrowserTest : public InProcessBrowserTest {
   std::wstring iframe_xpath_;
   // The current url for the top level page.
   GURL current_url_;
-  // If not empty, the GURL for the first iframe.
-  GURL iframe0_url_;
-  // If not empty, the GURL for the second iframe.
-  GURL iframe1_url_;
+  // If not empty, the GURLs for the iframes loaded by LoadIFrames().
+  std::vector<GURL> iframe_urls_;
 
   // TODO(phajdan.jr): Remove after we can ask TestServer whether it is started.
   bool started_test_server_;
@@ -445,11 +448,12 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, NoInfobarForOffTheRecord) {
 IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, IFramesWithFreshPosition) {
   html_for_tests_ = "files/geolocation/iframes_different_origin.html";
   ASSERT_TRUE(Initialize(INITIALIZATION_IFRAMES));
+  LoadIFrames(2);
   LOG(WARNING) << "frames loaded";
 
   iframe_xpath_ = L"//iframe[@id='iframe_0']";
   AddGeolocationWatch(true);
-  SetInfobarResponse(iframe0_url_, true);
+  SetInfobarResponse(iframe_urls_[0], true);
   CheckGeoposition(MockLocationProvider::instance_->position_);
   // Disables further prompts from this iframe.
   CheckStringValueFromJavascript("0", "geoSetMaxNavigateCount(0)");
@@ -475,7 +479,7 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, IFramesWithFreshPosition) {
   // Now go ahead an authorize the second frame.
   iframe_xpath_ = L"//iframe[@id='iframe_1']";
   // Infobar was displayed, allow access and check there's no error code.
-  SetInfobarResponse(iframe1_url_, true);
+  SetInfobarResponse(iframe_urls_[1], true);
   LOG(WARNING) << "Checking position...";
   CheckGeoposition(fresh_position);
   LOG(WARNING) << "...done.";
@@ -484,10 +488,11 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, IFramesWithFreshPosition) {
 IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, IFramesWithCachedPosition) {
   html_for_tests_ = "files/geolocation/iframes_different_origin.html";
   ASSERT_TRUE(Initialize(INITIALIZATION_IFRAMES));
+  LoadIFrames(2);
 
   iframe_xpath_ = L"//iframe[@id='iframe_0']";
   AddGeolocationWatch(true);
-  SetInfobarResponse(iframe0_url_, true);
+  SetInfobarResponse(iframe_urls_[0], true);
   CheckGeoposition(MockLocationProvider::instance_->position_);
 
   // Refresh geoposition, but let's not yet create the watch on the second frame
@@ -509,7 +514,7 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, IFramesWithCachedPosition) {
   // afterwards. We're only interested in the first navigation for the success
   // callback from the cached position.
   CheckStringValueFromJavascript("1", "geoSetMaxNavigateCount(1)");
-  SetInfobarResponse(iframe1_url_, true);
+  SetInfobarResponse(iframe_urls_[1], true);
   CheckGeoposition(cached_position);
 }
 
@@ -518,11 +523,12 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest,
                        DISABLED_CancelPermissionForFrame) {
   html_for_tests_ = "files/geolocation/iframes_different_origin.html";
   ASSERT_TRUE(Initialize(INITIALIZATION_IFRAMES));
+  LoadIFrames(2);
   LOG(WARNING) << "frames loaded";
 
   iframe_xpath_ = L"//iframe[@id='iframe_0']";
   AddGeolocationWatch(true);
-  SetInfobarResponse(iframe0_url_, true);
+  SetInfobarResponse(iframe_urls_[0], true);
   CheckGeoposition(MockLocationProvider::instance_->position_);
   // Disables further prompts from this iframe.
   CheckStringValueFromJavascript("0", "geoSetMaxNavigateCount(0)");
@@ -557,6 +563,7 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, DISABLED_NoInfoBarBeforeStart) {
   // See http://crbug.com/42789
   html_for_tests_ = "files/geolocation/iframes_different_origin.html";
   ASSERT_TRUE(Initialize(INITIALIZATION_IFRAMES));
+  LoadIFrames(2);
   LOG(WARNING) << "frames loaded";
 
   // Access navigator.geolocation, but ensure it won't request permission.
@@ -565,14 +572,14 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, DISABLED_NoInfoBarBeforeStart) {
 
   iframe_xpath_ = L"//iframe[@id='iframe_0']";
   AddGeolocationWatch(true);
-  SetInfobarResponse(iframe0_url_, true);
+  SetInfobarResponse(iframe_urls_[0], true);
   CheckGeoposition(MockLocationProvider::instance_->position_);
   CheckStringValueFromJavascript("0", "geoSetMaxNavigateCount(0)");
 
   // Permission should be requested after adding a watch.
   iframe_xpath_ = L"//iframe[@id='iframe_1']";
   AddGeolocationWatch(true);
-  SetInfobarResponse(iframe1_url_, true);
+  SetInfobarResponse(iframe_urls_[1], true);
   CheckGeoposition(MockLocationProvider::instance_->position_);
 }
 
@@ -601,3 +608,27 @@ IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, TwoWatchesInOneFrame) {
   WaitForNavigation();
   CheckGeoposition(final_position);
 }
+
+IN_PROC_BROWSER_TEST_F(GeolocationBrowserTest, TabDestroyed) {
+  html_for_tests_ = "files/geolocation/tab_destroyed.html";
+  ASSERT_TRUE(Initialize(INITIALIZATION_IFRAMES));
+  LoadIFrames(3);
+
+  iframe_xpath_ = L"//iframe[@id='iframe_0']";
+  AddGeolocationWatch(true);
+
+  iframe_xpath_ = L"//iframe[@id='iframe_1']";
+  AddGeolocationWatch(false);
+
+  iframe_xpath_ = L"//iframe[@id='iframe_2']";
+  AddGeolocationWatch(false);
+
+  std::string script = 
+      "window.domAutomationController.setAutomationId(0);"
+      "window.domAutomationController.send(window.close());";
+  bool result =
+      ui_test_utils::ExecuteJavaScript(
+      current_browser_->GetSelectedTabContents()->render_view_host(),
+      L"", UTF8ToWide(script));
+  EXPECT_EQ(result, true);
+}

chrome/browser/geolocation/geolocation_permission_context.cc

@@ -23,6 +23,9 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/notification_registrar.h"
+#include "chrome/common/notification_source.h"
+#include "chrome/common/notification_type.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "grit/generated_resources.h"
@@ -50,7 +53,7 @@ const char kGeolocationLearnMoreUrl[] =
 // things listening for such notifications.
 // For the time being this class is self-contained and it doesn't seem pulling
 // the notification infrastructure would simplify.
-class GeolocationInfoBarQueueController {
+class GeolocationInfoBarQueueController : NotificationObserver {
  public:
   GeolocationInfoBarQueueController(
       GeolocationPermissionContext* geolocation_permission_context,
@@ -79,6 +82,11 @@ class GeolocationInfoBarQueueController {
       int render_process_id, int render_view_id, int bridge_id,
       const GURL& requesting_frame, const GURL& embedder, bool allowed);
 
+  // NotificationObserver
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
  private:
   struct PendingInfoBarRequest;
   typedef std::vector<PendingInfoBarRequest> PendingInfoBarRequests;
@@ -90,6 +98,8 @@ class GeolocationInfoBarQueueController {
   std::vector<PendingInfoBarRequest>::iterator CancelInfoBarRequestInternal(
       std::vector<PendingInfoBarRequest>::iterator i);
 
+  NotificationRegistrar registrar_;
+
   GeolocationPermissionContext* const geolocation_permission_context_;
   Profile* const profile_;
   // Contains all pending infobar requests.
@@ -307,6 +317,24 @@ void GeolocationInfoBarQueueController::OnPermissionSet(
   }
 }
 
+void GeolocationInfoBarQueueController::Observe(
+    NotificationType type, const NotificationSource& source,
+    const NotificationDetails& details) {
+  registrar_.Remove(this, NotificationType::TAB_CONTENTS_DESTROYED,
+                    source);
+  TabContents* tab_contents = Source<TabContents>(source).ptr();
+  for (PendingInfoBarRequests::iterator i = pending_infobar_requests_.begin();
+       i != pending_infobar_requests_.end();) {
+    if (i->infobar_delegate == NULL &&
+        tab_contents == tab_util::GetTabContentsByID(i->render_process_id,
+                                                     i->render_view_id)) {
+      i = pending_infobar_requests_.erase(i);
+    } else {
+      ++i;
+    }
+  }
+}
+
 void GeolocationInfoBarQueueController::ShowQueuedInfoBar(
     int render_process_id, int render_view_id) {
   TabContents* tab_contents =
@@ -324,6 +352,13 @@ void GeolocationInfoBarQueueController::ShowQueuedInfoBar(
     // Check if already displayed.
     if (i->infobar_delegate)
       break;
+    if (!registrar_.IsRegistered(
+        this, NotificationType::TAB_CONTENTS_DESTROYED,
+        Source<TabContents>(tab_contents))) {
+      registrar_.Add(
+          this, NotificationType::TAB_CONTENTS_DESTROYED,
+          Source<TabContents>(tab_contents));
+    }
     i->infobar_delegate = new GeolocationConfirmInfoBarDelegate(
         tab_contents, this,
         render_process_id, render_view_id,

chrome/browser/geolocation/geolocation_permission_context_unittest.cc

@@ -399,3 +399,33 @@ TEST_F(GeolocationPermissionContextTests, QueuedOriginMultipleTabs) {
 
   extra_tabs_.reset();
 }
+
+TEST_F(GeolocationPermissionContextTests, TabDestroyed) {
+  GURL requesting_frame_0("http://www.example.com/geolocation");
+  GURL requesting_frame_1("http://www.example-2.com/geolocation");
+  EXPECT_EQ(
+      CONTENT_SETTING_ASK,
+      profile()->GetGeolocationContentSettingsMap()->GetContentSetting(
+          requesting_frame_0, requesting_frame_0));
+  EXPECT_EQ(
+      CONTENT_SETTING_ASK,
+      profile()->GetGeolocationContentSettingsMap()->GetContentSetting(
+          requesting_frame_1, requesting_frame_0));
+
+  NavigateAndCommit(requesting_frame_0);
+  EXPECT_EQ(0, contents()->infobar_delegate_count());
+  // Request permission for two frames.
+  geolocation_permission_context_->RequestGeolocationPermission(
+      process_id(), render_id(), bridge_id(), requesting_frame_0);
+  geolocation_permission_context_->RequestGeolocationPermission(
+      process_id(), render_id(), bridge_id() + 1, requesting_frame_1);
+  // Ensure only one infobar is created.
+  EXPECT_EQ(1, contents()->infobar_delegate_count());
+  ConfirmInfoBarDelegate* infobar_0 =
+      contents()->GetInfoBarDelegateAt(0)->AsConfirmInfoBarDelegate();
+  ASSERT_TRUE(infobar_0);
+  string16 text_0 = infobar_0->GetMessageText();
+
+  // Delete the tab contents.
+  DeleteContents();
+}

chrome/test/data/geolocation/tab_destroyed.html

+<html>
+<script>
+var iframe_hosts = ['http://127.0.0.1', 'http://localhost', 'http://127.0.0.1'];
+function getIFrameSrc(iframe_id) {
+  var port = location.port;
+  var path = location.pathname.substring(0, location.pathname.lastIndexOf('/'));
+  var file = 'simple.html';
+  if (iframe_id >= 1)
+     file = 'tab_destroyed_frame.html';
+  var url = iframe_hosts[iframe_id] + ':' + port + path + '/' + file;
+  return url;
+}
+function addIFrame(iframe_id, iframe_src) {
+  var id = 'iframe_' + iframe_id;
+  var iframe = document.getElementById(id);
+  if (iframe_src) {
+    iframe.src = iframe_src;
+  } else {
+    iframe.src = getIFrameSrc(iframe_id);
+  }
+  return "" + iframe_id;
+}
+</script>
+<body>
+<iframe id="iframe_0"></iframe>
+<iframe id="iframe_1"></iframe>
+<iframe id="iframe_2"></iframe>
+Testing Geolocation with iframes.
+</body>
+</html>
+

chrome/test/data/geolocation/tab_destroyed_frame.html

+<html>
+ <head>
+  <script>
+    var last_position = 0;
+    var last_error = 0;
+    var watch_id = 0;
+    var navigation_count = 0;
+    var max_navigation_count = undefined;
+    var iteration = 0;
+    function geoNavigateIfNeeded(msg) {
+      if (max_navigation_count == undefined ||
+          navigation_count++ < max_navigation_count) {
+        ++iteration;
+        document.location.hash = '#' + iteration + ':' + msg;
+      }
+    }
+    function geoSuccessCallback(position) {
+      last_position = position;
+      geoNavigateIfNeeded('geoSuccessCallback');
+    }
+    function geoErrorCallback(error) {
+      last_error = error;
+      geoNavigateIfNeeded('geoErrorCallback');
+    }
+    function geoStart() {
+      watch_id = navigator.geolocation.watchPosition(
+          geoSuccessCallback, geoErrorCallback,
+          {maximumAge:600000, timeout:100000, enableHighAccuracy:true});
+      geoNavigateIfNeeded('geoStart');
+      return watch_id;
+    }
+    function geoGetLastPositionLatitude() {
+      return "" + last_position.coords.latitude;
+    }
+    function geoGetLastPositionLongitude() {
+      return "" + last_position.coords.longitude;
+    }
+    function geoGetLastError() {
+      return "" + (last_error ? last_error.code : 0);
+    }
+    function geoSetMaxNavigateCount(max_navigations) {
+      navigation_count = 0;
+      max_navigation_count = max_navigations;
+      return "" + max_navigation_count;
+    }
+    function geoAccessNavigatorGeolocation() {
+      return "" + typeof(navigator.geolocation);
+    }
+  </script>
+ </head>
+ <body>
+ <input type="button" value="manual" onclick="geoStart()"/>
+ </body>
+</html>
+