Fix Guest geolocation API, we were using |bridge_id| and |request_id| interchangeably which is wrong.

A |bridge_id| is specific to geolocation request in chrome/browser/geolocation/* where request_id is BrowserPluginGuest's local id of all types of permission requests.
Fix WebViewTest.GeolocationAPIEmbedderHasAccess* test flakiness, after frustrating hours, I found that ui_test_utils::OverrideGeolocation() is not set up to mock multiple geolocation requests reliably, made tests separate and use cached values for geolocation requests other than the first (/w maxiumAge + timeout = 0).

BUG=225819,223877
TEST=Added tests to cover bridge_id change, using iframe to trigger second geolocation request from different frame (testMultipleBridgeIdAllow).
Fixed WebViewTest.GeolocationAPIEmbedderHasAccess flakiness due to mocking geolocation (ran them separately on each of win_rel, linux_chromeos, mac_rel and linux_rel trybots for 50/100 times).


Review URL: https://chromiumcodereview.appspot.com/13712002

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

1 files changed, 20 insertions, 10 deletions

diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index bd64baf..83b6292 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -539,14 +539,16 @@ void BrowserPluginGuest::AskEmbedderForGeolocationPermission(
     int bridge_id,
     const GURL& requesting_frame,
     const GeolocationCallback& callback) {
-  if (geolocation_request_callback_map_.size() >=
-          kNumMaxOutstandingPermissionRequests) {
+  if (geolocation_request_map_.size() >= kNumMaxOutstandingPermissionRequests) {
     // Deny the geolocation request.
     callback.Run(false);
     return;
   }
   int request_id = next_permission_request_id_++;
-  geolocation_request_callback_map_[request_id] = callback;
+  geolocation_request_map_[request_id] = std::make_pair(callback, bridge_id);
+  DCHECK(bridge_id_to_request_id_map_.find(bridge_id) ==
+         bridge_id_to_request_id_map_.end());
+  bridge_id_to_request_id_map_[bridge_id] = request_id;
 
   base::DictionaryValue request_info;
   request_info.Set(browser_plugin::kURL,
@@ -558,19 +560,27 @@ void BrowserPluginGuest::AskEmbedderForGeolocationPermission(
 }
 
 void BrowserPluginGuest::CancelGeolocationRequest(int bridge_id) {
+  std::map<int, int>::iterator iter =
+      bridge_id_to_request_id_map_.find(bridge_id);
+  if (iter == bridge_id_to_request_id_map_.end())
+    return;
+
+  int request_id = iter->second;
   GeolocationRequestsMap::iterator callback_iter =
-      geolocation_request_callback_map_.find(bridge_id);
-  if (callback_iter != geolocation_request_callback_map_.end())
-    geolocation_request_callback_map_.erase(callback_iter);
+      geolocation_request_map_.find(request_id);
+  if (callback_iter != geolocation_request_map_.end())
+    geolocation_request_map_.erase(callback_iter);
 }
 
 void BrowserPluginGuest::SetGeolocationPermission(int request_id,
                                                   bool allowed) {
   GeolocationRequestsMap::iterator callback_iter =
-      geolocation_request_callback_map_.find(request_id);
-  if (callback_iter != geolocation_request_callback_map_.end()) {
-    callback_iter->second.Run(allowed);
-    geolocation_request_callback_map_.erase(callback_iter);
+      geolocation_request_map_.find(request_id);
+  if (callback_iter != geolocation_request_map_.end()) {
+    GeolocationRequestItem& item = callback_iter->second;
+    item.first.Run(allowed);
+    bridge_id_to_request_id_map_.erase(item.second);
+    geolocation_request_map_.erase(callback_iter);
   }
 }