Consider redirects to data:// urls as cancelling a request and don't report conflicts

If an extension A tries to redirect a request to http://foo and an extension B tries to redirect a request to data://..., we consider the redirect of extension B as an attempt to cancel the request. As cancelling a request is considered to take precedence over redirects, this does not generate a conflict message any more.


BUG=106431
TEST=no


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

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

chrome/browser/extensions/extension_webrequest_api_helpers.cc

@@ -7,6 +7,7 @@
 #include "base/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_webrequest_api.h"
+#include "chrome/common/url_constants.h"
 #include "net/http/http_util.h"
 
 namespace extension_webrequest_api_helpers {
@@ -247,34 +248,67 @@ void MergeCancelOfResponses(
   }
 }
 
-void MergeOnBeforeRequestResponses(
+// Helper function for MergeOnBeforeRequestResponses() that allows considering
+// only data:// urls. These are considered a special case of cancelling a
+// request. This helper function allows us to ignore all other redirects
+// in case any extension wants to cancel the request by redirecting to a
+// data:// url.
+// Returns whether a redirect occurred.
+static bool MergeOnBeforeRequestResponsesHelper(
     const EventResponseDeltas& deltas,
     GURL* new_url,
     std::set<std::string>* conflicting_extensions,
-    EventLogEntries* event_log_entries) {
-  EventResponseDeltas::const_iterator delta;
-
+    EventLogEntries* event_log_entries,
+    bool consider_only_data_scheme_urls) {
   bool redirected = false;
+
+  EventResponseDeltas::const_iterator delta;
   for (delta = deltas.begin(); delta != deltas.end(); ++delta) {
-    if (!(*delta)->new_url.is_empty()) {
-      if (!redirected || *new_url == (*delta)->new_url) {
-        *new_url = (*delta)->new_url;
-        redirected = true;
-        EventLogEntry log_entry(
-            net::NetLog::TYPE_CHROME_EXTENSION_REDIRECTED_REQUEST,
-            make_scoped_refptr(
-                new NetLogExtensionIdParameter((*delta)->extension_id)));
-        event_log_entries->push_back(log_entry);
-      } else {
-        conflicting_extensions->insert((*delta)->extension_id);
-        EventLogEntry log_entry(
-            net::NetLog::TYPE_CHROME_EXTENSION_REDIRECTED_REQUEST,
-            make_scoped_refptr(
-                new NetLogExtensionIdParameter((*delta)->extension_id)));
-        event_log_entries->push_back(log_entry);
-      }
+    if ((*delta)->new_url.is_empty())
+      continue;
+    if (consider_only_data_scheme_urls &&
+        !(*delta)->new_url.SchemeIs(chrome::kDataScheme)) {
+      continue;
+    }
+
+    if (!redirected || *new_url == (*delta)->new_url) {
+      *new_url = (*delta)->new_url;
+      redirected = true;
+      EventLogEntry log_entry(
+          net::NetLog::TYPE_CHROME_EXTENSION_REDIRECTED_REQUEST,
+          make_scoped_refptr(
+              new NetLogExtensionIdParameter((*delta)->extension_id)));
+      event_log_entries->push_back(log_entry);
+    } else {
+      conflicting_extensions->insert((*delta)->extension_id);
+      EventLogEntry log_entry(
+          net::NetLog::TYPE_CHROME_EXTENSION_IGNORED_DUE_TO_CONFLICT,
+          make_scoped_refptr(
+              new NetLogExtensionIdParameter((*delta)->extension_id)));
+      event_log_entries->push_back(log_entry);
     }
   }
+  return redirected;
+}
+
+void MergeOnBeforeRequestResponses(
+    const EventResponseDeltas& deltas,
+    GURL* new_url,
+    std::set<std::string>* conflicting_extensions,
+    EventLogEntries* event_log_entries) {
+
+  // First handle only redirects to data:// URLs. These are a special case as
+  // they represent a way of cancelling a request.
+  if (MergeOnBeforeRequestResponsesHelper(
+          deltas, new_url, conflicting_extensions, event_log_entries, true)) {
+    // If any extension cancelled a request by redirecting to a data:// URL,
+    // we don't consider the other redirects.
+    return;
+  }
+
+  // Handle all other redirects.
+  MergeOnBeforeRequestResponsesHelper(
+            deltas, new_url, conflicting_extensions, event_log_entries, false);
 }
 
 void MergeOnBeforeSendHeadersResponses(

chrome/browser/extensions/extension_webrequest_api_unittest.cc

@@ -988,6 +988,72 @@ TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeRequestResponses) {
   EXPECT_EQ(4u, event_log.size());
 }
 
+// This tests that we can redirect to data:// urls, which is considered
+// a kind of cancelling requests.
+TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeRequestResponses2) {
+  using namespace extension_webrequest_api_helpers;
+  EventResponseDeltas deltas;
+  EventLogEntries event_log;
+  std::set<std::string> conflicting_extensions;
+  GURL effective_new_url;
+
+  // Single redirect.
+  GURL new_url_0("http://foo.com");
+  linked_ptr<EventResponseDelta> d0(
+      new EventResponseDelta("extid0", base::Time::FromInternalValue(2000)));
+  d0->new_url = GURL(new_url_0);
+  deltas.push_back(d0);
+  MergeOnBeforeRequestResponses(
+      deltas, &effective_new_url, &conflicting_extensions, &event_log);
+  EXPECT_EQ(new_url_0, effective_new_url);
+
+  // Cancel request by redirecting to a data:// URL. This shall override
+  // the other redirect but not cause any conflict warnings.
+  GURL new_url_1("data://foo");
+  linked_ptr<EventResponseDelta> d1(
+      new EventResponseDelta("extid1", base::Time::FromInternalValue(1500)));
+  d1->new_url = GURL(new_url_1);
+  deltas.push_back(d1);
+  deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
+  event_log.clear();
+  MergeOnBeforeRequestResponses(
+      deltas, &effective_new_url, &conflicting_extensions, &event_log);
+  EXPECT_EQ(new_url_1, effective_new_url);
+  EXPECT_TRUE(conflicting_extensions.empty());
+  EXPECT_EQ(1u, event_log.size());
+
+  // Cancel request by redirecting to the same data:// URL. This shall
+  // not create any conflicts as it is in line with d1.
+  GURL new_url_2("data://foo");
+  linked_ptr<EventResponseDelta> d2(
+      new EventResponseDelta("extid2", base::Time::FromInternalValue(1000)));
+  d2->new_url = GURL(new_url_2);
+  deltas.push_back(d2);
+  deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
+  event_log.clear();
+  MergeOnBeforeRequestResponses(
+      deltas, &effective_new_url, &conflicting_extensions, &event_log);
+  EXPECT_EQ(new_url_1, effective_new_url);
+  EXPECT_TRUE(conflicting_extensions.empty());
+  EXPECT_EQ(2u, event_log.size());
+
+  // Cancel redirect by redirecting to a different data:// URL. This needs
+  // to create a conflict.
+  GURL new_url_3("data://something_totally_different");
+  linked_ptr<EventResponseDelta> d3(
+      new EventResponseDelta("extid3", base::Time::FromInternalValue(500)));
+  d3->new_url = GURL(new_url_3);
+  deltas.push_back(d3);
+  deltas.sort(&InDecreasingExtensionInstallationTimeOrder);
+  event_log.clear();
+  MergeOnBeforeRequestResponses(
+      deltas, &effective_new_url, &conflicting_extensions, &event_log);
+  EXPECT_EQ(new_url_1, effective_new_url);
+  EXPECT_EQ(1u, conflicting_extensions.size());
+  EXPECT_TRUE(ContainsKey(conflicting_extensions, "extid3"));
+  EXPECT_EQ(3u, event_log.size());
+}
+
 TEST(ExtensionWebRequestHelpersTest, TestMergeOnBeforeSendHeadersResponses) {
   using namespace extension_webrequest_api_helpers;
   net::HttpRequestHeaders base_headers;