Improve <adview> implementation and add tests.

chrome/renderer/resources/extensions/ad_view.js

 // Copyright (c) 2013 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.
 
 // Shim that simulates a <adview> tag via Mutation Observers.
 //
 // The actual tag is implemented via the browser plugin. The internals of this
 // are hidden via Shadow DOM.
 
 // TODO(rpaquay): This file is currently very similar to "web_view.js". Do we
 //                want to refactor to extract common pieces?
 
+

[benwells, 2013/04/02 00:49:45]

Nit: remove extra blank line

[rpaquay, 2013/04/02 16:44:47]

Done.

 var adViewCustom = require('adViewCustom');
 var chrome = requireNative('chrome').GetChrome();
 var forEach = require('utils').forEach;
 var watchForTag = require('tagWatcher').watchForTag;
 
 

[benwells, 2013/04/02 00:49:45]

Nit: remove extra blank line

[rpaquay, 2013/04/02 16:44:47]

Done.

+/**
+ * Define "allowCustomAdNetworks" function such that it returns "true" if the
+ * "adViewCustom" module was injected. This is so that the

[benwells, 2013/04/02 00:49:45]

Is there some precedent for this style of multi line comment? For consistency
with the rest of chrome I think you should just use the normal pattern for
comments: that is, just "//" on each line.

[rpaquay, 2013/04/02 16:44:47]

There are a few files in this same folder using multi line comments
(web_view.js, last_error.js, platform_app.js). That (and also reading the
Javascript Style Guide) is the reason I made the switch to multi line comments.

+ * "kEnableAdviewSrcAttribute" flag is respected.
+ */
 var allowCustomAdNetworks = (function(allow){
   return function() { return Boolean(allow); }
 })(adViewCustom ? adViewCustom.enabled : false);
 
 

[benwells, 2013/04/02 00:49:45]

Nit: remove extra blank line

[rpaquay, 2013/04/02 16:44:47]

Done.

-// List of attribute names to "blindly" sync between <adview> tag and internal
-// browser plugin.
+/**
+ * List of attribute names to "blindly" sync between <adview> tag and internal
+ * browser plugin.
+ */
 var AD_VIEW_ATTRIBUTES = [
   'name',
 ];
 
