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

Unified Diff: extensions/browser/api/web_request/web_request_api.cc

Issue 1413543005: Use FrameTreeNode ID as frameId in extension APIs (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Nits in #16, s/FindByFrameTreeNodeID/FindFrameByFrameTreeNodeID/ Created 5 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: extensions/browser/api/web_request/web_request_api.cc
diff --git a/extensions/browser/api/web_request/web_request_api.cc b/extensions/browser/api/web_request/web_request_api.cc
index 33616986882c26ad23ae7afcf88338c3875cb35b..87ff4ad4eeefff215dabc79a584e1a81e4e9baa7 100644
--- a/extensions/browser/api/web_request/web_request_api.cc
+++ b/extensions/browser/api/web_request/web_request_api.cc
@@ -127,10 +127,6 @@ const char* GetRequestStageAsString(
return "Not reached";
}
-int GetFrameId(bool is_main_frame, int frame_id) {
- return is_main_frame ? 0 : frame_id;
-}
-
bool IsWebRequestEvent(const std::string& event_name) {
std::string web_request_event_name(event_name);
if (base::StartsWith(web_request_event_name,
@@ -202,8 +198,6 @@ bool GetWebViewInfo(const net::URLRequest* request,
void ExtractRequestInfoDetails(const net::URLRequest* request,
bool* is_main_frame,
int* frame_id,
- bool* parent_is_main_frame,
- int* parent_frame_id,
int* render_process_host_id,
int* routing_id,
ResourceType* resource_type) {
@@ -213,8 +207,6 @@ void ExtractRequestInfoDetails(const net::URLRequest* request,
*frame_id = info->GetRenderFrameID();
*is_main_frame = info->IsMainFrame();
- *parent_frame_id = info->GetParentRenderFrameID();
- *parent_is_main_frame = info->ParentIsMainFrame();
*render_process_host_id = info->GetChildID();
*routing_id = info->GetRouteID();
@@ -225,6 +217,40 @@ void ExtractRequestInfoDetails(const net::URLRequest* request,
*resource_type = content::RESOURCE_TYPE_LAST_TYPE;
}
+// Enriches |dict| with information that can only be determined on the UI
+// thread, such as frameId. |dict| must be populated by ExtractRequestInfo
+// before calling this method.
+void ExtractRequestInfoOnUI(base::DictionaryValue* dict) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ DCHECK(dict);
+
+ // Update frameId / parentFrameId.
+ // -1 if unknown
+ // 0 if top-level frame
+ // >0 otherwise.
+ int render_process_id = -1;
+ int render_frame_id = -1;
nasko 2015/11/06 23:28:27 What type of id is this? The FTN or the routing id
+ int frame_tree_node_id = -1;
+ int parent_frame_tree_node_id = -1;
+ if (dict->GetInteger(keys::kFrameIdKey, &render_frame_id) &&
+ dict->GetInteger(keys::kProcessIdKey, &render_process_id)) {
+ content::RenderFrameHost* rfh =
+ content::RenderFrameHost::FromID(render_process_id, render_frame_id);
nasko 2015/11/06 23:28:27 If this comes from caller of the extension API, wo
robwu 2015/11/07 00:23:40 Heh. This ID is correct. I'm abusing kFrameIdKey a
+ if (rfh) {
+ if (content::RenderFrameHost* parent = rfh->GetParent()) {
nasko 2015/11/06 23:28:27 Assignment should not be done inside an if clause.
+ frame_tree_node_id = rfh->GetFrameTreeNodeID();
+ parent_frame_tree_node_id =
+ parent->GetParent() ? parent->GetFrameTreeNodeID() : 0;
+ } else {
+ frame_tree_node_id = 0;
+ }
+ }
+ }
+ dict->SetInteger(keys::kFrameIdKey, frame_tree_node_id);
+ dict->SetInteger(keys::kParentFrameIdKey, parent_frame_tree_node_id);
+ dict->Remove(keys::kProcessIdKey, nullptr);
nasko 2015/11/06 23:28:27 Why are we removing this? I feel strongly about ke
robwu 2015/11/07 00:23:40 This is the webRequest API. processId was never in
+}
+
// Extracts the body from |request| and writes the data into |out|.
void ExtractRequestInfoBody(const net::URLRequest* request,
base::DictionaryValue* out) {
@@ -357,6 +383,7 @@ void SendOnMessageEventOnUI(
return;
scoped_ptr<base::ListValue> event_args(new base::ListValue);
+ ExtractRequestInfoOnUI(event_argument.get());
event_args->Append(event_argument.release());
EventRouter* event_router = EventRouter::Get(browser_context);
@@ -523,8 +550,9 @@ struct ExtensionWebRequestEventRouter::EventListener {
// is also used to find and remove an event listener when an extension is
// unloaded. At this point, the event listener cannot be mapped back to
// the original process, so 0 is used instead of the actual process ID.
- DCHECK(embedder_process_id == 0 || that.embedder_process_id == 0);
- return false;
+ if (embedder_process_id == 0 || that.embedder_process_id == 0) {
+ return false;
+ }
}
if (embedder_process_id != that.embedder_process_id)
@@ -741,27 +769,21 @@ void ExtensionWebRequestEventRouter::ExtractRequestInfo(
base::DictionaryValue* out) {
bool is_main_frame = false;
int frame_id = -1;
nasko 2015/11/06 23:28:27 I think this is the frame routing id, correct? If
robwu 2015/12/07 23:44:22 Done.
- bool parent_is_main_frame = false;
- int parent_frame_id = -1;
- int frame_id_for_extension = -1;
- int parent_frame_id_for_extension = -1;
int render_process_host_id = -1;
int routing_id = -1;
ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE;
ExtractRequestInfoDetails(request, &is_main_frame, &frame_id,
- &parent_is_main_frame, &parent_frame_id,
&render_process_host_id, &routing_id,
&resource_type);
- frame_id_for_extension = GetFrameId(is_main_frame, frame_id);
- parent_frame_id_for_extension = GetFrameId(parent_is_main_frame,
- parent_frame_id);
out->SetString(keys::kRequestIdKey,
base::Uint64ToString(request->identifier()));
out->SetString(keys::kUrlKey, request->url().spec());
out->SetString(keys::kMethodKey, request->method());
- out->SetInteger(keys::kFrameIdKey, frame_id_for_extension);
- out->SetInteger(keys::kParentFrameIdKey, parent_frame_id_for_extension);
+ // Note: This (frameId, processId) pair is used by ExtractRequestInfoOnUI to
+ // find a RenderFrameHost, which provides the final frameId.
+ out->SetInteger(keys::kFrameIdKey, frame_id);
+ out->SetInteger(keys::kProcessIdKey, render_process_host_id);
out->SetString(keys::kTypeKey, helpers::ResourceTypeToString(resource_type));
out->SetDouble(keys::kTimeStampKey, base::Time::Now().ToDoubleT() * 1000);
if (web_request_event_router_delegate_) {
@@ -1254,21 +1276,12 @@ bool ExtensionWebRequestEventRouter::DispatchEvent(
// 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 (const EventListener* listener : listeners) {
- // Filter out the optional keys that this listener didn't request.
- scoped_ptr<base::ListValue> args_filtered(args.DeepCopy());
- base::DictionaryValue* dict = NULL;
- CHECK(args_filtered->GetDictionary(0, &dict) && dict);
- if (!(listener->extra_info_spec & ExtraInfoSpec::REQUEST_HEADERS))
- dict->Remove(keys::kRequestHeadersKey, NULL);
- if (!(listener->extra_info_spec & ExtraInfoSpec::RESPONSE_HEADERS))
- dict->Remove(keys::kResponseHeadersKey, NULL);
- EventRouter::DispatchEventToSender(
- listener->ipc_sender.get(), browser_context, listener->extension_id,
- listener->histogram_value, listener->sub_event_name,
- args_filtered.Pass(), EventRouter::USER_GESTURE_UNKNOWN,
- EventFilteringInfo());
+ scoped_ptr<std::vector<EventListener>> listeners_to_dispatch(
+ new std::vector<EventListener>());
+ listeners_to_dispatch->reserve(listeners.size());
+ for (const EventListener* listener : listeners) {
+ listeners_to_dispatch->push_back(*listener);
if (listener->extra_info_spec &
(ExtraInfoSpec::BLOCKING | ExtraInfoSpec::ASYNC_BLOCKING)) {
listener->blocked_requests.insert(request->identifier());
@@ -1286,6 +1299,18 @@ bool ExtensionWebRequestEventRouter::DispatchEvent(
}
}
+ // TODO(robwu): Avoid unnecessary copy, by changing |args| to be a
+ // scoped_ptr<base::DictionaryValue> and transferring the ownership.
+ const base::DictionaryValue* dict = nullptr;
+ CHECK(args.GetDictionary(0, &dict) && dict);
+ base::DictionaryValue* args_copy = dict->DeepCopy();
+ BrowserThread::PostTaskAndReply(
+ BrowserThread::UI, FROM_HERE,
+ base::Bind(&ExtractRequestInfoOnUI, base::Unretained(args_copy)),
+ base::Bind(&ExtensionWebRequestEventRouter::DispatchEventOnIO,
+ base::Unretained(this), browser_context,
+ base::Passed(&listeners_to_dispatch), base::Owned(args_copy)));
+
if (num_handlers_blocking > 0) {
BlockedRequest& blocked_request = blocked_requests_[request->identifier()];
blocked_request.request = request;
@@ -1298,6 +1323,54 @@ bool ExtensionWebRequestEventRouter::DispatchEvent(
return false;
}
+void ExtensionWebRequestEventRouter::DispatchEventOnIO(
+ void* browser_context,
+ scoped_ptr<std::vector<EventListener>> listeners,
+ base::DictionaryValue* dict) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ DCHECK(listeners.get());
+ DCHECK_GT(listeners->size(), 0UL);
+ DCHECK(dict);
+
+ std::string event_name =
+ EventRouter::GetBaseEventName((*listeners)[0].sub_event_name);
+ DCHECK(IsWebRequestEvent(event_name));
+
+ const std::set<EventListener>& event_listeners =
+ listeners_[browser_context][event_name];
+ void* cross_browser_context = GetCrossBrowserContext(browser_context);
+ const std::set<EventListener>* cross_event_listeners =
+ cross_browser_context ? &listeners_[cross_browser_context][event_name]
+ : nullptr;
+
+ for (const EventListener& target : *listeners) {
+ std::set<EventListener>::const_iterator listener =
+ event_listeners.find(target);
+ // Ignore listener if it was removed between the thread hops.
+ if (listener == event_listeners.end()) {
+ if (!cross_event_listeners)
+ continue;
+ listener = cross_event_listeners->find(target);
+ if (listener == cross_event_listeners->end())
+ continue;
+ }
+
+ // Filter out the optional keys that this listener didn't request.
+ scoped_ptr<base::ListValue> args_filtered(new base::ListValue);
+ args_filtered->Append(dict->DeepCopy());
+ if (!(listener->extra_info_spec & ExtraInfoSpec::REQUEST_HEADERS))
+ dict->Remove(keys::kRequestHeadersKey, nullptr);
+ if (!(listener->extra_info_spec & ExtraInfoSpec::RESPONSE_HEADERS))
+ dict->Remove(keys::kResponseHeadersKey, nullptr);
+
+ EventRouter::DispatchEventToSender(
+ listener->ipc_sender.get(), browser_context, listener->extension_id,
+ listener->histogram_value, listener->sub_event_name,
+ args_filtered.Pass(), EventRouter::USER_GESTURE_UNKNOWN,
+ EventFilteringInfo());
+ }
+}
+
void ExtensionWebRequestEventRouter::OnEventHandled(
void* browser_context,
const std::string& extension_id,
@@ -1452,14 +1525,11 @@ bool ExtensionWebRequestEventRouter::IsPageLoad(
const net::URLRequest* request) const {
bool is_main_frame = false;
int frame_id = -1;
- bool parent_is_main_frame = false;
- int parent_frame_id = -1;
int render_process_host_id = -1;
int routing_id = -1;
ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE;
ExtractRequestInfoDetails(request, &is_main_frame, &frame_id,
- &parent_is_main_frame, &parent_frame_id,
&render_process_host_id,
&routing_id, &resource_type);
@@ -1602,15 +1672,12 @@ ExtensionWebRequestEventRouter::GetMatchingListeners(
bool is_main_frame = false;
int frame_id = -1;
- bool parent_is_main_frame = false;
- int parent_frame_id = -1;
int render_process_host_id = -1;
int routing_id = -1;
ResourceType resource_type = content::RESOURCE_TYPE_LAST_TYPE;
const GURL& url = request->url();
ExtractRequestInfoDetails(request, &is_main_frame, &frame_id,
- &parent_is_main_frame, &parent_frame_id,
&render_process_host_id,
&routing_id, &resource_type);

Powered by Google App Engine
This is Rietveld 408576698