Make delayed-permission-denied-for-multiple-requests pass

webkit/tools/test_shell/test_geolocation_service.cc

Index: webkit/tools/test_shell/test_geolocation_service.cc
diff --git a/webkit/tools/test_shell/test_geolocation_service.cc b/webkit/tools/test_shell/test_geolocation_service.cc
index 50fe3696cee51820598c87d02c0f8c5f0d730e8e..6653ecf1e1701352393e1f3d47ecfa59c3ee1ad6 100644
--- a/webkit/tools/test_shell/test_geolocation_service.cc
+++ b/webkit/tools/test_shell/test_geolocation_service.cc
@@ -7,22 +7,24 @@
 #include "third_party/WebKit/WebKit/chromium/public/WebGeolocationServiceBridge.h"
 
 TestGeolocationService::TestGeolocationService()
-    : allowed_(false) {
+    : allowed_(false),
+      permission_set_(false) {
 }
 
 TestGeolocationService::~TestGeolocationService() {
-
 }
 
 void TestGeolocationService::SetGeolocationPermission(bool allowed) {
   allowed_ = allowed;
+  permission_set_ = true;
+  TryToSendPermissions();
 }
 
 void TestGeolocationService::requestPermissionForFrame(
     int bridgeId, const WebKit::WebURL& url) {
-  pending_permissions_.push_back(std::make_pair(bridgeId, allowed_));
-  permission_timer_.Start(base::TimeDelta::FromMilliseconds(0),
-                          this, &TestGeolocationService::SendPermission);
+  DCHECK(bridges_map_.Lookup(bridgeId)) << "Unknown bridge " << bridgeId;
+  pending_permissions_.push_back(bridgeId);
+  TryToSendPermissions();
 }
 
 int TestGeolocationService::attachBridge(
@@ -32,15 +34,30 @@ int TestGeolocationService::attachBridge(
 
 void TestGeolocationService::detachBridge(int bridgeId) {
   bridges_map_.Remove(bridgeId);
+  std::vector<int>::iterator i = pending_permissions_.begin();
+  while (i != pending_permissions_.end()) {
+    if (*i == bridgeId)
+      pending_permissions_.erase(i);
+    else
+      ++i;
+  }
+}
+
+void TestGeolocationService::TryToSendPermissions() {
+  if (permission_set_ && !permission_timer_.IsRunning())
+    permission_timer_.Start(base::TimeDelta::FromMilliseconds(0),
+                            this, &TestGeolocationService::SendPermission);
 }
 
 void TestGeolocationService::SendPermission() {
-  for (std::vector<std::pair<int, bool> >::const_iterator i =
-      pending_permissions_.begin(); i != pending_permissions_.end(); ++i) {
+  DCHECK(permission_set_);
+  std::vector<int> pending_permissions;
+  pending_permissions.swap(pending_permissions_);
+  for (std::vector<int>::const_iterator i = pending_permissions.begin();
+      i != pending_permissions.end(); ++i) {
     WebKit::WebGeolocationServiceBridge* bridge =
-       bridges_map_.Lookup(i->first);
+       bridges_map_.Lookup(*i);
     DCHECK(bridge);

[bulach, 2010/09/06 11:51:48]

not entirely sure why you're copying the vector, is it because the bridge can be
detached while traversing it? perhaps then remove this DCHECK with an if?

[joth, 2010/09/06 12:08:57]

I'm not copying the vector, I'm swapping it (important difference)
 I just find it cleaner style and easier to reason about this way.

-    bridge->setIsAllowed(i->second);
+    bridge->setIsAllowed(allowed_);
   }
-  pending_permissions_.clear();
 }