Tweaks to extension webRequest API.

- onBeforeSendHeaders respects extraInfoSpec.
- request headers are now an array of {name, value} objects instead of a flat
string.
- onBeforeRequest is sent after redirects now. This allows extensions to intercept
redirects and cancel/redirect them elsewhere.

BUG=60101
TEST=no

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

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

10 files changed, 239 insertions, 82 deletions

diff --git a/chrome/browser/extensions/extension_webrequest_api.cc b/chrome/browser/extensions/extension_webrequest_api.cc
index fc67acf..f6ebd81 100644
--- a/chrome/browser/extensions/extension_webrequest_api.cc
+++ b/chrome/browser/extensions/extension_webrequest_api.cc
@@ -163,9 +163,7 @@ struct ExtensionWebRequestEventRouter::ExtraInfoSpec {
     REQUEST_HEADERS = 1<<1,
     STATUS_LINE = 1<<2,
     RESPONSE_HEADERS = 1<<3,
-    REDIRECT_REQUEST_LINE = 1<<4,
-    REDIRECT_REQUEST_HEADERS = 1<<5,
-    BLOCKING = 1<<6,
+    BLOCKING = 1<<4,
   };
 
   static bool InitFromValue(const ListValue& value, int* extra_info_spec);
@@ -277,7 +275,6 @@ bool ExtensionWebRequestEventRouter::ExtraInfoSpec::InitFromValue(
     if (!value.GetString(i, &str))
       return false;
 
-    // TODO(mpcomplete): not all of these are valid for every event.
     if (str == "requestLine")
       *extra_info_spec |= REQUEST_LINE;
     else if (str == "requestHeaders")
@@ -286,10 +283,6 @@ bool ExtensionWebRequestEventRouter::ExtraInfoSpec::InitFromValue(
       *extra_info_spec |= STATUS_LINE;
     else if (str == "responseHeaders")
       *extra_info_spec |= RESPONSE_HEADERS;
-    else if (str == "redirectRequestLine")
-      *extra_info_spec |= REDIRECT_REQUEST_LINE;
-    else if (str == "redirectRequestHeaders")
-      *extra_info_spec |= REDIRECT_REQUEST_HEADERS;
     else if (str == "blocking")
       *extra_info_spec |= BLOCKING;
     else
@@ -334,9 +327,10 @@ int ExtensionWebRequestEventRouter::OnBeforeRequest(
   ResourceType::Type resource_type = ResourceType::LAST_TYPE;
   ExtractRequestInfo(request, &tab_id, &window_id, &resource_type);
 
+  int extra_info_spec = 0;
   std::vector<const EventListener*> listeners =
       GetMatchingListeners(profile_id, keys::kOnBeforeRequest, request->url(),
-                           tab_id, window_id, resource_type);
+                           tab_id, window_id, resource_type, &extra_info_spec);
   if (listeners.empty())
     return net::OK;
 
@@ -351,8 +345,7 @@ int ExtensionWebRequestEventRouter::OnBeforeRequest(
   dict->SetString(keys::kMethodKey, request->method());
   dict->SetInteger(keys::kTabIdKey, tab_id);
   dict->SetString(keys::kTypeKey, ResourceTypeToString(resource_type));
-  dict->SetDouble(keys::kTimeStampKey,
-                  request->request_time().ToDoubleT() * 1000);
+  dict->SetDouble(keys::kTimeStampKey, base::Time::Now().ToDoubleT() * 1000);
   args.Append(dict);
 
   if (DispatchEvent(profile_id, event_router, request, listeners, args)) {
@@ -383,8 +376,10 @@ int ExtensionWebRequestEventRouter::OnBeforeSendHeaders(
   if (GetAndSetSignaled(request->identifier(), kOnBeforeSendHeaders))
     return net::OK;
 
+  int extra_info_spec = 0;
   std::vector<const EventListener*> listeners =
-      GetMatchingListeners(profile_id, keys::kOnBeforeSendHeaders, request);
+      GetMatchingListeners(profile_id, keys::kOnBeforeSendHeaders, request,
+                           &extra_info_spec);
   if (listeners.empty())
     return net::OK;
 
@@ -394,8 +389,18 @@ int ExtensionWebRequestEventRouter::OnBeforeSendHeaders(
                   base::Uint64ToString(request->identifier()));
   dict->SetString(keys::kUrlKey, request->url().spec());
   dict->SetDouble(keys::kTimeStampKey, base::Time::Now().ToDoubleT() * 1000);
-  // TODO(mpcomplete): better format for headers?
-  dict->SetString(keys::kRequestHeadersKey, headers->ToString());
+
+  if (extra_info_spec & ExtraInfoSpec::REQUEST_HEADERS) {
+    ListValue* headers_value = new ListValue();
+    for (net::HttpRequestHeaders::Iterator it(*headers); it.GetNext(); ) {
+      DictionaryValue* header = new DictionaryValue();
+      header->SetString(keys::kHeaderNameKey, it.name());
+      header->SetString(keys::kHeaderValueKey, it.name());
+      headers_value->Append(header);
+    }
+    dict->Set(keys::kRequestHeadersKey, headers_value);
+  }
+
   args.Append(dict);
 
   if (DispatchEvent(profile_id, event_router, request, listeners, args)) {
@@ -427,8 +432,10 @@ void ExtensionWebRequestEventRouter::OnRequestSent(
 
   ClearSignaled(request->identifier(), kOnBeforeRedirect);
 
+  int extra_info_spec = 0;
   std::vector<const EventListener*> listeners =
-      GetMatchingListeners(profile_id, keys::kOnRequestSent, request);
+      GetMatchingListeners(profile_id, keys::kOnRequestSent, request,
+                           &extra_info_spec);
   if (listeners.empty())
     return;
 
@@ -457,11 +464,14 @@ void ExtensionWebRequestEventRouter::OnBeforeRedirect(
   if (GetAndSetSignaled(request->identifier(), kOnBeforeRedirect))
     return;
 
+  ClearSignaled(request->identifier(), kOnBeforeRequest);
   ClearSignaled(request->identifier(), kOnBeforeSendHeaders);
   ClearSignaled(request->identifier(), kOnRequestSent);
 
+  int extra_info_spec = 0;
   std::vector<const EventListener*> listeners =
-      GetMatchingListeners(profile_id, keys::kOnBeforeRedirect, request);
+      GetMatchingListeners(profile_id, keys::kOnBeforeRedirect, request,
+                           &extra_info_spec);
   if (listeners.empty())
     return;
 
@@ -475,8 +485,7 @@ void ExtensionWebRequestEventRouter::OnBeforeRedirect(
   dict->SetString(keys::kRedirectUrlKey, new_location.spec());
   dict->SetInteger(keys::kStatusCodeKey, http_status_code);
   dict->SetDouble(keys::kTimeStampKey, time.ToDoubleT() * 1000);
-  // TODO(battre): support "statusLine", "responseHeaders",
-  //     "redirectRequestLine" and "redirectRequestHeaders".
+  // TODO(battre): support "statusLine", "responseHeaders"
   args.Append(dict);
 
   DispatchEvent(profile_id, event_router, request, listeners, args);
@@ -495,8 +504,10 @@ void ExtensionWebRequestEventRouter::OnResponseStarted(
 
   base::Time time(base::Time::Now());
 
+  int extra_info_spec = 0;
   std::vector<const EventListener*> listeners =
-      GetMatchingListeners(profile_id, keys::kOnResponseStarted, request);
+      GetMatchingListeners(profile_id, keys::kOnResponseStarted, request,
+                           &extra_info_spec);
   if (listeners.empty())
     return;
 
@@ -531,8 +542,10 @@ void ExtensionWebRequestEventRouter::OnCompleted(
 
   base::Time time(base::Time::Now());
 
+  int extra_info_spec = 0;
   std::vector<const EventListener*> listeners =
-      GetMatchingListeners(profile_id, keys::kOnCompleted, request);
+      GetMatchingListeners(profile_id, keys::kOnCompleted, request,
+                           &extra_info_spec);
   if (listeners.empty())
     return;
 
@@ -567,8 +580,10 @@ void ExtensionWebRequestEventRouter::OnErrorOccurred(
 
   base::Time time(base::Time::Now());
 
+  int extra_info_spec = 0;
   std::vector<const EventListener*> listeners =
-      GetMatchingListeners(profile_id, keys::kOnErrorOccurred, request);
+      GetMatchingListeners(profile_id, keys::kOnErrorOccurred, request,
+                           &extra_info_spec);
   if (listeners.empty())
     return;
 
@@ -612,13 +627,20 @@ bool ExtensionWebRequestEventRouter::DispatchEvent(
     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*>::const_iterator it = listeners.begin();
        it != listeners.end(); ++it) {
+    // Filter out the optional keys that this listener didn't request.
+    scoped_ptr<ListValue> args_filtered(args.DeepCopy());
+    DictionaryValue* dict = NULL;
+    CHECK(args_filtered->GetDictionary(0, &dict) && dict);
+    if (!((*it)->extra_info_spec & ExtraInfoSpec::REQUEST_HEADERS))
+      dict->Remove(keys::kRequestHeadersKey, NULL);
+
+    base::JSONWriter::Write(args_filtered.get(), false, &json_args);
     event_router->DispatchEventToExtension(
         (*it)->extension_id, (*it)->sub_event_name, json_args,
         profile_id, true, GURL());
@@ -726,9 +748,12 @@ ExtensionWebRequestEventRouter::GetMatchingListeners(
     const GURL& url,
     int tab_id,
     int window_id,
-    ResourceType::Type resource_type) {
+    ResourceType::Type resource_type,
+    int* extra_info_spec) {
   // TODO(mpcomplete): handle profile_id == invalid (should collect all
   // listeners).
+  *extra_info_spec = 0;
+
   std::vector<const EventListener*> matching_listeners;
   std::set<EventListener>& listeners = listeners_[profile_id][event_name];
   for (std::set<EventListener>::iterator it = listeners.begin();
@@ -745,6 +770,7 @@ ExtensionWebRequestEventRouter::GetMatchingListeners(
       continue;
 
     matching_listeners.push_back(&(*it));
+    *extra_info_spec |= it->extra_info_spec;
   }
   return matching_listeners;
 }
@@ -753,14 +779,16 @@ std::vector<const ExtensionWebRequestEventRouter::EventListener*>
 ExtensionWebRequestEventRouter::GetMatchingListeners(
     ProfileId profile_id,
     const std::string& event_name,
-    net::URLRequest* request) {
+    net::URLRequest* request,
+    int* extra_info_spec) {
   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);
+      profile_id, event_name, request->url(), tab_id, window_id, resource_type,
+      extra_info_spec);
 }
 
 void ExtensionWebRequestEventRouter::DecrementBlockCount(
@@ -911,11 +939,23 @@ bool WebRequestEventHandled::RunImpl() {
     }
 
     if (value->HasKey("requestHeaders")) {
-      std::string request_headers_str;
+      ListValue* request_headers_value = NULL;
       request_headers.reset(new net::HttpRequestHeaders());
-      EXTENSION_FUNCTION_VALIDATE(value->GetString("requestHeaders",
-                                                   &request_headers_str));
-      request_headers->AddHeadersFromString(request_headers_str);
+      EXTENSION_FUNCTION_VALIDATE(value->GetList(keys::kRequestHeadersKey,
+                                                 &request_headers_value));
+      for (size_t i = 0; i < request_headers_value->GetSize(); ++i) {
+        DictionaryValue* header_value = NULL;
+        std::string name;
+        std::string value;
+        EXTENSION_FUNCTION_VALIDATE(
+            request_headers_value->GetDictionary(i, &header_value));
+        EXTENSION_FUNCTION_VALIDATE(
+            header_value->GetString(keys::kHeaderNameKey, &name));
+        EXTENSION_FUNCTION_VALIDATE(
+            header_value->GetString(keys::kHeaderValueKey, &value));
+
+        request_headers->SetHeader(name, value);
+      }
     }
   }
 
diff --git a/chrome/browser/extensions/extension_webrequest_api.h b/chrome/browser/extensions/extension_webrequest_api.h
index a23f037..a894a9e 100644
--- a/chrome/browser/extensions/extension_webrequest_api.h
+++ b/chrome/browser/extensions/extension_webrequest_api.h
@@ -153,20 +153,23 @@ class ExtensionWebRequestEventRouter {
       const ListValue& args);
 
   // Returns a list of event listeners that care about the given event, based
-  // on their filter parameters.
+  // on their filter parameters. |extra_info_spec| will contain the combined
+  // set of extra_info_spec flags that every matching listener asked for.
   std::vector<const EventListener*> GetMatchingListeners(
       ProfileId profile_id,
       const std::string& event_name,
       const GURL& url,
       int tab_id,
       int window_id,
-      ResourceType::Type resource_type);
+      ResourceType::Type resource_type,
+      int* extra_info_spec);
 
   // Same as above, but retrieves the filter parameters from the request.
   std::vector<const EventListener*> GetMatchingListeners(
       ProfileId profile_id,
       const std::string& event_name,
-      net::URLRequest* request);
+      net::URLRequest* request,
+      int* extra_info_spec);
 
   // Decrements the count of event handlers blocking the given request. When the
   // count reaches 0 (or immediately if the request is being cancelled or
diff --git a/chrome/browser/extensions/extension_webrequest_api_constants.cc b/chrome/browser/extensions/extension_webrequest_api_constants.cc
index 5c2dbce..64b633d 100644
--- a/chrome/browser/extensions/extension_webrequest_api_constants.cc
+++ b/chrome/browser/extensions/extension_webrequest_api_constants.cc
@@ -17,6 +17,8 @@ const char kTimeStampKey[] = "timeStamp";
 const char kTypeKey[] = "type";
 const char kUrlKey[] = "url";
 const char kRequestHeadersKey[] = "requestHeaders";
+const char kHeaderNameKey[] = "name";
+const char kHeaderValueKey[] = "value";
 
 const char kOnBeforeRedirect[] = "experimental.webRequest.onBeforeRedirect";
 const char kOnBeforeRequest[] = "experimental.webRequest.onBeforeRequest";
diff --git a/chrome/browser/extensions/extension_webrequest_api_constants.h b/chrome/browser/extensions/extension_webrequest_api_constants.h
index 42111c0..73b8664 100644
--- a/chrome/browser/extensions/extension_webrequest_api_constants.h
+++ b/chrome/browser/extensions/extension_webrequest_api_constants.h
@@ -22,6 +22,9 @@ extern const char kTimeStampKey[];
 extern const char kTypeKey[];
 extern const char kUrlKey[];
 extern const char kRequestHeadersKey[];
+extern const char kHeadersKey[];
+extern const char kHeaderNameKey[];
+extern const char kHeaderValueKey[];
 
 // Events.
 extern const char kOnBeforeRedirect[];
diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json
index 8993dcc..8de6a8d 100644
--- a/chrome/common/extensions/api/extension_api.json
+++ b/chrome/common/extensions/api/extension_api.json
@@ -3541,6 +3541,18 @@
         }
       },
       {
+        "id": "HttpHeaders",
+        "type": "array",
+        "description": "An array of HTTP headers, in the form of name/value pairs.",
+        "items": {
+          "type": "object",
+          "properties": {
+            "name": {"type": "string"},
+            "value": {"type": "string"}
+          }
+        }
+      },
+      {
         "id": "BlockingResponse",
         "type": "object",
         "description": "Return value for event handlers that have the 'blocking' extraInfoSpec applied. Allows the event handler to modify network requests.",
@@ -3556,7 +3568,7 @@
             "description": "Only used as a response to the onBeforeRequest event. If set, the original request is prevented from being sent and is instead redirected to the given URL."
           },
           "requestHeaders": {
-            "type": "string",
+            "$ref": "HttpHeaders",
             "optional": true,
             "description": "Only used as a response to the onBeforeSendHeaders event. If set, the request is made with these request headers instead. The headers should be delimited by literal \"\\r\\n\" substrings."
           }
@@ -3584,7 +3596,7 @@
             "description": "Array of extra information that should be passed to the listener function.",
             "items": {
               "type": "string",
-              "enum": ["requestLine", "requestHeaders", "statusLine", "responseHeaders", "redirectRequestLine", "redirectRequestHeaders", "blocking"]
+              "enum": ["requestLine", "requestHeaders", "statusLine", "responseHeaders", "blocking"]
             }
           },
           {"type": "string", "name": "eventName"},
@@ -3663,7 +3675,7 @@
               "requestId": {"type": "string", "description": "The ID of the request. Request IDs are unique within a browser session. As a result, they could be used to relate different events of the same request."},
               "url": {"type": "string"},
               "timeStamp": {"type": "number", "description": "The time when the browser was about to send headers, in milliseconds since the epoch."},
-              "requestHeaders": {"type": "string", "description": "The HTTP request headers that are going to be sent out with this request."}
+              "requestHeaders": {"$ref": "HttpHeaders", "optional": true, "description": "The HTTP request headers that are going to be sent out with this request."}
             }
           }
         ],
@@ -3792,7 +3804,7 @@
             "description": "Array of extra information that should be passed to the listener function.",
             "items": {
               "type": "string",
-              "enum": ["statusLine", "responseHeaders", "redirectRequestLine", "redirectRequestHeaders"]
+              "enum": ["statusLine", "responseHeaders"]
             }
           }
         ]
diff --git a/chrome/common/extensions/docs/experimental.webRequest.html b/chrome/common/extensions/docs/experimental.webRequest.html
index 892e149..5737193 100644
--- a/chrome/common/extensions/docs/experimental.webRequest.html
+++ b/chrome/common/extensions/docs/experimental.webRequest.html
@@ -312,6 +312,8 @@
                       <li>
                         <a href="#type-RequestFilter">RequestFilter</a>
                       </li><li>
+                        <a href="#type-HttpHeaders">HttpHeaders</a>
+                      </li><li>
                         <a href="#type-BlockingResponse">BlockingResponse</a>
                       </li>
                     </ol>
@@ -2122,18 +2124,18 @@ unexpected results.
                 <!-- TYPE -->
                 <div style="display:inline">
                   (
-                    <span class="optional" style="display: none; ">optional</span>
+                    <span class="optional">optional</span>
                     <span class="enum" style="display: none; ">enumerated</span>
                     <span id="typeTemplate">
-                      <span style="display: none; ">
-                        <a> Type</a>
-                      </span>
                       <span>
-                        <span style="display: none; ">
+                        <a href="experimental.webRequest.html#type-HttpHeaders">HttpHeaders</a>
+                      </span>
+                      <span style="display: none; ">
+                        <span>
                           array of <span><span></span></span>
                         </span>
-                        <span>string</span>
-                        <span style="display: none; "></span>
+                        <span>paramType</span>
+                        <span></span>
                       </span>
                     </span>
                   )
@@ -4838,6 +4840,89 @@ unexpected results.
         </div>
 
             </div><div class="apiItem">
+              <a name="type-HttpHeaders"></a>
+              <h4>HttpHeaders</h4>
+
+              <div>
+          <dt>
+            <var style="display: none; ">paramName</var>
+              <em>
+
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span class="optional" style="display: none; ">optional</span>
+                    <span class="enum" style="display: none; ">enumerated</span>
+                    <span id="typeTemplate">
+                      <span style="display: none; ">
+                        <a> Type</a>
+                      </span>
+                      <span>
+                        <span>
+                          array of <span><span>
+                      <span style="display: none; ">
+                        <a> Type</a>
+                      </span>
+                      <span>
+                        <span style="display: none; ">
+                          array of <span><span></span></span>
+                        </span>
+                        <span>object</span>
+                        <span style="display: none; "></span>
+                      </span>
+                    </span></span>
+                        </span>
+                        <span style="display: none; ">paramType</span>
+                        <span style="display: none; "></span>
+                      </span>
+                    </span>
+                  )
+                </div>
+
+              </em>
+          </dt>
+          <dd class="todo" style="display: none; ">
+            Undocumented.
+          </dd>
+          <dd>An array of HTTP headers, in the form of name/value pairs.</dd>
+          <dd style="display: none; ">
+            This parameter was added in version
+            <b><span></span></b>.
+            You must omit this parameter in earlier versions,
+            and you may omit it in any version.  If you require this
+            parameter, the manifest key
+            <a href="manifest.html#minimum_chrome_version">minimum_chrome_version</a>
+            can ensure that your extension won't be run in an earlier browser version.
+          </dd>
+
+          <!-- OBJECT PROPERTIES -->
+          <dd style="display: none; ">
+            <dl>
+              <div>
+                <div>
+                </div>
+              </div>
+            </dl>
+          </dd>
+
+          <!-- OBJECT METHODS -->
+          <dd style="display: none; ">
+            <div></div>
+          </dd>
+
+          <!-- OBJECT EVENT FIELDS -->
+          <dd style="display: none; ">
+            <div></div>
+          </dd>
+
+          <!-- FUNCTION PARAMETERS -->
+          <dd style="display: none; ">
+            <div></div>
+          </dd>
+
+        </div>
+
+            </div><div class="apiItem">
               <a name="type-BlockingResponse"></a>
               <h4>BlockingResponse</h4>
 
@@ -5033,15 +5118,15 @@ unexpected results.
                     <span class="optional">optional</span>
                     <span class="enum" style="display: none; ">enumerated</span>
                     <span id="typeTemplate">
-                      <span style="display: none; ">
-                        <a> Type</a>
-                      </span>
                       <span>
-                        <span style="display: none; ">
+                        <a href="experimental.webRequest.html#type-HttpHeaders">HttpHeaders</a>
+                      </span>
+                      <span style="display: none; ">
+                        <span>
                           array of <span><span></span></span>
                         </span>
-                        <span>string</span>
-                        <span style="display: none; "></span>
+                        <span>paramType</span>
+                        <span></span>
                       </span>
                     </span>
                   )
