Issue 12189018: <webview>: Implement WebRequest API

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+358 lines, -22 lines)			Patch
M	chrome/browser/chrome_content_browser_client.cc	View	1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20	2 chunks	+2 lines, -0 lines	0 comments	Download
M	chrome/browser/extensions/api/web_request/web_request_api.h	View	1 2 3 4 5 6 7 8 9 10 11 12 13	2 chunks	+9 lines, -2 lines	0 comments	Download
M	chrome/browser/extensions/api/web_request/web_request_api.cc	View	1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17	13 chunks	+94 lines, -15 lines	0 comments	Download
M	chrome/browser/extensions/event_router.h	View	1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17	1 chunk	+5 lines, -0 lines	0 comments	Download
M	chrome/browser/extensions/event_router.cc	View	1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17	2 chunks	+26 lines, -0 lines	0 comments	Download
A	chrome/browser/webview/webview_guest.h	View	1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22	1 chunk	+49 lines, -0 lines	0 comments	Download
A	chrome/browser/webview/webview_guest.cc	View	1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22	1 chunk	+80 lines, -0 lines	0 comments	Download
M	chrome/chrome_browser.gypi	View	1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20	1 chunk	+2 lines, -0 lines	0 comments	Download
M	chrome/common/extensions/api/web_request_internal.json	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/common/extensions/permissions/permission_set.cc	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+5 lines, -0 lines	0 comments	Download
M	chrome/renderer/resources/extensions/web_request_custom_bindings.js	View	1 2 3 4 5 6 7 8 9 10 11	3 chunks	+6 lines, -3 lines	0 comments	Download
M	chrome/renderer/resources/extensions/web_view.js	View	1 2 3 4 5 6 7 8 9 10 11	3 chunks	+8 lines, -0 lines	0 comments	Download
M	chrome/renderer/resources/extensions/web_view_experimental.js	View	1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17	2 chunks	+33 lines, -1 line	0 comments	Download
M	chrome/test/data/extensions/platform_apps/web_view/shim/main.js	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+14 lines, -0 lines	0 comments	Download
M	content/common/browser_plugin/browser_plugin_constants.h	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+1 line, -0 lines	0 comments	Download
M	content/common/browser_plugin/browser_plugin_constants.cc	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+1 line, -0 lines	0 comments	Download
M	content/renderer/browser_plugin/browser_plugin_bindings.cc	View	1 2 3 4 5 6 7 8 9 10 11	2 chunks	+21 lines, -0 lines	0 comments	Download

Messages

Total messages: 36 (0 generated)

Expand Messages | Collapse Messages

Fady Samuel

This patch is not yet complete and is not yet ready for formal review. Sharing ...

7 years, 10 months ago (2013-02-04 21:54:48 UTC) #1

Matt Perry

https://codereview.chromium.org/12189018/diff/3001/chrome/renderer/resources/extensions/web_view.js File chrome/renderer/resources/extensions/web_view.js (right): https://codereview.chromium.org/12189018/diff/3001/chrome/renderer/resources/extensions/web_view.js#newcode137 chrome/renderer/resources/extensions/web_view.js:137: 'webRequest.' + eventName, null, {}, null, right here, you ...

7 years, 10 months ago (2013-02-05 00:47:25 UTC) #2

Fady Samuel

Hi Matt, I believe I've addressed most of the points we discussed yesterday. Could you ...

7 years, 10 months ago (2013-02-05 17:18:35 UTC) #3

Matt Perry

https://codereview.chromium.org/12189018/diff/17013/chrome/browser/extensions/api/web_request/web_request_api.cc File chrome/browser/extensions/api/web_request/web_request_api.cc (right): https://codereview.chromium.org/12189018/diff/17013/chrome/browser/extensions/api/web_request/web_request_api.cc#newcode129 chrome/browser/extensions/api/web_request/web_request_api.cc:129: bool IsWebRequestEvent(std::string event_name) { Use a local temporary rather ...

7 years, 10 months ago (2013-02-06 01:55:14 UTC) #4

Fady Samuel

This code is not yet ready for review. Please see the other issue I sent ...

7 years, 10 months ago (2013-02-06 02:07:12 UTC) #5

Matt Perry

