Add request_id to HttpRequestInfo and pass it to the NetworkDelegate for events.

chrome/browser/extensions/extension_webrequest_api.cc

Index: chrome/browser/extensions/extension_webrequest_api.cc
diff --git a/chrome/browser/extensions/extension_webrequest_api.cc b/chrome/browser/extensions/extension_webrequest_api.cc
index 5459a2a6f3a180ebb66ecd0a1f8ebbeecc6c63ee..3798b953ce0424180143f7a0bd08063d064882da 100644
--- a/chrome/browser/extensions/extension_webrequest_api.cc
+++ b/chrome/browser/extensions/extension_webrequest_api.cc
@@ -17,6 +17,7 @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_extent.h"
 #include "chrome/common/extensions/url_pattern.h"
+#include "chrome/common/url_constants.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
 #include "content/browser/renderer_host/resource_dispatcher_host_request_info.h"
@@ -32,6 +33,7 @@ namespace {
 static const char *kWebRequestEvents[] = {

[willchan no longer on Chromium, 2011/03/26 01:46:49]

Where's OnRequestDeleted?

Nit: FYI, these should probably be static const char* const kWebRequestEvents.
That extra const will let the compiler optimize away an extra load. Also, things
like strlen(kWebRequestEvents[0]) become compile-time constants rather than
being dynamically computed at run-time.

[Matt Perry, 2011/03/28 22:51:01]

These are only the extension-visible events. Extensions don't get notified when
a request is deleted.
Done. Thanks for the tip.

[willchan no longer on Chromium, 2011/03/29 20:46:27]

OK, I defer to you here. Just noting that this means that an extension shouldn't
keep a map of requests, because the lack of a request deletion notification
means that an extension would not know when to remove a request from the map,
and thus an extension would leak. I think we may have touched upon this before
and you had a good answer, but I forget. It doesn't seem obvious to me how this
would be addressed.

   keys::kOnBeforeRedirect,
   keys::kOnBeforeRequest,
+  keys::kOnBeforeSendHeaders,
   keys::kOnCompleted,
   keys::kOnErrorOccurred,
   keys::kOnHeadersReceived,
@@ -155,7 +157,7 @@ struct ExtensionWebRequestEventRouter::ExtraInfoSpec {
     RESPONSE_HEADERS = 1<<3,
     REDIRECT_REQUEST_LINE = 1<<4,
     REDIRECT_REQUEST_HEADERS = 1<<5,
-    BLOCKING = 1<<5,
+    BLOCKING = 1<<6,
   };
 
   static bool InitFromValue(const ListValue& value, int* extra_info_spec);
@@ -280,14 +282,14 @@ ExtensionWebRequestEventRouter::ExtensionWebRequestEventRouter() {
 ExtensionWebRequestEventRouter::~ExtensionWebRequestEventRouter() {
 }
 
-bool ExtensionWebRequestEventRouter::OnBeforeRequest(
+int ExtensionWebRequestEventRouter::OnBeforeRequest(
     ProfileId profile_id,
     ExtensionEventRouterForwarder* event_router,
     net::URLRequest* request,
     net::CompletionCallback* callback) {
   // TODO(jochen): Figure out what to do with events from the system context.
   if (profile_id == Profile::kInvalidProfileId)
-    return false;
+    return net::OK;
 
   int tab_id = -1;
   int window_id = -1;
@@ -298,7 +300,14 @@ bool ExtensionWebRequestEventRouter::OnBeforeRequest(
       GetMatchingListeners(profile_id, keys::kOnBeforeRequest, request->url(),
                            tab_id, window_id, resource_type);
   if (listeners.empty())
-    return false;
+    return net::OK;
+
+  // If this is an HTTP request, keep track of it. HTTP-specific events only
+  // have the request ID, so we'll need to look up the URLRequest from that.
+  if (request->url().SchemeIs(chrome::kHttpScheme) ||
+      request->url().SchemeIs(chrome::kHttpsScheme)) {
+    http_requests_[request->identifier()] = request;

[willchan no longer on Chromium, 2011/03/26 01:46:49]

As discussed previously, this may include more than just URLRequestHttpJob.
AppCacheURLRequestJob notably will be caught. You may want to edit that one too,
although that's an edge case.

[Matt Perry, 2011/03/28 22:51:01]

OK. I think it doesn't affect anything if these are caught, since they'll just
sit in the map until they are deleted. Is there a way to tell whether it's an
AppCache job at this point?

[willchan no longer on Chromium, 2011/03/29 20:46:27]

No there isn't. I suspect that eventually you'll want to edit
AppCacheURLRequestJob to call NetworkDelegate at the appropriate points.

+  }
 
   ListValue args;
   DictionaryValue* dict = new DictionaryValue();
@@ -310,18 +319,73 @@ bool ExtensionWebRequestEventRouter::OnBeforeRequest(
   dict->SetString(keys::kTypeKey, ResourceTypeToString(resource_type));
   args.Append(dict);
 
+  if (DispatchEvent(profile_id, event_router, request, callback, listeners,
+                    args))
+    return net::ERR_IO_PENDING;
+  return net::OK;
+}
+
+int ExtensionWebRequestEventRouter::OnBeforeSendHeaders(
+    ProfileId profile_id,
+    ExtensionEventRouterForwarder* event_router,
+    uint64 request_id,
+    net::HttpRequestHeaders* headers,
+    net::CompletionCallback* callback) {
+  // TODO(jochen): Figure out what to do with events from the system context.
+  if (profile_id == Profile::kInvalidProfileId)
+    return net::OK;
+
+  HttpRequestMap::iterator iter = http_requests_.find(request_id);
+  if (iter == http_requests_.end())
+    return net::OK;
+
+  net::URLRequest* request = iter->second;
+  http_requests_.erase(iter);
+
+  std::vector<const EventListener*> listeners =
+      GetMatchingListeners(profile_id, keys::kOnBeforeSendHeaders, request);
+  if (listeners.empty())
+    return net::OK;
+
+  ListValue args;
+  DictionaryValue* dict = new DictionaryValue();
+  dict->SetString(keys::kRequestIdKey,
+                  base::Uint64ToString(request->identifier()));
+  dict->SetString(keys::kUrlKey, request->url().spec());
+  dict->SetDouble(keys::kTimeStampKey, base::Time::Now().ToDoubleT() * 1000);
+  // TODO(mpcomplete): request headers.
+  args.Append(dict);
+
+  if (DispatchEvent(profile_id, event_router, request, callback, listeners,
+                    args))
+    return net::ERR_IO_PENDING;
+  return net::OK;
+}
+
+void ExtensionWebRequestEventRouter::OnURLRequestDestroyed(
+    ProfileId profile_id, net::URLRequest* request) {
+  http_requests_.erase(request->identifier());

[willchan no longer on Chromium, 2011/03/26 01:46:49]

Doesn't erase() cause crashes if the key doesn't exist? What if it completed
successfully and got removed in OnBeforeSendHeaders() first? Is there a test for
this somewhere?

[Matt Perry, 2011/03/28 22:51:01]

No, that's not an error.
That's actually the common case. Most of the time, OnBeforeSendHeaders is called
before the request is deleted. Sometimes (like with AppCache jobs, I believe),
this path is taken. I don't have any explicit tests for this.

[willchan no longer on Chromium, 2011/03/29 20:46:27]

Learn something new everyday.

+}
+
+bool ExtensionWebRequestEventRouter::DispatchEvent(
+    ProfileId profile_id,
+    ExtensionEventRouterForwarder* event_router,
+    net::URLRequest* request,
+    net::CompletionCallback* callback,
+    const std::vector<const EventListener*>& listeners,
+    const ListValue& args) {
   std::string json_args;
   base::JSONWriter::Write(&args, false, &json_args);
 
   // TODO(mpcomplete): Consider consolidating common (extension_id,json_args)
   // pairs into a single message sent to a list of sub_event_names.
   int num_handlers_blocking = 0;
-  for (std::vector<const EventListener*>::iterator it = listeners.begin();
+  for (std::vector<const EventListener*>::const_iterator it = listeners.begin();
        it != listeners.end(); ++it) {
     event_router->DispatchEventToExtension(
         (*it)->extension_id, (*it)->sub_event_name, json_args,
         profile_id, true, GURL());
-    if ((*it)->extra_info_spec & ExtraInfoSpec::BLOCKING) {
+    if (callback && (*it)->extra_info_spec & ExtraInfoSpec::BLOCKING) {
       (*it)->blocked_requests.insert(request->identifier());
       ++num_handlers_blocking;
     }
@@ -442,6 +506,20 @@ ExtensionWebRequestEventRouter::GetMatchingListeners(
   return matching_listeners;
 }
 
+std::vector<const ExtensionWebRequestEventRouter::EventListener*>
+ExtensionWebRequestEventRouter::GetMatchingListeners(
+    ProfileId profile_id,
+    const std::string& event_name,
+    net::URLRequest* request) {
+  int tab_id = -1;
+  int window_id = -1;
+  ResourceType::Type resource_type = ResourceType::LAST_TYPE;
+  ExtractRequestInfo(request, &tab_id, &window_id, &resource_type);
+
+  return GetMatchingListeners(
+      profile_id, event_name, request->url(), tab_id, window_id, resource_type);
+}
+
 void ExtensionWebRequestEventRouter::DecrementBlockCount(uint64 request_id,
                                                          bool cancel) {
   // It's possible that this request was already cancelled by a previous event