diff --git a/chrome/test/data/extensions/api_test/webrequest/events/test.html b/chrome/test/data/extensions/api_test/webrequest/events/test.html
index b5b8045..783f0df 100644
--- a/chrome/test/data/extensions/api_test/webrequest/events/test.html
+++ b/chrome/test/data/extensions/api_test/webrequest/events/test.html
@@ -56,14 +56,14 @@ function navigateAndWait(url, callback) {
 //     relative order of "a" and "d" does not matter.
 // filter: filter dictionary passed on to the event subscription of the
 //     webRequest API.
-// blocking: whether request shall be blocking
-function expect(data, order, filter, blocking) {
+// extraInfoSpec: the union of all desired extraInfoSpecs for the events.
+function expect(data, order, filter, extraInfoSpec) {
   expectedEventData = data;
   capturedEventData = [];
   expectedEventOrder = order;
   eventsCaptured = chrome.test.callbackAdded();
   removeListeners();
-  initListeners(filter, blocking);
+  initListeners(filter || {}, extraInfoSpec || []);
 }
 
 function checkExpectations() {
@@ -151,15 +151,21 @@ function captureEvent(name, details) {
   return retval;
 }
 
-function initListeners(filter, blocking) {
+// Simple array intersection. We use this to filter extraInfoSpec so
+// that only the allowed specs are sent to each listener.
+function intersect(array1, array2) {
+  return array1.filter(function(x) { return array2.indexOf(x) != -1; });
+}
+
+function initListeners(filter, extraInfoSpec) {
   chrome.experimental.webRequest.onBeforeRequest.addListener(
       function(details) {
     return captureEvent("onBeforeRequest", details);
-  }, filter, blocking ? ["blocking"] : []);
+  }, filter, intersect(extraInfoSpec, ["blocking"]));
   chrome.experimental.webRequest.onBeforeSendHeaders.addListener(
       function(details) {
     return captureEvent("onBeforeSendHeaders", details);
-  }, filter, blocking ? ["blocking"] : []);
+  }, filter, intersect(extraInfoSpec, ["blocking", "requestHeaders"]));
   chrome.experimental.webRequest.onRequestSent.addListener(
       function(details) {
     return captureEvent("onRequestSent", details);
@@ -240,7 +246,7 @@ runTests([
   function simpleLoadHttp() {
     expect(
       [  // events
-        { label: "onBeforeRequest",
+        { label: "onBeforeRequest-1",
           event: "onBeforeRequest",
           details: {
             method: "GET",
@@ -271,6 +277,15 @@ runTests([
             statusCode: 301
           }
         },
+        { label: "onBeforeRequest-2",
+          event: "onBeforeRequest",
+          details: {
+            method: "GET",
+            tabId: tabId,
+            type: "main_frame",
+            url: URL_HTTP_SIMPLE_LOAD
+          }
+        },
         { label: "onBeforeSendHeaders-2",
           event: "onBeforeSendHeaders",
           details: {
@@ -301,9 +316,12 @@ runTests([
         }
       ],
       [  // event order
-        ["onBeforeRequest", "onBeforeSendHeaders-1", "onRequestSent-1",
-         "onBeforeRedirect", "onBeforeSendHeaders-2", "onRequestSent-2",
-         "onResponseStarted", "onCompleted"] ]);
+        ["onBeforeRequest-1", "onBeforeSendHeaders-1", "onRequestSent-1",
+         "onBeforeRedirect",
+         "onBeforeRequest-2", "onBeforeSendHeaders-2", "onRequestSent-2",
+         "onResponseStarted", "onCompleted"] ],
+      {},
+      ["requestHeaders"]);
     navigateAndWait(URL_HTTP_SIMPLE_LOAD_REDIRECT);
   },
 
@@ -423,13 +441,15 @@ runTests([
         ["onBeforeRequest"]
       ],
       {},  // filter
-      true  // blocking
-      );
+      ["blocking"]);
     navigateAndWait(getURL("complexLoad/a.html"));
   },
 
   // Navigates to a page with a blocking handler that redirects to a different
   // page.
+  // TODO(mpcomplete): We should see an onBeforeRedirect as well, but our
+  // process switching logic cancels the original redirect request and
+  // starts a new one instead. See http://crbug.com/79520.
   function complexLoadRedirected() {
     expect(
       [  // events
@@ -443,10 +463,6 @@ runTests([
           },
           retval: {redirectUrl: getURL("simpleLoad/a.html")}
         },
-        // TODO(mpcomplete): This second event should not fire in general. Our
-        // process switching logic cancels the original redirect request and
-        // starts this new one. This should be an onBeforeRedirect instead. See
-        // http://crbug.com/79520.
         { label: "onBeforeRequest-2",
           event: "onBeforeRequest",
           details: {
@@ -476,8 +492,7 @@ runTests([
          "onCompleted"]
       ],
       {}, // filter
-      true  // blocking
-      );
+      ["blocking"]);
     navigateAndWait(getURL("complexLoad/a.html"));
   },
 
@@ -542,9 +557,7 @@ runTests([
         urls: [getURL("complexLoad/*")],
         types: ["main_frame", "image"],
         tabId: tabId
-      },
-      false  // blocking
-      );
+      });
     chrome.tabs.create({ url: getURL("simpleLoad/a.html") },
         function(newTab) {
       chrome.tabs.remove(newTab.id);
@@ -597,9 +610,9 @@ runTests([
           event: "onBeforeSendHeaders",
           details: {
             url: URL_ECHO_USER_AGENT,
-            requestHeadersExist: true,
+            // Note: no requestHeaders because we don't ask for them.
           },
-          retval: {requestHeaders: "User-Agent: FoobarUA"}
+          retval: {requestHeaders: [{name: "User-Agent", value: "FoobarUA"}]}
         },
         { label: "onRequestSent",
           event: "onRequestSent",
@@ -627,7 +640,7 @@ runTests([
         ["onBeforeRequest", "onBeforeSendHeaders", "onRequestSent",
          "onResponseStarted", "onCompleted"]
       ],
-      {}, true);
+      {}, ["blocking"]);
     // Check the page content for our modified User-Agent string.
     navigateAndWait(URL_ECHO_USER_AGENT, function() {
       chrome.test.listenOnce(chrome.extension.onRequest, function(request) {
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 0f19d69..d1d46bfb 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -371,7 +371,7 @@ void URLRequest::Start() {
     }
   }
 
-  StartInternal();
+  StartJob(URLRequestJobManager::GetInstance()->CreateJob(this));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -388,14 +388,10 @@ void URLRequest::BeforeRequestComplete(int error) {
     new_url.Swap(&delegate_redirect_url_);
     StartJob(new URLRequestRedirectJob(this, new_url));
   } else {
-    StartInternal();
+    StartJob(URLRequestJobManager::GetInstance()->CreateJob(this));
   }
 }
 
-void URLRequest::StartInternal() {
-  StartJob(URLRequestJobManager::GetInstance()->CreateJob(this));
-}
-
 void URLRequest::StartJob(URLRequestJob* job) {
   DCHECK(!is_pending_);
   DCHECK(!job_);
@@ -638,7 +634,7 @@ int URLRequest::Redirect(const GURL& location, int http_status_code) {
     final_upload_progress_ = job_->GetUploadProgress();
 
   PrepareToRestart();
-  StartInternal();
+  Start();
   return OK;
 }
 
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index b6d58ce..2ad1162 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -587,8 +587,6 @@ class URLRequest : public base::NonThreadSafe {
   friend class URLRequestJob;
   typedef std::map<const void*, linked_ptr<UserData> > UserDataMap;
 
-  void StartInternal();
-
   // Resumes or blocks a request paused by the NetworkDelegate::OnBeforeRequest
   // handler. If |blocked| is true, the request is blocked and an error page is
   // returned indicating so. This should only be called after Start is called
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index f32af25..1c0987f 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -137,6 +137,11 @@ class BlockingNetworkDelegate : public TestNetworkDelegate {
   virtual int OnBeforeURLRequest(net::URLRequest* request,
                                  net::CompletionCallback* callback,
                                  GURL* new_url) {
+    if (redirect_url_ == request->url()) {
+      // We've already seen this request and redirected elsewhere.
+      return net::OK;
+    }
+
     TestNetworkDelegate::OnBeforeURLRequest(request, callback, new_url);
 
     if (!redirect_url_.is_empty())