-// List of custom attributes (and their behavior)
-//
-// name: attribute name.
-// onInit(adview): callback invoked when the <adview> element is created.
-// onMutate(adview, mutation): callback invoked when attribute is mutated.
+/**
+ * List of custom attributes (and their behavior).
+ *
+ * name: attribute name.
+ * onMutation(adview, mutation): callback invoked when attribute is mutated.
+ * isProperty: True if the attribute should be exposed as a property.
+ */
 var AD_VIEW_CUSTOM_ATTRIBUTES = [
   {
-    'name': "ad-network",
-    'onInit': function(adview) {
-      if (adview.node_.hasAttribute(this.name)) {
-        var value = adview.node_.getAttribute(this.name);
-        var item = getAdNetworkInfo(value);
-        if (item) {
-          adview.objectNode_.setAttribute("src", item.url);
-        }
-        else if (allowCustomAdNetworks()) {
-          console.log('The ad-network \"' + value + '\" is not recognized, ' +
-            'but custom ad-networks are enabled.');
-        }
-        else {
-          console.error('The ad-network \"' + value + '\" is not recognized.');
-        }
-      }
+    name: 'ad-network',

[benwells, 2013/04/02 00:49:45]

Is there any reason to lump the refactor and the tests into the same CL? They
feel orthogonal. This refactor seems non-trivial.

[rpaquay, 2013/04/02 16:44:47]

Most of the refactor was initiated as a result of adding new tests. The code was
becoming too complex to handle all the corner cases, so a refactor was needed to
manage complexity.

+    onMutation: function(adview, mutation) {
+      adview.handleAdNetworkMutation(mutation);
+    },
+    isProperty: function() {
+      return true;
     }
   },
   {
-    'name': "src",
-    'onInit': function(adview) {
-      if (allowCustomAdNetworks()) {
-        if (adview.node_.hasAttribute(this.name)) {
-          var newValue = adview.node_.getAttribute(this.name);
-          adview.objectNode_.setAttribute("src", newValue);
-        }
-      }
+    name: 'src',
+    onMutation: function(adview, mutation) {
+      adview.handleSrcMutation(mutation);
     },
-    'onMutation': function(adview, mutation) {
-      if (allowCustomAdNetworks()) {
-        if (adview.node_.hasAttribute(this.name)) {
-          var newValue = adview.node_.getAttribute(this.name);
-          // Note: setAttribute does not work as intended here.
-          //adview.objectNode_.setAttribute(this.name, newValue);
-          adview.objectNode_[this.name] = newValue;
-        }
-        else {
-          // If an attribute is removed from the BrowserPlugin, then remove it
-          // from the <adview> as well.
-          this.objectNode_.removeAttribute(this.name);
-        }
-      }
+    isProperty: function() {
+      return allowCustomAdNetworks();
     }
   }
 ];
 
-// List of api methods. These are forwarded to the browser plugin.
+/**
+ * List of api methods. These are forwarded to the browser plugin.
+ */
 var AD_VIEW_API_METHODS = [
  // Empty for now.
 ];
 
-// List of events to blindly forward from the browser plugin to the <adview>.
+/**
+ * List of events to blindly forward from the browser plugin to the <adview>.
+ */
 var AD_VIEW_EVENTS = {
-  'loadcommit' : [],
+  'loadabort' : ['url', 'isTopLevel', 'reason'],
+  'loadcommit' : ['url', 'isTopLevel'],
   'sizechanged': ['oldHeight', 'oldWidth', 'newHeight', 'newWidth'],
 };
 
-// List of supported ad-networks.
-//
-// name: identifier of the ad-network, corresponding to a valid value
-//       of the "ad-network" attribute of an <adview> element.
-// url: url to navigate to when initially displaying the <adview>.
-// origin: origin of urls the <adview> is allowed navigate to.
+/**
+ * List of supported ad-networks.
+ *
+ * name: identifier of the ad-network, corresponding to a valid value
+ *       of the "ad-network" attribute of an <adview> element.
+ * url: url to navigate to when initially displaying the <adview>.
+ * origin: origin of urls the <adview> is allowed navigate to.
+ */
 var AD_VIEW_AD_NETWORKS_WHITELIST = [
   {
-    'name': 'admob',
-    'url': 'https://admob-sdk.doubleclick.net/chromeapps',
-    'origin': 'https://double.net'
+    name: 'admob',
+    url: 'https://admob-sdk.doubleclick.net/chromeapps',
+    origin: 'https://double.net'
   },
 ];
 
-//
-// Return the whitelisted ad-network entry named |name|.
-//
+/**
+ * Return the whitelisted ad-network entry named |name|.
+ */
 function getAdNetworkInfo(name) {
   var result = null;
   forEach(AD_VIEW_AD_NETWORKS_WHITELIST, function(i, item) {
     if (item.name === name)
       result = item;
   });
   return result;
 }
 
 /**
  * @constructor
  */
-function AdView(node) {
-  this.node_ = node;
-  var shadowRoot = node.webkitCreateShadowRoot();
+function AdView(adviewNode) {
+  this.adviewNode_ = adviewNode;
+  this.browserPluginNode_ = this.createBrowserPluginNode_();
+  var shadowRoot = this.adviewNode_.webkitCreateShadowRoot();
+  shadowRoot.appendChild(this.browserPluginNode_);
 
-  this.objectNode_ = document.createElement('object');
-  this.objectNode_.type = 'application/browser-plugin';
+  this.setupCustomAttributes_();
+  this.setupAdviewNodeObservers_();
+  this.setupAdviewNodeMethods_();
+  this.setupAdviewNodeProperties_();
+  this.setupAdviewNodeEvents_();
+  this.setupBrowserPluginNodeObservers_();
+}
+
+/**
+ * @private
+ */
+AdView.prototype.createBrowserPluginNode_ = function() {
+  var browserPluginNode = document.createElement('object');
+  browserPluginNode.type = 'application/browser-plugin';
   // The <object> node fills in the <adview> container.
-  this.objectNode_.style.width = '100%';
-  this.objectNode_.style.height = '100%';
+  browserPluginNode.style.width = '100%';
+  browserPluginNode.style.height = '100%';
   forEach(AD_VIEW_ATTRIBUTES, function(i, attributeName) {
     // Only copy attributes that have been assigned values, rather than copying
     // a series of undefined attributes to BrowserPlugin.
-    if (this.node_.hasAttribute(attributeName)) {
-      this.objectNode_.setAttribute(
-        attributeName, this.node_.getAttribute(attributeName));
+    if (this.adviewNode_.hasAttribute(attributeName)) {
+      browserPluginNode.setAttribute(
+        attributeName, this.adviewNode_.getAttribute(attributeName));
     }
   }, this);
 
+  return browserPluginNode;
+}
+
+/**
+ * @private
+ */
+AdView.prototype.setupCustomAttributes_ = function() {
   forEach(AD_VIEW_CUSTOM_ATTRIBUTES, function(i, attributeInfo) {
-    if (attributeInfo.onInit) {
-      attributeInfo.onInit(this);
+    if (attributeInfo.onMutation) {
+      attributeInfo.onMutation(this);
     }
   }, this);
+}
 
-  shadowRoot.appendChild(this.objectNode_);
-
-  // this.objectNode_[apiMethod] are not necessarily defined immediately after
-  // the shadow object is appended to the shadow root.
+/**
+ * @private
+ */
+AdView.prototype.setupAdviewNodeMethods_ = function() {
+  // this.browserPluginNode_[apiMethod] are not necessarily defined immediately
+  // after the shadow object is appended to the shadow root.
   var self = this;
   forEach(AD_VIEW_API_METHODS, function(i, apiMethod) {
-    node[apiMethod] = function(var_args) {
-      return self.objectNode_[apiMethod].apply(self.objectNode_, arguments);
+    self.adviewNode_[apiMethod] = function(var_args) {
+      return self.browserPluginNode_[apiMethod].apply(
+        self.browserPluginNode_, arguments);
     };
   }, this);
+}
 
+/**
+ * @private
+ */
+AdView.prototype.setupAdviewNodeObservers_ = function() {
   // Map attribute modifications on the <adview> tag to property changes in
   // the underlying <object> node.
   var handleMutation = function(i, mutation) {
-    this.handleMutation_(mutation);
+    this.handleAdviewAttributeMutation_(mutation);
   }.bind(this);
   var observer = new WebKitMutationObserver(function(mutations) {
     forEach(mutations, handleMutation);
   });
   observer.observe(
-      this.node_,
+      this.adviewNode_,
       {attributes: true, attributeFilter: AD_VIEW_ATTRIBUTES});
 
-  var handleObjectMutation = function(i, mutation) {
-    this.handleObjectMutation_(mutation);
-  }.bind(this);
-  var objectObserver = new WebKitMutationObserver(function(mutations) {
-    forEach(mutations, handleObjectMutation);
-  });
-  objectObserver.observe(
-      this.objectNode_,
-      {attributes: true, attributeFilter: AD_VIEW_ATTRIBUTES});
+  this.setupAdviewNodeCustomObservers_();
+}
 
-  // Map custom attribute modifications on the <adview> tag to property changes
-  // in the underlying <object> node.
-  var handleCustomMutation = function(i, mutation) {
-    this.handleCustomMutation_(mutation);
+/**
+ * @private
+ */
+AdView.prototype.setupAdviewNodeCustomObservers_ = function() {
+  var handleMutation = function(i, mutation) {
+    this.handleAdviewCustomAttributeMutation_(mutation);
   }.bind(this);
   var observer = new WebKitMutationObserver(function(mutations) {
-    forEach(mutations, handleCustomMutation);
+    forEach(mutations, handleMutation);
   });
   var customAttributeNames =
     AD_VIEW_CUSTOM_ATTRIBUTES.map(function(item) { return item.name; });
   observer.observe(
-      this.node_,
+      this.adviewNode_,
       {attributes: true, attributeFilter: customAttributeNames});
+}
 
-  var objectNode = this.objectNode_;
+/**
+ * @private
+ */
+AdView.prototype.setupBrowserPluginNodeObservers_ = function() {
+  var handleMutation = function(i, mutation) {
+    this.handleBrowserPluginAttributeMutation_(mutation);
+  }.bind(this);
+  var objectObserver = new WebKitMutationObserver(function(mutations) {
+    forEach(mutations, handleMutation);
+  });
+  objectObserver.observe(
+      this.browserPluginNode_,
+      {attributes: true, attributeFilter: AD_VIEW_ATTRIBUTES});
+}
+
+/**
+ * @private
+ */
+AdView.prototype.setupAdviewNodeProperties_ = function() {
+  var browserPluginNode = this.browserPluginNode_;
   // Expose getters and setters for the attributes.
   forEach(AD_VIEW_ATTRIBUTES, function(i, attributeName) {
-    Object.defineProperty(this.node_, attributeName, {
+    Object.defineProperty(this.adviewNode_, attributeName, {
       get: function() {
-        return objectNode[attributeName];
+        return browserPluginNode[attributeName];
       },
       set: function(value) {
-        objectNode[attributeName] = value;
+        browserPluginNode[attributeName] = value;
       },
       enumerable: true
     });
   }, this);
 
+  // Expose getters and setters for the custom attributes.
+  var adviewNode = this.adviewNode_;
+  forEach(AD_VIEW_CUSTOM_ATTRIBUTES, function(i, attributeInfo) {
+    if (attributeInfo.isProperty()) {
+      var attributeName = attributeInfo.name;
+      Object.defineProperty(this.adviewNode_, attributeName, {
+        get: function() {
+          return adviewNode.getAttribute(attributeName);
+        },
+        set: function(value) {
+          adviewNode.setAttribute(attributeName, value);
+        },
+        enumerable: true
+      });
+    }
+  }, this);
+
+  this.setupAdviewContentWindowProperty_();
+}
+
+/**
+ * @private
+ */
+AdView.prototype.setupAdviewContentWindowProperty_ = function() {
+  var browserPluginNode = this.browserPluginNode_;
   // We cannot use {writable: true} property descriptor because we want dynamic
   // getter value.
-  Object.defineProperty(this.node_, 'contentWindow', {
+  Object.defineProperty(this.adviewNode_, 'contentWindow', {
     get: function() {
       // TODO(fsamuel): This is a workaround to enable
       // contentWindow.postMessage until http://crbug.com/152006 is fixed.
-      if (objectNode.contentWindow)
-        return objectNode.contentWindow.self;
+      if (browserPluginNode.contentWindow)
+        return browserPluginNode.contentWindow.self;
       console.error('contentWindow is not available at this time. ' +
           'It will become available when the page has finished loading.');
     },
     // No setter.
     enumerable: true
   });
-
-  for (var eventName in AD_VIEW_EVENTS) {
-    this.setupEvent_(eventName, AD_VIEW_EVENTS[eventName]);
-  }
 }
 
 /**
  * @private
  */
-AdView.prototype.handleMutation_ = function(mutation) {
+AdView.prototype.handleAdviewAttributeMutation_ = function(mutation) {
   // This observer monitors mutations to attributes of the <adview> and
   // updates the BrowserPlugin properties accordingly. In turn, updating
   // a BrowserPlugin property will update the corresponding BrowserPlugin
   // attribute, if necessary. See BrowserPlugin::UpdateDOMAttribute for more
   // details.
-  this.objectNode_[mutation.attributeName] =
-      this.node_.getAttribute(mutation.attributeName);
+  this.browserPluginNode_[mutation.attributeName] =
+      this.adviewNode_.getAttribute(mutation.attributeName);
 };
 
 /**
  * @private
  */
-AdView.prototype.handleCustomMutation_ = function(mutation) {
-  // This observer monitors mutations to attributes of the <adview> and
-  // updates the BrowserPlugin properties accordingly. In turn, updating
-  // a BrowserPlugin property will update the corresponding BrowserPlugin
-  // attribute, if necessary. See BrowserPlugin::UpdateDOMAttribute for more
-  // details.
+AdView.prototype.handleAdviewCustomAttributeMutation_ = function(mutation) {
   forEach(AD_VIEW_CUSTOM_ATTRIBUTES, function(i, item) {
     if (mutation.attributeName.toUpperCase() == item.name.toUpperCase()) {
       if (item.onMutation) {
         item.onMutation.bind(item)(this, mutation);
       }
     }
   }, this);
 };
 
 /**
  * @private
  */
-AdView.prototype.handleObjectMutation_ = function(mutation) {
+AdView.prototype.handleBrowserPluginAttributeMutation_ = function(mutation) {
   // This observer monitors mutations to attributes of the BrowserPlugin and
   // updates the <adview> attributes accordingly.
-  if (!this.objectNode_.hasAttribute(mutation.attributeName)) {
+  if (!this.browserPluginNode_.hasAttribute(mutation.attributeName)) {
     // If an attribute is removed from the BrowserPlugin, then remove it
     // from the <adview> as well.
-    this.node_.removeAttribute(mutation.attributeName);
+    this.adviewNode_.removeAttribute(mutation.attributeName);
   } else {
     // Update the <adview> attribute to match the BrowserPlugin attribute.
     // Note: Calling setAttribute on <adview> will trigger its mutation
     // observer which will then propagate that attribute to BrowserPlugin. In
     // cases where we permit assigning a BrowserPlugin attribute the same value
     // again (such as navigation when crashed), this could end up in an infinite
     // loop. Thus, we avoid this loop by only updating the <adview> attribute
     // if the BrowserPlugin attributes differs from it.
-    var oldValue = this.node_.getAttribute(mutation.attributeName);
-    var newValue = this.objectNode_.getAttribute(mutation.attributeName);
+    var oldValue = this.adviewNode_.getAttribute(mutation.attributeName);
+    var newValue = this.browserPluginNode_.getAttribute(mutation.attributeName);
     if (newValue != oldValue) {
-      this.node_.setAttribute(mutation.attributeName, newValue);
+      this.adviewNode_.setAttribute(mutation.attributeName, newValue);
     }
   }
 };
 
 /**
  * @private
  */
+AdView.prototype.navigateToUrl_ = function(url) {
+  var newValue = url;
+  var oldValue = this.browserPluginNode_.hasAttribute('src') ?
+    this.browserPluginNode_.getAttribute('src') :
+    null;
+
+  if (newValue === oldValue)
+    return;
+
+  if (url != null) {
+    // Note: setAttribute does not work as intended here.
+    //this.browserPluginNode_.setAttribute('src', url);
+    this.browserPluginNode_['src'] = url;
+    if (allowCustomAdNetworks()) {
+      this.adviewNode_.setAttribute('src', url);
+    }
+  }
+  else {
+    this.browserPluginNode_.removeAttribute('src');
+    if (allowCustomAdNetworks()) {
+      this.adviewNode_.removeAttribute('src');
+    }
+  }
+}
+
+/**
+ * @public
+ */
+AdView.prototype.handleAdNetworkMutation = function(mutation) {
+  if (this.adviewNode_.hasAttribute('ad-network')) {
+    var value = this.adviewNode_.getAttribute('ad-network');
+    var item = getAdNetworkInfo(value);
+    if (item) {
+      this.navigateToUrl_(item.url);
+    }
+    else {
+      if (allowCustomAdNetworks()) {
+        console.log('The ad-network "' + value + '" is not recognized, ' +
+          'but custom ad-networks are enabled.');
+        var newValue = value;
+        var oldValue = (mutation ? mutation.oldValue : undefined);
+        if (newValue !== oldValue) {
+          this.navigateToUrl_('');
+        }
+      }
+      else {
+        // Ignore the new attribute value and set it to empty string.
+        // Avoid infinite loop by checking for empty string as new value.
+        if (value != '') {
+          console.error('The ad-network "' + value + '" is not recognized.');
+          this.adviewNode_.setAttribute('ad-network', '');
+        }
+        this.navigateToUrl_('');
+      }
+    }
+  }
+  else {
+    this.navigateToUrl_('');
+  }
+}
+
+/**
+ * @public
+ */
+AdView.prototype.handleSrcMutation = function(mutation) {
+  if (allowCustomAdNetworks()) {
+    if (this.adviewNode_.hasAttribute('src')) {
+      var newValue = this.adviewNode_.getAttribute('src');
+      // Note: setAttribute does not work as intended here.
+      //this.browserPluginNode_.setAttribute('src', newValue);
+      this.browserPluginNode_['src'] = newValue;
+    }
+    else {
+      // If an attribute is removed from the <adview>, then remove it
+      // from the BrowserPlugin as well.
+      this.browserPluginNode_.removeAttribute('src');
+    }
+  }
+  else {
+    if (this.adviewNode_.hasAttribute('src')) {
+      var value = this.adviewNode_.getAttribute('src');
+      // Ignore the new attribute value and set it to empty string.
+      // Avoid infinite loop by checking for empty string as new value.
+      if (value != '') {
+        console.error('Setting the "src" attribute of an <adview> ' +
+          'element is not supported.  Use the "ad-network" attribute ' +
+          'instead.');
+        this.adviewNode_.setAttribute('src', '');
+      }
+    }
+  }
+}
+
+/**
+ * @private
+ */
+AdView.prototype.setupAdviewNodeEvents_ = function() {
+  for (var eventName in AD_VIEW_EVENTS) {
+    this.setupEvent_(eventName, AD_VIEW_EVENTS[eventName]);
+  }
+}
+
+/**
+ * @private
+ */
 AdView.prototype.setupEvent_ = function(eventname, attribs) {
-  var node = this.node_;
-  this.objectNode_.addEventListener('-internal-' + eventname, function(e) {
+  var adviewNode = this.adviewNode_;
+  var internalname = '-internal-' + eventname;
+  this.browserPluginNode_.addEventListener(internalname, function(e) {
     var evt = new Event(eventname, { bubbles: true });
     var detail = e.detail ? JSON.parse(e.detail) : {};
     forEach(attribs, function(i, attribName) {
       evt[attribName] = detail[attribName];
     });
-    node.dispatchEvent(evt);
+    adviewNode.dispatchEvent(evt);
   });
 }
 
+/**
+ * @public
+ */
+AdView.prototype.dispatchEvent = function(eventname, detail) {
+  // Create event object.
+  var evt = new Event(eventname, { bubbles: true });
+  for(var item in detail) {
+      evt[item] = detail[item];
+  }
+
+  // Dispatch event.
+  this.adviewNode_.dispatchEvent(evt);
+}
+
+
 //
 // Hook up <adview> tag creation in DOM.
 //
-var watchForTag = require("tagWatcher").watchForTag;
-
 window.addEventListener('DOMContentLoaded', function() {
   watchForTag('ADVIEW', function(addedNode) { new AdView(addedNode); });
 });