Merge branch 'master' into widespace-integration

* master: (236 commits)
  trim all the columns (ampproject#3894)
  Refactoring: Turn private custom element methods into functions. (ampproject#3882)
  Lower the load priority of ad shaped iframes. (ampproject#3863)
  JsDoc fix (ampproject#3892)
  Add screenshots for Opera to AMP Validator extension. (ampproject#3866)
  Fix renaming of generated JSCompiler_prototypeAlias variable. (ampproject#3887)
  fix typo in amp-sidebar.md (ampproject#3833)
  Validator Roll-up (ampproject#3885)
  [CryptoService] Leverage browser native Crypto API to hash strings. (ampproject#3850)
  Size update (ampproject#3883)
  copy amp-ad docs to builtins (ampproject#3879)
  move doc to extension (ampproject#3878)
  [amp-experiment] Exposes isDismissed() method in AmpUserNotification (ampproject#3832)
  fix action-impl warning on dist (ampproject#3867)
  Add params for microad (ampproject#3827)
  Fixed some A4A tests. (ampproject#3859)
  Updates to colanalytics vendor config for amp-analytics. (ampproject#3849)
  Changes to implement A4A (AMP ads for AMP pages) (ampproject#3534)
  Addresses comment left over from PR#3841 (ampproject#3853)
  Expose submit event with on=submit:el.action syntax. (ampproject#3739)
  ...

.eslintrc

@@ -12,6 +12,21 @@
  "es6": true,
  "browser": true
  },
+ "globals": {
+ "it": false,
+ "chai": false,
+ "expect": false,
+ "describe": false,
+ "beforeEach": false,
+ "afterEach": false,
+ "before": false,
+ "after": false,
+ "AMP": false,
+ "assert": false,
+ "sinon": true,
+ "sandbox": true,
+ "context": false
+ },
  "rules": {
  "array-bracket-spacing": [2, "never"],
  "arrow-parens": [2, "as-needed"],
@@ -55,12 +70,14 @@
  }],
  "no-useless-call": 2,
  "no-useless-concat": 2,
+ "no-undef": 2,
  "no-var": 2,
  "no-warning-comments": [2, { "terms": ["do not submit"], "location": "anywhere" }],
  "object-curly-spacing": [2, "never", {
  "objectsInObjects": false,
  "arraysInObjects": false
  }],
+ "object-shorthand": [2, "properties"],
  "prefer-const": 2,
  "quotes": [2, "single", "avoid-escape"],
  "radix": 2,

.gitignore

@@ -14,3 +14,5 @@ npm-debug.log
 .tm_properties
 .settings
 build-system/runner/TESTS-TestSuites.xml
+
+/test/manual/amp-ad.adtech.html

.travis.yml

@@ -25,17 +25,19 @@ before_script:
  - npm install -g gulp
  - pip install --user protobuf
 script:
+ - npm run node-test
  - gulp lint
  - gulp build --fortesting
+ - gulp check-types
  - gulp dist --fortesting
  - gulp presubmit
  # dep-check needs to occur after build since we rely on build to generate
  # the css files into js files.
  - gulp dep-check
  # Unit tests with Travis' default chromium
- - gulp test --compiled --fortesting
+ - gulp test --nobuild --compiled --fortesting
  # Integration tests with all saucelabs browsers
- - gulp test --saucelabs --integration --compiled --fortesting
+ - gulp test --nobuild --saucelabs --integration --compiled --fortesting
  # All unit tests with an old chrome (best we can do right now to pass tests
  # and not start relying on new features).
  # Disabled because it regressed. Better to run the other saucelabs tests.

3p/3p.js

@@ -51,7 +51,7 @@ export function register(id, draw) {
 
 /**
  * Execute the 3p integration with the given id.
- * @param {id} id
+ * @param {string} id
  * @param {!Window} win
  * @param {!Object} data
  */
@@ -83,12 +83,14 @@ export function writeScript(win, url, opt_cb) {
  * Asynchronously load the given script URL.
  * @param {!Window} win
  * @param {string} url
- * @param {function()=} cb
+ * @param {function()=} opt_cb
  */
-export function loadScript(win, url, cb) {
+export function loadScript(win, url, opt_cb) {
  const s = win.document.createElement('script');
  s.src = url;
- s.onload = cb;
+ if (opt_cb) {
+ s.onload = opt_cb;
+ }
  win.document.body.appendChild(s);
 }
 
@@ -97,7 +99,7 @@ export function loadScript(win, url, cb) {
  * This is a lightweight helper, because we cannot guarantee that
  * Promises are available inside the 3p frame.
  * @param {!Window} win
- * @param {function} fn
+ * @param {function()} fn
  */
 export function nextTick(win, fn) {
  const P = win.Promise;

3p/README.md

@@ -19,7 +19,6 @@ Examples: Youtube, Vimeo videos; Tweets, Instagrams; comment systems; polls; qui
 - If you can make it not-iframe-based that is much better. (See e.g. the pinterest embed). We will always ask to do this first. E.g. adding a CORS endpoint to your server might make this possible.
 - Must play well within [AMP's sizing framework](https://github.com/ampproject/amphtml/blob/master/spec/amp-html-layout.md).
 - All JS on container page must be open source and bundled with AMP.
-- Direct iframe embeds not using our 3p iframe mechanism (used e.g. for ads) are preferred.
 - JavaScript loaded into iframe should be reasonable with respect to functionality.
 - Use the `sandbox` attribute on iframe if possible.
 - Provide unit and integration tests.
@@ -30,7 +29,7 @@ Examples: Youtube, Vimeo videos; Tweets, Instagrams; comment systems; polls; qui
 - We welcome pull requests by all ad networks for inclusion into AMP.
 - All ads and all sub resources must be served from HTTPS.
 - Must play well within [AMP's sizing framework](https://github.com/ampproject/amphtml/blob/master/spec/amp-html-layout.md).
-- Direct iframe embeds not using our 3p iframe mechanism (used by most ads) are preferred.
+- For display ads support, always implement amp-ad and instruct your client to use your amp-ad implementation instead of using amp-iframe. Althought amp-iframe will render the ad, ad clicks will break and viewability information is not available.
 - Providing an optional image only zero-iframe embed is appreciated.
 - Support viewability and other metrics/instrumentation as supplied by AMP (via postMessage API)
 - Try to keep overall iframe count at one per ad. Explain why more are needed.

3p/environment.js

@@ -104,7 +104,7 @@ function instrumentSrcdoc(parent, iframe) {
 /**
  * Instrument added nodes if they are instrumentable iframes.
  * @param {!Window} win
- * @param {!Array<!Node>} addedNodes
+ * @param {!Array<!Node>|NodeList<!Node>|NodeList<!Element>|null} addedNodes
  */
 function maybeInstrumentsNodes(win, addedNodes) {
  for (let n = 0; n < addedNodes.length; n++) {
@@ -281,8 +281,8 @@ export function becomeVisible() {
 
 /**
  * Calculates the minimum time that a timeout should have right now.
- * @param {number} time
- * @return {number}
+ * @param {number|undefined} time
+ * @return {number|undefined}
  */
 function minTime(time) {
  if (!inViewport) {

3p/integration.js

@@ -74,10 +74,14 @@ import {sharethrough} from '../ads/sharethrough';
 import {eplanning} from '../ads/eplanning';
 import {microad} from '../ads/microad';
 import {yahoojp} from '../ads/yahoojp';
+import {chargeads} from '../ads/chargeads';
+import {nend} from '../ads/nend';
+import {adgeneration} from '../ads/adgeneration';
+import {genieessp} from '../ads/genieessp';
 
 /**
  * Whether the embed type may be used with amp-embed tag.
- * @const {!Object<string: boolean>}
+ * @const {!Object<string, boolean>}
  */
 const AMP_EMBED_ALLOWED = {
  taboola: true,
@@ -134,6 +138,10 @@ register('sharethrough', sharethrough);
 register('eplanning', eplanning);
 register('microad', microad);
 register('yahoojp', yahoojp);
+register('chargeads', chargeads);
+register('nend', nend);
+register('adgeneration', adgeneration);
+register('genieessp', genieessp);
 
 // For backward compat, we always allow these types without the iframe
 // opting in.
@@ -264,8 +272,10 @@ window.draw3p = function(opt_configCallback, opt_allowed3pTypes,
  updateVisibilityState(window);
  nonSensitiveDataPostMessage('render-start');
  } catch (e) {
- lightweightErrorReport(e);
- throw e;
+ if (!window.context.mode.test) {
+ lightweightErrorReport(e);
+ throw e;
+ }
  }
 };
 
@@ -274,17 +284,11 @@ function triggerNoContentAvailable() {
 }
 
 function triggerDimensions(width, height) {
- nonSensitiveDataPostMessage('embed-size', {
- width: width,
- height: height,
- });
+ nonSensitiveDataPostMessage('embed-size', {width, height});
 }
 
 function triggerResizeRequest(width, height) {
- nonSensitiveDataPostMessage('embed-size', {
- width: width,
- height: height,
- });
+ nonSensitiveDataPostMessage('embed-size', {width, height});
 }
 
 /**
@@ -294,7 +298,7 @@ function triggerResizeRequest(width, height) {
  * the IntersectionObserver spec callback.
  * http://rawgit.com/slightlyoff/IntersectionObserver/master/index.html#callbackdef-intersectionobservercallback
  * @param {function(!Array<IntersectionObserverEntry>)} observerCallback
- * @returns {!function} A function which removes the event listener that
+ * @returns {!function()} A function which removes the event listener that
  * observes for intersection messages.
  */
 function observeIntersection(observerCallback) {
@@ -324,8 +328,8 @@ function updateVisibilityState(global) {
 
 /**
  * Registers a callback for communicating when a resize request succeeds.
- * @param {function(number)} observerCallback
- * @returns {!function} A function which removes the event listener that
+ * @param {function(number, number)} observerCallback
+ * @returns {!function()} A function which removes the event listener that
  * observes for resize status messages.
  */
 function onResizeSuccess(observerCallback) {
@@ -336,8 +340,8 @@ function onResizeSuccess(observerCallback) {
 
 /**
  * Registers a callback for communicating when a resize request is denied.
- * @param {function(number)} observerCallback
- * @returns {!function} A function which removes the event listener that
+ * @param {function(number, number)} observerCallback
+ * @returns {!function()} A function which removes the event listener that
  * observes for resize status messages.
  */
 function onResizeDenied(observerCallback) {
@@ -462,7 +466,7 @@ export function ensureFramed(window) {
 /**
  * Expects the fragment to contain JSON.
  * @param {string} fragment Value of location.fragment
- * @return {!JSONObject}
+ * @return {!JSONType}
  * @visibleForTesting
  */
 export function parseFragment(fragment) {
@@ -473,7 +477,7 @@ export function parseFragment(fragment) {
  if (json.indexOf('{%22') == 0) {
  json = decodeURIComponent(json);
  }
- return json ? JSON.parse(json) : {};
+ return /** @type {!JSONType} */ (json ? JSON.parse(json) : {});
 }
 
 /**

3p/messaging.js

@@ -25,7 +25,7 @@ export function nonSensitiveDataPostMessage(type, opt_object) {
  }
  const object = opt_object || {};
  object.type = type;
- object.sentinel = 'amp-$internalRuntimeToken$';
+ object.sentinel = window.context.amp3pSentinel;
  window.parent./*OK*/postMessage(object,
  window.context.location.origin);
 }
@@ -45,7 +45,7 @@ const listeners = [];
  */
 export function listenParent(win, type, callback) {
  const listener = {
- type: type,
+ type,
  cb: callback,
  };
  listeners.push(listener);
@@ -77,8 +77,9 @@ function startListening(win) {
  return;
  }
  // Parse JSON only once per message.
- const data = JSON.parse(event.data.substr(4));
- if (data.sentinel != 'amp-$internalRuntimeToken$') {
+ const data = /** @type {!Object} */ (
+ JSON.parse(event.data.substr(4)));
+ if (data.sentinel != win.context.amp3pSentinel) {
  return;
  }
  // Don't let other message handlers interpret our events.

3p/twitter.js

@@ -59,43 +59,20 @@ export function twitter(global, data) {
  // Dimensions are given by the parent frame.
  delete data.width;
  delete data.height;
- twttr.widgets.createTweet(data.tweetid, tweet, data)./*OK*/then(() => {
- const iframe = global.document.querySelector('#c iframe');
- // There is no iframe if the tweet was deleted. Thanks for resolving
- // the promise, though :)
- if (iframe && iframe.contentWindow) {
- // Unfortunately the tweet isn't really done at this time.
- // We listen for resize to learn when things are
- // really done.
- iframe.contentWindow.addEventListener('resize', function() {
- resize(iframe.contentWindow.document.body);
- }, true);
- resize(iframe.contentWindow.document.body);
- } else if (global.MutationObserver) {
- // If no iframe is created, Twitter likely went into their ShadowDOM
- // based pass. MutationObserver should definitely be available then.
- // Because Twitter does not actually provide an event for when
- // rendering is done, we use a MutationObserver and schedule a measure
- // operation whenever any elements were added to the DOM.
- let timeout;
- function measure() {
- // Throttle if multiple things happen at once.
- clearTimeout(timeout);
- timeout = setTimeout(function() {
- const root = global.document.querySelector('#tweet *');
- if (root) {
- resize(root);
- }
- }, 0);
- }
- const observer = new MutationObserver(measure);
- observer.observe(global.document.querySelector('#c'), {
- subtree: true,
- childList: true,
- });
- measure();
- } else {
- console./*OK*/error('No iframe or shadow root found for tweet.');
+
+ let twitterWidgetSandbox;
+ twttr.events.bind('resize', event => {
+ // To be safe, make sure the resize event was triggered for the widget we created below.
+ if (twitterWidgetSandbox === event.target) {
+ resize(twitterWidgetSandbox);
+ }
+ });
+
+ twttr.widgets.createTweet(data.tweetid, tweet, data)./*OK*/then(el => {
+ if (el) {
+ // Not a deleted tweet
+ twitterWidgetSandbox = el;
+ resize(twitterWidgetSandbox);
  }
  });
  });

DEVELOPING.md

@@ -57,13 +57,15 @@ If you have any questions, feel free to ask on the issue or join us on [Slack](h
 | `gulp watch` | Watches for changes in files, re-build. |
 | `gulp test` | Runs tests in Chrome. |
 | `gulp test --verbose` | Runs tests in Chrome with logging enabled. |
+| `gulp test --nobuild` | Runs tests without re-build. |
 | `gulp test --watch` | Watches for changes in files, runs corresponding test(s) in Chrome. |
 | `gulp test --watch --verbose` | Same as "watch" with logging enabled. |
 | `gulp test --saucelabs` | Runs test on saucelabs (requires [setup](#saucelabs)). |
 | `gulp test --safari` | Runs tests in Safari. |
 | `gulp test --firefox` | Runs tests in Firefox. |
 | `gulp test --files=<test-files-path-glob>` | Runs specific test files. |
 | `gulp serve` | Serves content in repo root dir over http://localhost:8000/. Examples live in http://localhost:8000/examples.build/ |
+| `npm run node-test` | Run node tests for tasks and offline/node code using [ava](https://github.com/avajs/ava). |
 
 
 #### Saucelabs
@@ -79,7 +81,7 @@ Also for local testing, download [saucelabs connect](https://docs.saucelabs.com/
 
 If your pull request contains JS or CSS changes and it does not change the build system, it will be automatically built and tested on [Travis](https://travis-ci.org/ampproject/amphtml/builds). After the travis run completes, the result will be logged to your PR.
 
-If a test flaked on a pull request you can ask a project owner to restart the tests for you.
+If a test flaked on a pull request you can ask a project owner to restart the tests for you. Use [`this.retries(x)`](https://mochajs.org/#retry-tests) as the last resort.
 
 ### Manual testing
 

TICKEVENTS.md

@@ -25,7 +25,7 @@ As an example if we executed `perf.tick('label')` we assume we have a counterpar
 | Name | id | Description |
 ----------------------|-------------------|------------------------------------|
 | Install Styles | `is` | Set when the styles are installed. |
-| Window load event | `ol` | Window load even fired.  |
+| Window load event | `ol` | Window load event fired. |
 | Prerender Complete | `pc` | The runtime completes prerending a single document. |
 | Frames per second | `fps` | Tick to measure fps. |
 | Frames per second during ad load | `fal`| Tick to measure fps when at least one ad is on the page. |

ads/README.md

@@ -3,7 +3,7 @@
 See also our [ad integration guidelines](../3p/README.md#ads) and [3rd party ads integration guidelines](./integration-guide.md)
 
 ## Overview
-Ads are just another external resource and must play within the same constraints placed on all resources in AMP. We aim to support a large subset of existing ads with little or no changes to how the integrations work. Our long term goal is to further improve the impact of ads on the user experience through changes across the entire vertical client side stack.
+Ads are just another external resource and must play within the same constraints placed on all resources in AMP. We aim to support a large subset of existing ads with little or no changes to how the integrations work. Our long term goal is to further improve the impact of ads on the user experience through changes across the entire vertical client side stack. Although technically feasible, do not use amp-iframe to render display ads. Using amp-iframe for display ads breaks ad clicks and prevents recording viewability information.
 
 ## Constraints
 A summary of constraints placed on external resources such as ads in AMP HTML: