From 719ec114d3396b8cf2e038a973216c7ab396661e Mon Sep 17 00:00:00 2001
From: Malte Ubl <malte.ubl@gmail.com>
Date: Mon, 21 Dec 2015 12:51:18 -0800
Subject: [PATCH] Introduces an optional mechanism for advanced advertising
 techniques to manipulate incoming ad configuration in a callback function.

This is only available for publishers who host the ad iframe themselves.
With that they can obviously do whatever they want, but with this there is a larger incentive to stay within the AMP system and don't rely on implicit APIs (such as how we pass in configuration).

Fixes #1210
---
 3p/integration.js                   | 29 +++++++---
 builtins/amp-ad.md                  | 18 +++++++
 test/functional/test-integration.js | 84 ++++++++++++++++++++++++++++-
 3 files changed, 123 insertions(+), 8 deletions(-)

diff --git a/3p/integration.js b/3p/integration.js
index 37560ea25cc6..c69e98e43153 100644
--- a/3p/integration.js
+++ b/3p/integration.js
@@ -45,15 +45,27 @@ register('twitter', twitter);
 
 /**
  * Visible for testing.
- * Draws an ad to the window. Expects the data to include the ad type.
+ * Draws a 3p embed to the window. Expects the data to include the 3p type.
  * @param {!Window} win
  * @param {!Object} data
+ * @param {function(!Object, function(!Object))|undefined} configCallback
+ *     Optional callback that allows user code to manipulate the incoming
+ *     configuration. See
+ *     https://github.com/ampproject/amphtml/issues/1210 for some context
+ *     on this.
  */
-export function draw3p(win, data) {
+export function draw3p(win, data, configCallback) {
   const type = data.type;
-  assert(window.context.location.originValidated != null,
+  assert(win.context.location.originValidated != null,
       'Origin should have been validated');
-  run(type, win, data);
+  if (configCallback) {
+    configCallback(data, data => {
+      assert(data, 'Expected configuration to be passed as first argument');
+      run(type, win, data);
+    });
+  } else {
+    run(type, win, data);
+  }
 };
 
 /**
@@ -85,8 +97,13 @@ function masterSelection(type) {
 
 /**
  * Draws an embed, optionally synchronously, to the DOM.
+ * @param {function(!Object, function(!Object))} opt_configCallback If provided
+ *     will be invoked with two arguments:
+ *     1. The configuration parameters supplied to this embed.
+ *     2. A callback that MUST be called for rendering to proceed. It takes
+ *        no arguments. Configuration is expected to be modified in-place.
  */
-window.draw3p = function() {
+window.draw3p = function(opt_configCallback) {
   const data = parseFragment(location.hash);
   window.context = data._context;
   window.context.location = parseUrl(data._context.location.href);
@@ -104,7 +121,7 @@ window.draw3p = function() {
   // This only actually works for ads.
   window.context.observeIntersection = observeIntersection;
   delete data._context;
-  draw3p(window, data);
+  draw3p(window, data, opt_configCallback);
 };
 
 function triggerNoContentAvailable() {
diff --git a/builtins/amp-ad.md b/builtins/amp-ad.md
index b4f1f95777c9..a066740c3410 100644
--- a/builtins/amp-ad.md
+++ b/builtins/amp-ad.md
@@ -91,3 +91,21 @@ To enable this, copy the file [remote.html](../3p/remote.html) to your web serve
 ```
 
 The `content` attribute of the meta tag is the absolute URL to your copy of the remote.html file on your web server. This URL must use a "https" schema. It is not allowed to reside on the same origin as your AMP files. E.g. if you host AMP files on "www.example.com", this URL must not be on "www.example.com" but e.g. "something-else.example.com" is OK.
+
+##### Enhance incoming ad configuration
+
+This is completely optional: It is sometimes desired to further process the incoming iframe configuration before drawing the ad using AMP's built-in system.
+
+This is supported by passing a callback to the `draw3p` function call in the [remote.html](../3p/remote.html) file. The callback receives the incoming configuration as first argument and then receives another callback as second argument (Called `done` in the example below). This callback must be called with the updated config in order for ad rendering to proceed.
+
+Example:
+
+```JS
+draw3p(function(config, done) {
+  config.targeting = Math.random() > 0.5 ? 'sport' : 'fashion';
+  // Don't actually call setTimeout here. This should only serve as an
+  // example that is OK to call the done callback asynchronously.
+  setTimeout(function() {
+    done(config);
+  }, 100)
+});
diff --git a/test/functional/test-integration.js b/test/functional/test-integration.js
index b2c34573d2f8..fb97687c8abf 100644
--- a/test/functional/test-integration.js
+++ b/test/functional/test-integration.js
@@ -17,10 +17,16 @@
 // Tests integration.js
 // Most coverage through test-3p-frame
 
-import {validateParentOrigin, parseFragment} from '../../3p/integration';
-import {registrations} from '../../src/3p';
+import {draw3p, validateParentOrigin, parseFragment}
+    from '../../3p/integration';
+import {registrations, register} from '../../src/3p';
 
 describe('3p integration.js', () => {
+
+  afterEach(() => {
+    delete registrations.testAction;
+  });
+
   it('should register integrations', () => {
     expect(registrations).to.include.key('a9');
     expect(registrations).to.include.key('adsense');
@@ -109,4 +115,78 @@ describe('3p integration.js', () => {
     expect(parseFragment('')).to.be.empty;
     expect(parseFragment('#')).to.be.empty;
   });
+
+  it('should call the right action based on type', () => {
+    const data = {
+      type: 'testAction',
+    };
+    const win = {
+      context: {
+        location: {
+          originValidated: true
+        },
+        data: data,
+      }
+    };
+    let called = false;
+    register('testAction', function(myWin, myData) {
+      called = true;
+      expect(myWin).to.equal(win);
+      expect(myData).to.equal(myData);
+    });
+    const domNode = {};
+    expect(called).to.be.false;
+    draw3p(win, data);
+    expect(called).to.be.true;
+  });
+
+  it('should support config processing in draw3p', () => {
+    const data = {
+      type: 'testAction2',
+    };
+    const win = {
+      context: {
+        location: {
+          originValidated: true
+        },
+        data: data,
+      }
+    };
+    let called = false;
+    register('testAction2', function(myWin, myData) {
+      expect(called).to.be.false;
+      called = true;
+      expect(myWin).to.equal(win);
+      expect(myData).to.not.equal(data);
+      expect(myData).to.have.property('custom');
+    });
+    const domNode = {};
+    expect(called).to.be.false;
+    let finish;
+    draw3p(win, data, (config, done) => {
+      finish = () => {
+        done({
+          custom: true
+        });
+      };
+    });
+    expect(called).to.be.false;
+    finish();
+    expect(called).to.be.true;
+  });
+
+  it('should throw if origin was never validated', () => {
+    const data = {
+      type: 'testAction',
+    };
+    const win = {
+      context: {
+        location: {},
+        data: data,
+      }
+    };
+    expect(() => {
+      draw3p(win, data);
+    }).to.throw(/Origin should have been validated/);
+  });
 });