Fix the Declarative WebRequest API for <webview>

chrome/browser/extensions/api/web_request/web_request_api.cc

Index: chrome/browser/extensions/api/web_request/web_request_api.cc
diff --git a/chrome/browser/extensions/api/web_request/web_request_api.cc b/chrome/browser/extensions/api/web_request/web_request_api.cc
index eef2a8075936f7784ace561b25dd44f2d2f08ca9..a85f8d7e4b39d5da4c19b7ac515d10db1f3670c7 100644
--- a/chrome/browser/extensions/api/web_request/web_request_api.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_api.cc
@@ -33,6 +33,7 @@
 #include "chrome/browser/extensions/extension_renderer_state.h"
 #include "chrome/browser/extensions/extension_warning_service.h"
 #include "chrome/browser/extensions/extension_warning_set.h"
+#include "chrome/browser/guest_view/web_view/web_view_constants.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/extensions/api/web_request.h"
@@ -163,6 +164,29 @@ bool IsRequestFromExtension(const net::URLRequest* request,
   return extension_info_map->process_map().Contains(info->GetChildID());
 }
 
+void ExtractRequestRoutingInfo(net::URLRequest* request,
+                               int* render_process_host_id,
+                               int* routing_id) {
+  if (!request->GetUserData(NULL))
+    return;
+  const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request);
+  *render_process_host_id = info->GetChildID();
+  *routing_id = info->GetRouteID();
+}
+
+// Given a |request|, this function determines whether it originated from
+// a <webview> guest process or not. If it is from a <webview> guest process,
+// then |web_view_info| is returned with information about the instance ID
+// that uniquely identifies the <webview> and its embedder.
+bool GetWebViewInfo(net::URLRequest* request,
+                    ExtensionRendererState::WebViewInfo* web_view_info) {
+  int render_process_host_id = -1;
+  int routing_id = -1;
+  ExtractRequestRoutingInfo(request, &render_process_host_id, &routing_id);
+  return ExtensionRendererState::GetInstance()->
+      GetWebViewInfo(render_process_host_id, routing_id, web_view_info);
+}
+
 void ExtractRequestInfoDetails(net::URLRequest* request,
                                bool* is_main_frame,
                                int64* frame_id,
@@ -363,12 +387,19 @@ void RemoveEventListenerOnUI(
   event_router->RemoveEventListener(event_name, process, extension_id);
 }
 
-// Sends an event to subscribers of chrome.declarativeWebRequest.onMessage.
+// Sends an event to subscribers of chrome.declarativeWebRequest.onMessage or
+// to subscribers of webview.onMessage if the action is being operated upon
+// a <webview> guest renderer.
 // |extension_id| identifies the extension that sends and receives the event.
+// |is_web_view_guest| indicates whether the action is for a <webview>.
+// |web_view_info| is a struct containing information about the <webview>
+// embedder.
 // |event_argument| is passed to the event listener.
 void SendOnMessageEventOnUI(
     void* profile_id,
     const std::string& extension_id,
+    int is_web_view_guest,

[battre, 2014/05/28 15:15:21]

bool?

[Fady Samuel, 2014/05/29 15:13:27]

Done.

+    const ExtensionRendererState::WebViewInfo& web_view_info,
     scoped_ptr<base::DictionaryValue> event_argument) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -381,10 +412,16 @@ void SendOnMessageEventOnUI(
 
   extensions::EventRouter* event_router = extensions::EventRouter::Get(profile);
 
+  extensions::EventFilteringInfo event_filtering_info;

[battre, 2014/05/28 15:15:21]

More stuff that could use some documentation. ;-)

No need to do it here, but I think that at least service_type_ and instance_id_
in EventfilteringInfo could use some comments.

[Fady Samuel, 2014/05/29 15:13:27]

I have no clue what service_type_ is but instance_id belongs to guest_view. I
added a comment.

+  event_filtering_info.SetURL(GURL());

[battre, 2014/05/28 15:15:21]

This looks crude... You set a URL such that EventFilteringInfo::has_url_ becomes
true but then url().is_valid() returns false? I think this is semantics that
should be described in EventFilteringInfo.

[Fady Samuel, 2014/05/29 15:13:27]

This is actually unnecessary. I've removed it.

+  if (is_web_view_guest)
+    event_filtering_info.SetInstanceID(web_view_info.instance_id);
+
   scoped_ptr<extensions::Event> event(new extensions::Event(
-      declarative_keys::kOnMessage, event_args.Pass(), profile,
-      GURL(), extensions::EventRouter::USER_GESTURE_UNKNOWN,
-      extensions::EventFilteringInfo()));
+      is_web_view_guest ? webview::kEventMessage : declarative_keys::kOnMessage,
+      event_args.Pass(), profile, GURL(),
+      extensions::EventRouter::USER_GESTURE_UNKNOWN,
+      event_filtering_info));
   event_router->DispatchEventToExtension(extension_id, event.Pass());
 }
 
@@ -1419,10 +1456,10 @@ void ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
     std::vector<const ExtensionWebRequestEventRouter::EventListener*>*
         matching_listeners) {
   std::string web_request_event_name(event_name);
-  ExtensionRendererState::WebViewInfo webview_info;
-  bool is_guest = ExtensionRendererState::GetInstance()->
-      GetWebViewInfo(render_process_host_id, routing_id, &webview_info);
-  if (is_guest)
+  ExtensionRendererState::WebViewInfo web_view_info;
+  bool is_web_view_guest = ExtensionRendererState::GetInstance()->
+      GetWebViewInfo(render_process_host_id, routing_id, &web_view_info);
+  if (is_web_view_guest)
     web_request_event_name.replace(0, sizeof(kWebRequest) - 1, kWebView);
 
   std::set<EventListener>& listeners =
@@ -1435,9 +1472,9 @@ void ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
       continue;
     }
 
