Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1154)

Unified Diff: chrome/browser/extensions/api/web_request/web_request_api_helpers.cc

Issue 10407105: Improve error messaging of webRequest API in case of conflicts (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Addressed Evan's comments Created 8 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/extensions/api/web_request/web_request_api_helpers.cc
diff --git a/chrome/browser/extensions/api/web_request/web_request_api_helpers.cc b/chrome/browser/extensions/api/web_request/web_request_api_helpers.cc
index 986ca116cb3ae99df4e96cbe5a327b5a89368b19..941ba4d83e9764ec85e8ec00af98b69837550b8d 100644
--- a/chrome/browser/extensions/api/web_request/web_request_api_helpers.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_api_helpers.cc
@@ -13,6 +13,7 @@
#include "base/time.h"
#include "base/values.h"
#include "chrome/browser/extensions/api/web_request/web_request_api.h"
+#include "chrome/browser/extensions/extension_warning_set.h"
#include "chrome/browser/renderer_host/web_cache_manager.h"
#include "chrome/common/url_constants.h"
#include "content/public/browser/browser_thread.h"
@@ -22,7 +23,11 @@
#include "net/http/http_util.h"
#include "net/url_request/url_request.h"
+// TODO(battre): move all static functions into an anonymous namespace at the
+// top of this file.
+
using base::Time;
+using extensions::ExtensionWarning;
namespace extension_web_request_api_helpers {
@@ -329,11 +334,13 @@ void MergeCancelOfResponses(
static bool MergeOnBeforeRequestResponsesHelper(
const EventResponseDeltas& deltas,
GURL* new_url,
- std::set<std::string>* conflicting_extensions,
+ extensions::ExtensionWarningSet* conflicting_extensions,
const net::BoundNetLog* net_log,
bool consider_only_cancel_scheme_urls) {
bool redirected = false;
+ // Extension that determines the |new_url|.
+ std::string winning_extension_id;
EventResponseDeltas::const_iterator delta;
for (delta = deltas.begin(); delta != deltas.end(); ++delta) {
if ((*delta)->new_url.is_empty())
@@ -346,12 +353,18 @@ static bool MergeOnBeforeRequestResponsesHelper(
if (!redirected || *new_url == (*delta)->new_url) {
*new_url = (*delta)->new_url;
+ winning_extension_id = (*delta)->extension_id;
redirected = true;
net_log->AddEvent(
net::NetLog::TYPE_CHROME_EXTENSION_REDIRECTED_REQUEST,
CreateNetLogExtensionIdCallback(delta->get()));
} else {
- conflicting_extensions->insert((*delta)->extension_id);
+ conflicting_extensions->insert(
+ ExtensionWarning::CreateRedirectConflictWarning(
+ (*delta)->extension_id,
+ winning_extension_id,
+ (*delta)->new_url,
+ *new_url));
net_log->AddEvent(
net::NetLog::TYPE_CHROME_EXTENSION_IGNORED_DUE_TO_CONFLICT,
CreateNetLogExtensionIdCallback(delta->get()));
@@ -363,7 +376,7 @@ static bool MergeOnBeforeRequestResponsesHelper(
void MergeOnBeforeRequestResponses(
const EventResponseDeltas& deltas,
GURL* new_url,
- std::set<std::string>* conflicting_extensions,
+ extensions::ExtensionWarningSet* conflicting_extensions,
const net::BoundNetLog* net_log) {
// First handle only redirects to data:// URLs and about:blank. These are a
@@ -561,7 +574,7 @@ static bool MergeRemoveRequestCookieModifications(
void MergeCookiesInOnBeforeSendHeadersResponses(
const EventResponseDeltas& deltas,
net::HttpRequestHeaders* request_headers,
- std::set<std::string>* conflicting_extensions,
+ extensions::ExtensionWarningSet* conflicting_extensions,
const net::BoundNetLog* net_log) {
// Skip all work if there are no registered cookie modifications.
bool cookie_modifications_exist = false;
@@ -593,10 +606,46 @@ void MergeCookiesInOnBeforeSendHeadersResponses(
}
}
+// Returns the extension ID of the first extension in |deltas| that sets the
+// request header identified by |key| to |value|.
+static std::string FindSetRequestHeader(
+ const EventResponseDeltas& deltas,
+ const std::string& key,
+ const std::string& value) {
+ EventResponseDeltas::const_iterator delta;
+ for (delta = deltas.begin(); delta != deltas.end(); ++delta) {
+ net::HttpRequestHeaders::Iterator modification(
+ (*delta)->modified_request_headers);
+ while (modification.GetNext()) {
+ if (key == modification.name() && value == modification.value())
+ return (*delta)->extension_id;
+ }
+ }
+ return "";
+}
+
+// Returns the extension ID of the first extension in |deltas| that removes the
+// request header identified by |key|.
+static std::string FindRemoveRequestHeader(
+ const EventResponseDeltas& deltas,
+ const std::string& key) {
+ EventResponseDeltas::const_iterator delta;
+ for (delta = deltas.begin(); delta != deltas.end(); ++delta) {
+ std::vector<std::string>::iterator i;
+ for (i = (*delta)->deleted_request_headers.begin();
+ i != (*delta)->deleted_request_headers.end();
+ ++i) {
+ if (*i == key)
+ return (*delta)->extension_id;
+ }
+ }
+ return "";
+}
+
void MergeOnBeforeSendHeadersResponses(
const EventResponseDeltas& deltas,
net::HttpRequestHeaders* request_headers,
- std::set<std::string>* conflicting_extensions,
+ extensions::ExtensionWarningSet* conflicting_extensions,
const net::BoundNetLog* net_log) {
EventResponseDeltas::const_iterator delta;
@@ -617,6 +666,8 @@ void MergeOnBeforeSendHeadersResponses(
// has been modified differently before. As deltas is sorted by decreasing
// extension installation order, this takes care of precedence.
bool extension_conflicts = false;
+ std::string winning_extension_id;
+ std::string conflicting_header;
{
net::HttpRequestHeaders::Iterator modification(
(*delta)->modified_request_headers);
@@ -626,15 +677,23 @@ void MergeOnBeforeSendHeadersResponses(
const std::string& value = modification.value();
// We must not delete anything that has been modified before.
- if (removed_headers.find(key) != removed_headers.end())
+ if (removed_headers.find(key) != removed_headers.end() &&
+ !extension_conflicts) {
+ winning_extension_id = FindRemoveRequestHeader(deltas, key);
+ conflicting_header = key;
extension_conflicts = true;
+ }
// We must not modify anything that has been set to a *different*
// value before.
- if (set_headers.find(key) != set_headers.end()) {
+ if (set_headers.find(key) != set_headers.end() &&
+ !extension_conflicts) {
std::string current_value;
if (!request_headers->GetHeader(key, &current_value) ||
current_value != value) {
+ winning_extension_id =
+ FindSetRequestHeader(deltas, key, current_value);
+ conflicting_header = key;
extension_conflicts = true;
}
}
@@ -649,8 +708,14 @@ void MergeOnBeforeSendHeadersResponses(
key != (*delta)->deleted_request_headers.end() &&
!extension_conflicts;
++key) {
- if (set_headers.find(*key) != set_headers.end())
+ if (set_headers.find(*key) != set_headers.end()) {
+ std::string current_value;
+ request_headers->GetHeader(*key, &current_value);
+ winning_extension_id =
+ FindSetRequestHeader(deltas, *key, current_value);
+ conflicting_header = *key;
extension_conflicts = true;
+ }
}
}
@@ -680,7 +745,10 @@ void MergeOnBeforeSendHeadersResponses(
net::NetLog::TYPE_CHROME_EXTENSION_MODIFIED_HEADERS,
base::Bind(&NetLogModificationCallback, delta->get()));
} else {
- conflicting_extensions->insert((*delta)->extension_id);
+ conflicting_extensions->insert(
+ ExtensionWarning::CreateRequestHeaderConflictWarning(
+ (*delta)->extension_id, winning_extension_id,
+ conflicting_header));
net_log->AddEvent(
net::NetLog::TYPE_CHROME_EXTENSION_IGNORED_DUE_TO_CONFLICT,
CreateNetLogExtensionIdCallback(delta->get()));
@@ -887,7 +955,7 @@ void MergeCookiesInOnHeadersReceivedResponses(
const EventResponseDeltas& deltas,
const net::HttpResponseHeaders* original_response_headers,
scoped_refptr<net::HttpResponseHeaders>* override_response_headers,
- std::set<std::string>* conflicting_extensions,
+ extensions::ExtensionWarningSet* conflicting_extensions,
const net::BoundNetLog* net_log) {
// Skip all work if there are no registered cookie modifications.
bool cookie_modifications_exist = false;
@@ -925,11 +993,29 @@ static ResponseHeader ToLowerCase(const ResponseHeader& header) {
return ResponseHeader(lower_key, header.second);
}
+// Returns the extension ID of the first extension in |deltas| that removes the
+// request header identified by |key|.
+static std::string FindRemoveResponseHeader(
+ const EventResponseDeltas& deltas,
+ const std::string& key) {
+ std::string lower_key = StringToLowerASCII(key);
+ EventResponseDeltas::const_iterator delta;
+ for (delta = deltas.begin(); delta != deltas.end(); ++delta) {
+ ResponseHeaders::const_iterator i;
+ for (i = (*delta)->deleted_response_headers.begin();
+ i != (*delta)->deleted_response_headers.end(); ++i) {
+ if (StringToLowerASCII(i->first) == lower_key)
+ return (*delta)->extension_id;
+ }
+ }
+ return "";
+}
+
void MergeOnHeadersReceivedResponses(
const EventResponseDeltas& deltas,
const net::HttpResponseHeaders* original_response_headers,
scoped_refptr<net::HttpResponseHeaders>* override_response_headers,
- std::set<std::string>* conflicting_extensions,
+ extensions::ExtensionWarningSet* conflicting_extensions,
const net::BoundNetLog* net_log) {
EventResponseDeltas::const_iterator delta;
@@ -959,10 +1045,14 @@ void MergeOnHeadersReceivedResponses(
// conflict. As deltas is sorted by decreasing extension installation order,
// this takes care of precedence.
bool extension_conflicts = false;
+ std::string conflicting_header;
+ std::string winning_extension_id;
ResponseHeaders::const_iterator i;
for (i = (*delta)->deleted_response_headers.begin();
i != (*delta)->deleted_response_headers.end(); ++i) {
if (removed_headers.find(ToLowerCase(*i)) != removed_headers.end()) {
+ winning_extension_id = FindRemoveResponseHeader(deltas, i->first);
+ conflicting_header = i->first;
extension_conflicts = true;
break;
}
@@ -994,7 +1084,10 @@ void MergeOnHeadersReceivedResponses(
net::NetLog::TYPE_CHROME_EXTENSION_MODIFIED_HEADERS,
CreateNetLogExtensionIdCallback(delta->get()));
} else {
- conflicting_extensions->insert((*delta)->extension_id);
+ conflicting_extensions->insert(
+ ExtensionWarning::CreateResponseHeaderConflictWarning(
+ (*delta)->extension_id, winning_extension_id,
+ conflicting_header));
net_log->AddEvent(
net::NetLog::TYPE_CHROME_EXTENSION_IGNORED_DUE_TO_CONFLICT,
CreateNetLogExtensionIdCallback(delta->get()));
@@ -1008,10 +1101,11 @@ void MergeOnHeadersReceivedResponses(
bool MergeOnAuthRequiredResponses(
const EventResponseDeltas& deltas,
net::AuthCredentials* auth_credentials,
- std::set<std::string>* conflicting_extensions,
+ extensions::ExtensionWarningSet* conflicting_extensions,
const net::BoundNetLog* net_log) {
CHECK(auth_credentials);
bool credentials_set = false;
+ std::string winning_extension_id;
for (EventResponseDeltas::const_iterator delta = deltas.begin();
delta != deltas.end();
@@ -1023,7 +1117,9 @@ bool MergeOnAuthRequiredResponses(
(*delta)->auth_credentials->username() ||
auth_credentials->password() != (*delta)->auth_credentials->password();
if (credentials_set && different) {
- conflicting_extensions->insert((*delta)->extension_id);
+ conflicting_extensions->insert(
+ ExtensionWarning::CreateCredentialsConflictWarning(
+ (*delta)->extension_id, winning_extension_id));
net_log->AddEvent(
net::NetLog::TYPE_CHROME_EXTENSION_IGNORED_DUE_TO_CONFLICT,
CreateNetLogExtensionIdCallback(delta->get()));
@@ -1033,6 +1129,7 @@ bool MergeOnAuthRequiredResponses(
CreateNetLogExtensionIdCallback(delta->get()));
*auth_credentials = *(*delta)->auth_credentials;
credentials_set = true;
+ winning_extension_id = (*delta)->extension_id;
}
}
return credentials_set;

Powered by Google App Engine
This is Rietveld 408576698