https://codereview.chromium.org/12189018/diff/17013/chrome/browser/extensions/api/web_request/web_request_api.cc File chrome/browser/extensions/api/web_request/web_request_api.cc (right): https://codereview.chromium.org/12189018/diff/17013/chrome/browser/extensions/api/web_request/web_request_api.cc#newcode1867 chrome/browser/extensions/api/web_request/web_request_api.cc:1867: if (!is_guest && extension->GetEffectiveHostPermissions().is_empty()) { On 2013/02/06 02:07:12, Fady ...

7 years, 10 months ago (2013-02-06 02:11:38 UTC) #6

Fady Samuel

This is mostly ready for review with two points missing: 1. chrome/browser/extensions/extension_process_manager.cc is hitting a ...

7 years, 10 months ago (2013-02-06 21:19:06 UTC) #7

Matt Perry

https://codereview.chromium.org/12189018/diff/14002/chrome/browser/chrome_content_browser_client.cc File chrome/browser/chrome_content_browser_client.cc (right): https://codereview.chromium.org/12189018/diff/14002/chrome/browser/chrome_content_browser_client.cc#newcode705 chrome/browser/chrome_content_browser_client.cc:705: BrowserThread::IO, indent+2 https://codereview.chromium.org/12189018/diff/14002/chrome/browser/chrome_content_browser_client.cc#newcode715 chrome/browser/chrome_content_browser_client.cc:715: void ChromeContentBrowserClient::RemoveWebViewEventListenersOnIOThread( nit: this sort ...

7 years, 10 months ago (2013-02-06 22:40:24 UTC) #8

jam

https://chromiumcodereview.appspot.com/12189018/diff/14002/content/public/browser/content_browser_client.h File content/public/browser/content_browser_client.h (right): https://chromiumcodereview.appspot.com/12189018/diff/14002/content/public/browser/content_browser_client.h#newcode121 content/public/browser/content_browser_client.h:121: virtual void GuestWebContentsDestroyed(WebContents* guest_web_contents, nit: don't add this since ...

7 years, 10 months ago (2013-02-06 23:34:00 UTC) #9

Fady Samuel

This patch was on the backburner for a couple of months. I've updated it and ...

7 years, 7 months ago (2013-05-15 21:58:53 UTC) #10

Matt Perry

https://codereview.chromium.org/12189018/diff/14002/chrome/browser/extensions/api/web_request/web_request_api.cc File chrome/browser/extensions/api/web_request/web_request_api.cc (right): https://codereview.chromium.org/12189018/diff/14002/chrome/browser/extensions/api/web_request/web_request_api.cc#newcode1210 chrome/browser/extensions/api/web_request/web_request_api.cc:1210: void ExtensionWebRequestEventRouter::RemoveWebViewEventListeners( On 2013/05/15 21:58:53, Fady Samuel wrote: > ...

7 years, 7 months ago (2013-05-16 21:16:38 UTC) #11

https://codereview.chromium.org/12189018/diff/14002/chrome/browser/extensions...
File chrome/browser/extensions/api/web_request/web_request_api.cc (right):