-    if (is_guest &&
-        (it->embedder_process_id != webview_info.embedder_process_id ||
-         it->webview_instance_id != webview_info.instance_id))
+    if (is_web_view_guest &&
+        (it->embedder_process_id != web_view_info.embedder_process_id ||
+         it->webview_instance_id != web_view_info.instance_id))
       continue;
 
     if (!it->filter.urls.is_empty() && !it->filter.urls.MatchesURL(url))
@@ -1451,7 +1488,7 @@ void ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
                   resource_type) == it->filter.types.end())
       continue;
 
-    if (!is_guest && !WebRequestPermissions::CanExtensionAccessURL(
+    if (!is_web_view_guest && !WebRequestPermissions::CanExtensionAccessURL(
             extension_info_map, it->extension_id, url, crosses_incognito,
             WebRequestPermissions::REQUIRE_HOST_PERMISSION))
       continue;
@@ -1793,6 +1830,9 @@ void ExtensionWebRequestEventRouter::SendMessages(
          message != messages.end(); ++message) {
       scoped_ptr<base::DictionaryValue> argument(new base::DictionaryValue);
       ExtractRequestInfo(blocked_request.request, argument.get());
+      ExtensionRendererState::WebViewInfo web_view_info;
+      bool is_web_view_guest = GetWebViewInfo(blocked_request.request,
+                                              &web_view_info);
       argument->SetString(keys::kMessageKey, *message);
       argument->SetString(keys::kStageKey,
                           GetRequestStageAsString(blocked_request.event));
@@ -1803,6 +1843,8 @@ void ExtensionWebRequestEventRouter::SendMessages(
           base::Bind(&SendOnMessageEventOnUI,
                      profile,
                      (*delta)->extension_id,
+                     is_web_view_guest,
+                     web_view_info,
                      base::Passed(&argument)));
     }
   }
@@ -1917,29 +1959,13 @@ bool ExtensionWebRequestEventRouter::ProcessDeclarativeRules(
     net::URLRequest* request,
     extensions::RequestStage request_stage,
     const net::HttpResponseHeaders* original_response_headers) {
-  bool is_main_frame = false;
-  int64 frame_id = -1;
-  bool parent_is_main_frame = false;
-  int64 parent_frame_id = -1;
-  int tab_id = -1;
-  int window_id = -1;
-  int render_process_host_id = -1;
-  int routing_id = -1;
-  ResourceType::Type resource_type = ResourceType::LAST_TYPE;
-
-  ExtractRequestInfoDetails(request, &is_main_frame, &frame_id,
-                            &parent_is_main_frame, &parent_frame_id,
-                            &tab_id, &window_id, &render_process_host_id,
-                            &routing_id, &resource_type);
-  ExtensionRendererState::WebViewInfo webview_info;
-  bool is_guest = ExtensionRendererState::GetInstance()->
-      GetWebViewInfo(render_process_host_id, routing_id, &webview_info);
+  ExtensionRendererState::WebViewInfo web_view_info;
+  bool is_web_view_guest = GetWebViewInfo(request, &web_view_info);
 
   RulesRegistryService::WebViewKey webview_key(
-      is_guest ? webview_info.embedder_process_id : 0,
-      is_guest ? webview_info.instance_id : 0);
+      is_web_view_guest ? web_view_info.embedder_process_id : 0,
+      is_web_view_guest ? web_view_info.instance_id : 0);
   RulesRegistryKey rules_key(profile, webview_key);
-
   // If this check fails, check that the active stages are up-to-date in
   // browser/extensions/api/declarative_webrequest/request_stage.h .
   DCHECK(request_stage & extensions::kActiveStages);
@@ -2183,11 +2209,11 @@ bool WebRequestInternalAddEventListenerFunction::RunSync() {
       extension_info_map()->extensions().GetByID(extension_id());
   std::string extension_name = extension ? extension->name() : extension_id();
 
-  bool is_guest = webview_instance_id != 0;
+  bool is_web_view_guest = webview_instance_id != 0;
   // We check automatically whether the extension has the 'webRequest'
   // permission. For blocking calls we require the additional permission
   // 'webRequestBlocking'.
-  if ((!is_guest && extra_info_spec &
+  if ((!is_web_view_guest && extra_info_spec &
           (ExtensionWebRequestEventRouter::ExtraInfoSpec::BLOCKING |
            ExtensionWebRequestEventRouter::ExtraInfoSpec::ASYNC_BLOCKING)) &&
        !extension->HasAPIPermission(
@@ -2202,7 +2228,8 @@ bool WebRequestInternalAddEventListenerFunction::RunSync() {
   // http://www.example.com/bar/*.
   // For this reason we do only a coarse check here to warn the extension
   // developer if he does something obviously wrong.
-  if (!is_guest && extensions::PermissionsData::GetEffectiveHostPermissions(
+  if (!is_web_view_guest &&
+      extensions::PermissionsData::GetEffectiveHostPermissions(
           extension).is_empty()) {
     error_ = keys::kHostPermissionsRequired;
     return false;