Fix the Declarative WebRequest API for <webview>

chrome/renderer/resources/extensions/web_view.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This module implements Webview (<webview>) as a custom element that wraps a
 // BrowserPlugin object element. The object element is hidden within
 // the shadow DOM of the Webview element.
 
 var DocumentNatives = requireNative('document_natives');
 var EventBindings = require('event_bindings');
@@ skipping 963 matching lines @@
     WebView.setPermission(
         self.instanceId, requestId, 'default', '', function(allowed) {
       if (allowed) {
         return;
       }
       showWarningMessage(event.permission);
     });
   }
 };
 
+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
  */
 WebViewInternal.prototype.setupWebRequestEvents = function() {
   var self = this;
   var request = {};
   var createWebRequestEvent = function(webRequestEvent) {
     return function() {
       if (!self[webRequestEvent.name]) {
         self[webRequestEvent.name] =
             new WebRequestEvent(
                 'webview.' + webRequestEvent.name,
                 webRequestEvent.parameters,
                 webRequestEvent.extraParameters, webRequestEvent.options,
                 self.viewInstanceId);
       }
       return self[webRequestEvent.name];
     };
   };
 
+  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.
   for (var i = 0; i < WebRequestSchema.events.length; ++i) {
     var webRequestEvent = createWebRequestEvent(WebRequestSchema.events[i]);
     Object.defineProperty(
         request,
         WebRequestSchema.events[i].name,
         {
           get: webRequestEvent,
           enumerable: true
         }
     );
-    this.maybeAttachWebRequestEventToObject(this.webviewNode,
-                                            WebRequestSchema.events[i].name,
-                                            webRequestEvent);
   }
   Object.defineProperty(
       this.webviewNode,
       'request',
       {
         value: request,
         enumerable: true
       }
   );
 };
@@ skipping 178 matching lines @@
 /**
  * Implemented when the experimental API is available.
  * @private
  */
 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 [];
 };
 
 /**
  * Calls to show contextmenu right away instead of dispatching a 'contextmenu'
  * event.
  * This will be overridden in web_view_experimental.js to implement contextmenu
  * API.
  * @private
  */
 WebViewInternal.prototype.maybeHandleContextMenu = function(e, webViewEvent) {
   var requestId = e.requestId;
   // Setting |params| = undefined will show the context menu unmodified, hence
   // the 'contextmenu' API is disabled for stable channel.
   var params = undefined;
   WebView.showContextMenu(this.instanceId, requestId, params);
 };
 
 /**
  * Implemented when the experimental API is available.
  * @private
  */
 WebViewInternal.prototype.setupExperimentalContextMenus_ = function() {};
 
 exports.WebView = WebView;
 exports.WebViewInternal = WebViewInternal;
 exports.CreateEvent = CreateEvent;