https://codereview.chromium.org/12189018/diff/14002/chrome/browser/extensions...
chrome/browser/extensions/api/web_request/web_request_api.cc:1210: void
ExtensionWebRequestEventRouter::RemoveWebViewEventListeners(
On 2013/05/15 21:58:53, Fady Samuel wrote:
> On 2013/02/06 22:40:24, Matt Perry wrote:
> > Are you sure this is necessary? When a context goes away, we call
> > chromeHidden.dispatchOnUnload (defined in event.js). That detaches all event
> > handlers for that context, which should in turn inform the browser process.
> > 
> > If that isn't working for webview, you'll also have to unregister the
> listeners
> > from the regular EventRouter.
> 
> Yes, this is necessary because the listeners a <webview> can go away while the
> embedder is still around.

I don't understand what you mean.

To clarify my comment: webRequest event listeners are stored both in the
ExtensionWebRequestEventRouter and in the main EventRouter. You'll need to
remove them from both.

Also, I'm curious why this isn't handled automatically by the code in
chromeHidden.dispatchOnUnload. Is dispatchOnUnload not getting called when a
<webview> goes away? If it's not, fixing that should remove the need for this
function.

https://codereview.chromium.org/12189018/diff/31001/chrome/browser/extensions...
File chrome/browser/extensions/extension_process_manager.cc (right):

https://codereview.chromium.org/12189018/diff/31001/chrome/browser/extensions...
chrome/browser/extensions/extension_process_manager.cc:433: // <webview>.
still?

https://codereview.chromium.org/12189018/diff/38001/chrome/browser/webview/we...
File chrome/browser/webview/webview_guest.h (right):

https://codereview.chromium.org/12189018/diff/38001/chrome/browser/webview/we...
chrome/browser/webview/webview_guest.h:12: class WebViewGuest : public
content::WebContentsObserver {
Please add comments to this class.

Fady Samuel

PTAL. Events to remove are determined on the IO thread and then events are removed ...

7 years, 7 months ago (2013-05-17 03:02:33 UTC) #12

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fsamuel@chromium.org/12189018/53001

7 years, 7 months ago (2013-05-17 19:30:52 UTC) #14

commit-bot: I haz the power

Failed to apply patch for chrome/browser/extensions/api/web_request/web_request_api.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...

7 years, 7 months ago (2013-05-17 19:30:57 UTC) #15

Failed to apply patch for
chrome/browser/extensions/api/web_request/web_request_api.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/api/web_request/web_request_api.cc
  Hunk #1 succeeded at 84 (offset 1 line).
  Hunk #2 succeeded at 131 (offset 1 line).
  Hunk #3 succeeded at 329 (offset 1 line).
  Hunk #4 succeeded at 393 (offset 1 line).
  Hunk #5 succeeded at 1154 (offset 1 line).
  Hunk #6 succeeded at 1168 (offset 1 line).
  Hunk #7 succeeded at 1215 (offset 1 line).
  Hunk #8 succeeded at 1325 (offset 1 line).
  Hunk #9 succeeded at 1342 (offset 1 line).
  Hunk #10 succeeded at 1358 (offset 1 line).
  Hunk #11 succeeded at 2046 (offset 1 line).
  Hunk #12 FAILED at 2076.
  Hunk #13 succeeded at 2086 (offset 2 lines).
  1 out of 13 hunks FAILED -- saving rejects to file
chrome/browser/extensions/api/web_request/web_request_api.cc.rej

Patch:       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
910a52f742e5c67da680efe43d0a92daa3ffec85..d6ff840d31e3c44515c40613edc5a2afb54af3ca
100644
--- a/chrome/browser/extensions/api/web_request/web_request_api.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_api.cc
@@ -83,6 +83,9 @@ namespace activitylog = activity_log_web_request_constants;
 
 namespace {
 
+const char kWebRequest[] = "webRequest";
+const char kWebView[] = "webview";
+
 // List of all the webRequest events.
 const char* const kWebRequestEvents[] = {
   keys::kOnBeforeRedirectEvent,
@@ -127,8 +130,11 @@ const char* GetRequestStageAsString(
 }
 
 bool IsWebRequestEvent(const std::string& event_name) {
+  std::string web_request_event_name(event_name);
+  if (web_request_event_name.find(kWebView) != std::string::npos)
+    web_request_event_name.replace(0, sizeof(kWebView) - 1, kWebRequest);
   return std::find(kWebRequestEvents, ARRAYEND(kWebRequestEvents),
-                   event_name) != ARRAYEND(kWebRequestEvents);
+                   web_request_event_name) != ARRAYEND(kWebRequestEvents);
 }
 
 // Returns whether |request| has been triggered by an extension in
@@ -322,6 +328,30 @@ StringValue* GetStatusLine(net::HttpResponseHeaders*
headers) {
   return new StringValue(headers ? headers->GetStatusLine() : std::string());
 }
 
+void RemoveEventListenerOnUI(
+  void* profile_id,
+  const std::string& event_name,
+  int process_id,
+  const std::string& extension_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  Profile* profile = reinterpret_cast<Profile*>(profile_id);
+  if (!g_browser_process->profile_manager()->IsValidProfile(profile))
+    return;
+
+  extensions::EventRouter* event_router =
+      extensions::ExtensionSystem::Get(profile)->event_router();
+  if (!event_router)
+    return;
+
+  content::RenderProcessHost* process =
+      content::RenderProcessHost::FromID(process_id);
+  if (!process)
+    return;
+
+  event_router->RemoveEventListener(event_name, process, extension_id);
+}
+
 // Sends an event to subscribers of chrome.declarativeWebRequest.onMessage.
 // |extension_id| identifies the extension that sends and receives the event.
 // |event_argument| is passed to the event listener.
@@ -362,8 +392,8 @@ struct ExtensionWebRequestEventRouter::EventListener {
   std::string sub_event_name;
   RequestFilter filter;
   int extra_info_spec;
-  int target_process_id;
-  int target_route_id;
+  int embedder_process_id;
+  int web_view_instance_id;
   base::WeakPtr<IPC::Sender> ipc_sender;
   mutable std::set<uint64> blocked_requests;
 
@@ -1123,9 +1153,10 @@ bool ExtensionWebRequestEventRouter::AddEventListener(
     const std::string& sub_event_name,
     const RequestFilter& filter,
     int extra_info_spec,
-    int target_process_id,
-    int target_route_id,
+    int embedder_process_id,
+    int web_view_instance_id,
     base::WeakPtr<IPC::Sender> ipc_sender) {
+
   if (!IsWebRequestEvent(event_name))
     return false;
 
@@ -1136,8 +1167,8 @@ bool ExtensionWebRequestEventRouter::AddEventListener(
   listener.filter = filter;
   listener.extra_info_spec = extra_info_spec;
   listener.ipc_sender = ipc_sender;
-  listener.target_process_id = target_process_id;
-  listener.target_route_id = target_route_id;
+  listener.embedder_process_id = embedder_process_id;
+  listener.web_view_instance_id = web_view_instance_id;
 
   if (listeners_[profile][event_name].count(listener) != 0u) {
     // This is likely an abuse of the API by a malicious extension.
@@ -1183,6 +1214,39 @@ void ExtensionWebRequestEventRouter::RemoveEventListener(
   helpers::ClearCacheOnNavigation();
 }
 
+void ExtensionWebRequestEventRouter::RemoveWebViewEventListeners(
+    void* profile,
+    const std::string& extension_id,
+    int embedder_process_id,
+    int web_view_instance_id) {
+  // Iterate over all listeners of all WebRequest events to delete
+  // any listeners that belong to the provided <webview>.
+  ListenerMapForProfile& map_for_profile = listeners_[profile];
+  for (ListenerMapForProfile::iterator event_iter = map_for_profile.begin();
+       event_iter != map_for_profile.end(); ++event_iter) {
+    std::vector<EventListener> listeners_to_delete;
+    std::set<EventListener>& listeners = event_iter->second;
+    for (std::set<EventListener>::iterator listener_iter = listeners.begin();
+         listener_iter != listeners.end(); ++listener_iter) {
+      const EventListener& listener = *listener_iter;
+      if (listener.embedder_process_id == embedder_process_id &&
+          listener.web_view_instance_id == web_view_instance_id)
+        listeners_to_delete.push_back(listener);
+    }
+    for (size_t i = 0; i < listeners_to_delete.size(); ++i) {
+      EventListener& listener = listeners_to_delete[i];
+      content::BrowserThread::PostTask(
+          content::BrowserThread::UI,
+          FROM_HERE,
+          base::Bind(&RemoveEventListenerOnUI,
+                     profile,
+                     listener.sub_event_name,
+                     embedder_process_id,
+                     extension_id));
+    }
+  }
+}
+
 void ExtensionWebRequestEventRouter::OnOTRProfileCreated(
     void* original_profile, void* otr_profile) {
   cross_profile_map_[original_profile] = otr_profile;
@@ -1260,10 +1324,15 @@ void
ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
     int* extra_info_spec,
     std::vector<const ExtensionWebRequestEventRouter::EventListener*>*
         matching_listeners) {
+  std::string web_request_event_name(event_name);
   ExtensionRendererState::WebViewInfo web_view_info;
   bool is_guest = ExtensionRendererState::GetInstance()->
       GetWebViewInfo(render_process_host_id, routing_id, &web_view_info);
-  std::set<EventListener>& listeners = listeners_[profile][event_name];
+  if (is_guest)
+    web_request_event_name.replace(0, sizeof(kWebRequest) - 1, kWebView);
+
+  std::set<EventListener>& listeners =
+      listeners_[profile][web_request_event_name];
   for (std::set<EventListener>::iterator it = listeners.begin();
        it != listeners.end(); ++it) {
     if (!it->ipc_sender.get()) {
@@ -1272,8 +1341,9 @@ void
ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
       continue;
     }
 
-    if (is_guest && (it->target_process_id != render_process_host_id||
-                     it->target_route_id != routing_id))
+    if (is_guest &&
+        (it->embedder_process_id != web_view_info.embedder_process_id ||
+         it->web_view_instance_id != web_view_info.web_view_instance_id))
       continue;
 
     if (!it->filter.urls.is_empty() && !it->filter.urls.MatchesURL(url))
@@ -1287,7 +1357,7 @@ void
ExtensionWebRequestEventRouter::GetMatchingListenersImpl(
                   resource_type) == it->filter.types.end())
       continue;
 
-    if (!WebRequestPermissions::CanExtensionAccessURL(
+    if (!is_guest && !WebRequestPermissions::CanExtensionAccessURL(
             extension_info_map, it->extension_id, url, crosses_incognito,
             WebRequestPermissions::REQUIRE_HOST_PERMISSION))
       continue;
@@ -1975,14 +2045,23 @@ bool WebRequestAddEventListener::RunImpl() {
   std::string sub_event_name;
   EXTENSION_FUNCTION_VALIDATE(args_->GetString(4, &sub_event_name));
 
+  int web_view_instance_id = 0;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(5, &web_view_instance_id));
+
+  base::WeakPtr<ChromeRenderMessageFilter> ipc_sender = ipc_sender_weak();
+
+  int embedder_process_id =
+      ipc_sender.get() ? ipc_sender->render_process_id() : -1;
+
   const Extension* extension =
       extension_info_map()->extensions().GetByID(extension_id());
   std::string extension_name = extension ? extension->name() : extension_id();
 
+  bool is_guest = web_view_instance_id != 0;
   // We check automatically whether the extension has the 'webRequest'
   // permission. For blocking calls we require the additional permission
   // 'webRequestBlocking'.
-  if ((extra_info_spec &
+  if ((!is_guest && extra_info_spec &
           (ExtensionWebRequestEventRouter::ExtraInfoSpec::BLOCKING |
            ExtensionWebRequestEventRouter::ExtraInfoSpec::ASYNC_BLOCKING)) &&
        !extension->HasAPIPermission(
@@ -1997,7 +2076,7 @@ bool WebRequestAddEventListener::RunImpl() {
   // 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 (extension->GetEffectiveHostPermissions().is_empty()) {
+  if (!is_guest && extension->GetEffectiveHostPermissions().is_empty()) {
     error_ = keys::kHostPermissionsRequired;
     return false;
   }
@@ -2005,8 +2084,8 @@ bool WebRequestAddEventListener::RunImpl() {
   bool success =
       ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
           profile_id(), extension_…
(message too large)

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fsamuel@chromium.org/12189018/62001

7 years, 7 months ago (2013-05-17 20:26:00 UTC) #16

commit-bot: I haz the power

Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=3608

7 years, 7 months ago (2013-05-17 20:35:30 UTC) #17

Charlie Reis

https://chromiumcodereview.appspot.com/12189018/diff/69001/content/browser/web_contents/web_contents_impl.cc File content/browser/web_contents/web_contents_impl.cc (right): https://chromiumcodereview.appspot.com/12189018/diff/69001/content/browser/web_contents/web_contents_impl.cc#newcode341 content/browser/web_contents/web_contents_impl.cc:341: browser_plugin_guest_->Destroy(); Is this something new that we didn't need ...

7 years, 7 months ago (2013-05-17 21:13:00 UTC) #18

Fady Samuel

On 2013/05/17 21:13:00, creis wrote: > https://chromiumcodereview.appspot.com/12189018/diff/69001/content/browser/web_contents/web_contents_impl.cc > File content/browser/web_contents/web_contents_impl.cc (right): > > https://chromiumcodereview.appspot.com/12189018/diff/69001/content/browser/web_contents/web_contents_impl.cc#newcode341 > ...

7 years, 7 months ago (2013-05-17 22:47:36 UTC) #19

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fsamuel@chromium.org/12189018/73001

7 years, 7 months ago (2013-05-17 22:47:56 UTC) #20

Fady Samuel

+jam@ for chrome/browser/webview/* review. This directory doesn't have an OWNER yet. I'll add myself and ...

7 years, 7 months ago (2013-05-17 22:54:42 UTC) #21

commit-bot: I haz the power

Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=3635

7 years, 7 months ago (2013-05-17 22:59:02 UTC) #22

Fady Samuel

jam@ is OOO, -jam@ +thakis@ for chrome/browser/webview/* review.

7 years, 7 months ago (2013-05-17 23:02:04 UTC) #23

Fady Samuel

+sky: Could you please provide a chrome OWNER approval? Nico seems OOO. I haven't been ...

7 years, 7 months ago (2013-05-21 18:26:13 UTC) #24

sky

What files do you need me to review? On Tue, May 21, 2013 at 11:26 ...

7 years, 7 months ago (2013-05-21 21:05:07 UTC) #25

sky

https://codereview.chromium.org/12189018/diff/73001/chrome/browser/webview/webview_guest.cc File chrome/browser/webview/webview_guest.cc (right): https://codereview.chromium.org/12189018/diff/73001/chrome/browser/webview/webview_guest.cc#newcode22 chrome/browser/webview/webview_guest.cc:22: static base::LazyInstance<WebViewProfileMap> g_webview_profile_map = This isn't global, it's static ...

7 years, 7 months ago (2013-05-22 00:44:18 UTC) #26

Fady Samuel

PTAL https://codereview.chromium.org/12189018/diff/73001/chrome/browser/webview/webview_guest.cc File chrome/browser/webview/webview_guest.cc (right): https://codereview.chromium.org/12189018/diff/73001/chrome/browser/webview/webview_guest.cc#newcode22 chrome/browser/webview/webview_guest.cc:22: static base::LazyInstance<WebViewProfileMap> g_webview_profile_map = On 2013/05/22 00:44:18, sky ...

7 years, 7 months ago (2013-05-23 04:37:37 UTC) #27

Fady Samuel

7 years, 7 months ago (2013-05-23 04:37:37 UTC) #28

sky

https://codereview.chromium.org/12189018/diff/84001/chrome/browser/webview/webview_guest.cc File chrome/browser/webview/webview_guest.cc (right): https://codereview.chromium.org/12189018/diff/84001/chrome/browser/webview/webview_guest.cc#newcode27 chrome/browser/webview/webview_guest.cc:27: Profile* profile, What if the profile is deleted between ...

7 years, 7 months ago (2013-05-23 15:41:10 UTC) #29

Fady Samuel

PTAL https://codereview.chromium.org/12189018/diff/84001/chrome/browser/webview/webview_guest.cc File chrome/browser/webview/webview_guest.cc (right): https://codereview.chromium.org/12189018/diff/84001/chrome/browser/webview/webview_guest.cc#newcode27 chrome/browser/webview/webview_guest.cc:27: Profile* profile, On 2013/05/23 15:41:11, sky wrote: > ...

7 years, 7 months ago (2013-05-24 03:13:47 UTC) #30

sky

7 years, 7 months ago (2013-05-24 15:58:03 UTC) #31

Fady Samuel

On 2013/05/24 15:58:03, sky wrote: > https://codereview.chromium.org/12189018/diff/84001/chrome/browser/webview/webview_guest.cc > File chrome/browser/webview/webview_guest.cc (right): > > https://codereview.chromium.org/12189018/diff/84001/chrome/browser/webview/webview_guest.cc#newcode27 > ...

7 years, 7 months ago (2013-05-25 03:25:50 UTC) #32

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fsamuel@chromium.org/12189018/96001

7 years, 7 months ago (2013-05-27 17:58:46 UTC) #35

Message was sent while issue was closed.

Change committed as 202469

Expand Messages | Collapse Messages

Issue 12189018: <webview>: Implement WebRequest API (Closed)

Description

Patch Set 1 #

Patch Set 2 : Support all events #

Patch Set 3 : Special case WebRequest listeners coming from <webview> #

Patch Set 4 : Use API definitions #

Patch Set 5 : Add parameters schema #

Patch Set 6 : Use webRequestDefinition #

Patch Set 7 : Removed unnecessary code #

Patch Set 8 : Lifetime of event listeners tied to <webview> lifetime #

Patch Set 9 : Diff from latest patch #

Patch Set 10 : Mark as experimental #

Patch Set 11 : Fixed DCHECK #

Patch Set 12 : Updated #

Patch Set 13 : Addressed comments #

Patch Set 14 : Merge with ToT + cleanup #

Patch Set 15 : Fixed unit test #

Patch Set 16 : Remove Events from EventRouter on WebView destruction #

Patch Set 17 : Fixed a comment #

Patch Set 18 : Merge with ToT #

Patch Set 19 : Minor fix to tear down #

Patch Set 20 : Removing tear down fix. Landing as a separate patch #

Patch Set 21 : Addressed sky@'s comments #

Patch Set 22 : Addressed sky's comments #

Patch Set 23 : Profile* => void* #

Messages