Fix the Declarative WebRequest API for <webview>

chrome/renderer/resources/extensions/web_view.js

Index: chrome/renderer/resources/extensions/web_view.js
diff --git a/chrome/renderer/resources/extensions/web_view.js b/chrome/renderer/resources/extensions/web_view.js
index 77bc1a433ce395b36a6da4cb29b393cdc6999db8..17c2aed9cdbeca8ed2ad52b445970eefd2af201b 100644
--- a/chrome/renderer/resources/extensions/web_view.js
+++ b/chrome/renderer/resources/extensions/web_view.js
@@ -981,6 +981,28 @@ WebViewInternal.prototype.handlePermissionEvent =
   }
 };
 
+var WebRequestMessageEvent = CreateEvent('webview.onMessage');
+
+function DeclarativeWebRequestEvent(opt_eventName,
+                                    opt_argSchemas,
+                                    opt_eventOptions,
+                                    opt_webViewInstanceId) {
+  var subEventName = opt_eventName + '/' + IdGenerator.GetNextId();
+  EventBindings.Event.call(this, subEventName, opt_argSchemas, opt_eventOptions,
+      opt_webViewInstanceId);
+
+  var self = this;
+  // TODO(lazyboy): When do we dispose this listener?

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

Isn't this taken care of by
ExtensionWebRequestEventRouter::RemoveWebViewEventListeners?

If that's the case, please add a comment.

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

No. This is a very crude way to do cleanup. These events are cleaned up when he
embedder goes away, not when the webview goes away. A webview's lifetime may be
significantly shorter than an embedder's lifetime and so we might not want a
bunch of event listeners hanging around. I don't have a good solution for this
yet but we're working on it.

+  WebRequestMessageEvent.addListener(function() {
+    // Re-dispatch to subEvent's listeners.
+    $Function.apply(self.dispatch, self, $Array.slice(arguments));
+  }, {instanceId: opt_webViewInstanceId || 0});

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

I am lacking the details here, but do you allow one webview to listen for
request messages of another webview?

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

No. Hence the filter by instance ID. Each webview has a unique instance ID. We
don't want the developer to worry about that detail though (it's an
implementation detail).

+}
+
+DeclarativeWebRequestEvent.prototype = {
+  __proto__: EventBindings.Event.prototype
+};
+
 /**
  * @private
  */
@@ -1001,12 +1023,34 @@ WebViewInternal.prototype.setupWebRequestEvents = function() {
     };
   };
 
+  var createDeclarativeWebRequestEvent = function(webRequestEvent) {
+    return function() {
+      if (!self[webRequestEvent.name]) {
+        // The onMessage event gets a special event type.

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

Document why?

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

Done.

+        var EventObj = webRequestEvent.name === 'onMessage' ?

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

EventClass?

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

Done.

+            DeclarativeWebRequestEvent : EventBindings.Event;
+        self[webRequestEvent.name] =
+            new EventObj(
+                'webview.' + webRequestEvent.name,
+                webRequestEvent.parameters,
+                webRequestEvent.options,
+                self.viewInstanceId);
+      }
+      return self[webRequestEvent.name];
+    };
+  };
+
   for (var i = 0; i < DeclarativeWebRequestSchema.events.length; ++i) {
     var eventSchema = DeclarativeWebRequestSchema.events[i];
-    var webRequestEvent = createWebRequestEvent(eventSchema);
-    this.maybeAttachWebRequestEventToObject(request,
-                                            eventSchema.name,
-                                            webRequestEvent);
+    var webRequestEvent = createDeclarativeWebRequestEvent(eventSchema);
+    Object.defineProperty(
+        request,
+        eventSchema.name,
+        {
+          get: webRequestEvent,
+          enumerable: true
+        }
+    );
   }
 
   // Populate the WebRequest events from the API definition.
@@ -1020,9 +1064,6 @@ WebViewInternal.prototype.setupWebRequestEvents = function() {
           enumerable: true
         }
     );
-    this.maybeAttachWebRequestEventToObject(this.webviewNode,
-                                            WebRequestSchema.events[i].name,
-                                            webRequestEvent);
   }
   Object.defineProperty(
       this.webviewNode,
@@ -1221,12 +1262,6 @@ WebViewInternal.prototype.maybeGetExperimentalEvents = function() {};
  * Implemented when the experimental API is available.
  * @private
  */
-WebViewInternal.prototype.maybeAttachWebRequestEventToObject = function() {};

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

I have no idea about this code. It would be great to get a reviewer who is
familiar with this.

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

Yes, will do.

-
-/**
- * Implemented when the experimental API is available.
- * @private
- */
 WebViewInternal.prototype.maybeGetExperimentalPermissions = function() {
   return [];
 };