Try to pull in changes from google:master

.babelrc

@@ -9,6 +9,7 @@
     "es6.blockScoping",
     "es6.classes",
     "es6.constants",
+    "es6.destructuring",
     "es6.tailCall",
     "es6.modules",
     "es6.parameters",

.eslintrc

@@ -1,12 +1,14 @@
 {
   "parser": "babel-eslint",
+  "rulePaths": ["build-system/eslint-rules/"],
+  "plugins": ["eslint-plugin-google-camelcase"],
   "ecmaFeatures": {
     "modules": true,
     "arrowFunctions": true,
     "blockBindings": true,
     "forOf": false,
-    "destructuring": false,
-    "spread": false
+    "destructuring": true,
+    "spread": true
   },
   "env": {
     "es6": true,
@@ -25,7 +27,9 @@
     "assert": false,
     "sinon": true,
     "sandbox": true,
-    "context": false
+    "context": false,
+    "global": false,
+    "describes": true
   },
   "rules": {
     "array-bracket-spacing": [2, "never"],
@@ -35,6 +39,7 @@
     "computed-property-spacing": [2, "never"],
     "curly": 2,
     "dot-location": [2, "property"],
+    "enforce-private-props": 2,
     "eol-last": 2,
     "google-camelcase/google-camelcase": 2,
     "indent": [2, 2, { "SwitchCase": 1 }],
@@ -45,11 +50,16 @@
       "ignorePattern": ""
     }],
     "no-alert": 2,
+    "no-array-destructuring": 2,
     "no-debugger": 2,
     "no-div-regex": 2,
+    "no-dupe-keys": 2,
+    "no-es2015-number-props": 2,
     "no-eval": 2,
+    "no-export-side-effect": 2,
     "no-extend-native": 2,
     "no-extra-bind": 2,
+    "no-for-of-statement": 2,
     "no-implicit-coercion": [2, { "boolean": false }],
     "no-implied-eval": 2,
     "no-iterator": 2,
@@ -61,6 +71,7 @@
     "no-self-compare": 2,
     "no-sequences": 2,
     "no-spaced-func": 2,
+    "no-spread": 2,
     "no-throw-literal": 2,
     "no-trailing-spaces": 2,
     "no-unused-expressions": 0,

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,17 @@
+Please pick a meaningful title for your pull request using sentence case.
+
+Do not overuse punctuation in the title like `(chore):`. If it is helpful feel free to start with a project name, though, like `ProjectX: Implement some feature`.
+
+# Title instructions above.
+
+Enter a succinct description of what is achieved by the PR. Ideally describe why the change is being made.
+
+Bullet points like
+
+- Implements aspect X
+- Leaves out feature Y because of A
+- Improves performance by B
+
+really help with making this more readable.
+
+Fixes/Implements/Related-to #1 (enter issue number, except in rare cases where none exists).

.gitignore

@@ -6,11 +6,14 @@ c
 dist
 dist.3p
 dist.tools
-examples.build
 examples.min
 node_modules
 npm-debug.log
 .idea
 .tm_properties
 .settings
+.vscode
+typings
+typings.json
 build-system/runner/TESTS-TestSuites.xml
+/test/manual/amp-ad.adtech.html

.travis.yml

@@ -18,30 +18,14 @@ addons:
     - protobuf-compiler
     - python-protobuf
 before_install:
-  - export CHROME_BIN=chromium-browser
+  - export CHROME_BIN=google-chrome
   - export DISPLAY=:99.0
   - sh -e /etc/init.d/xvfb start
+  - wget https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb
+  - sudo dpkg -i google-chrome*.deb
 before_script:
-  - npm install -g gulp
   - pip install --user protobuf
-script:
-  - 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
-  # Integration tests with all saucelabs browsers
-  - gulp test --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.
-  # - gulp test --saucelabs --oldchrome
-  - gulp validator
+script: node build-system/pr-check.js
 branches:
   only:
     - master
@@ -51,3 +35,8 @@ branches:
 env:
   global:
     - NPM_CONFIG_PROGRESS="false"
+
+cache:
+  yarn: true
+  directories:
+  - node_modules

3p/3p.js

@@ -24,6 +24,7 @@
 
 import {dev, user} from '../src/log';
 import {isArray} from '../src/types';
+import {map} from '../src/utils/object';
 import {rethrowAsync} from '../src/log';
 
 
@@ -35,7 +36,7 @@ let ThirdPartyFunctionDef;
  * @const {!Object<ThirdPartyFunctionDef>}
  * @visibleForTesting
  */
-export const registrations = {};
+export const registrations = map();
 
 /** @type {number} */
 let syncScriptLoads = 0;
@@ -45,7 +46,7 @@ let syncScriptLoads = 0;
  * @param {ThirdPartyFunctionDef} draw Function that draws the 3p integration.
  */
 export function register(id, draw) {
-  dev.assert(!registrations[id], 'Double registration %s', id);
+  dev().assert(!registrations[id], 'Double registration %s', id);
   registrations[id] = draw;
 }
 
@@ -57,7 +58,7 @@ export function register(id, draw) {
  */
 export function run(id, win, data) {
   const fn = registrations[id];
-  user.assert(fn, 'Unknown 3p: ' + id);
+  user().assert(fn, 'Unknown 3p: ' + id);
   fn(win, data);
 }
 
@@ -83,12 +84,19 @@ 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
+ * @param {function()=} opt_errorCb
  */
-export function loadScript(win, url, cb) {
+export function loadScript(win, url, opt_cb, opt_errorCb) {
+  /** @const {!Element} */
   const s = win.document.createElement('script');
   s.src = url;
-  s.onload = cb;
+  if (opt_cb) {
+    s.onload = opt_cb;
+  }
+  if (opt_errorCb) {
+    s.onerror = opt_errorCb;
+  }
   win.document.body.appendChild(s);
 }
 
@@ -151,23 +159,6 @@ export function validateSrcContains(string, src) {
   }
 }
 
-/**
- * Throws a non-interrupting exception if data contains a field not supported
- * by this embed type.
- * @param {!Object} data
- * @param {!Array<string>} allowedFields
- */
-export function checkData(data, allowedFields) {
-  // Throw in a timeout, because we do not want to interrupt execution,
-  // because that would make each removal an instant backward incompatible
-  // change.
-  try {
-    validateData(data, allowedFields);
-  } catch (e) {
-    rethrowAsync(e);
-  }
-}
-
 /**
  * Utility function to perform a potentially asynchronous task
  * exactly once for all frames of a given type and the provide the respective
@@ -209,15 +200,34 @@ export function computeInMasterFrame(global, taskId, work, cb) {
 }
 
 /**
- * Throws an exception if data does not contains a mandatory field.
+ * Validates given data. Throws an exception if the data does not
+ * contains a mandatory field. If called with the optional param
+ * opt_optionalFields, it also validates that the data contains no fields other
+ * than mandatory and optional fields.
+ *
+ * Mandatory fields also accept a string Array as an item. All items in that
+ * array are considered as alternatives to each other. So the validation checks
+ * that the data contains exactly one of those alternatives.
+ *
  * @param {!Object} data
- * @param {!Array<string>} mandatoryFields
+ * @param {!Array<string|!Array<string>>} mandatoryFields
+ * @param {Array<string>=} opt_optionalFields
  */
-export function validateDataExists(data, mandatoryFields) {
+export function validateData(data, mandatoryFields, opt_optionalFields) {
+  let allowedFields = opt_optionalFields || [];
   for (let i = 0; i < mandatoryFields.length; i++) {
     const field = mandatoryFields[i];
-    user.assert(data[field],
-        'Missing attribute for %s: %s.', data.type, field);
+    if (Array.isArray(field)) {
+      validateExactlyOne(data, field);
+      allowedFields = allowedFields.concat(field);
+    } else {
+      user().assert(data[field],
+          'Missing attribute for %s: %s.', data.type, field);
+      allowedFields.push(field);
+    }
+  }
+  if (opt_optionalFields) {
+    validateAllowedFields(data, allowedFields);
   }
 }
 
@@ -227,7 +237,7 @@ export function validateDataExists(data, mandatoryFields) {
  * @param {!Object} data
  * @param {!Array<string>} alternativeFields
  */
-export function validateExactlyOne(data, alternativeFields) {
+function validateExactlyOne(data, alternativeFields) {
   let countFileds = 0;
 
   for (let i = 0; i < alternativeFields.length; i++) {
@@ -237,19 +247,19 @@ export function validateExactlyOne(data, alternativeFields) {
     }
   }
 
-  user.assert(countFileds === 1,
+  user().assert(countFileds === 1,
       '%s must contain exactly one of attributes: %s.',
       data.type,
       alternativeFields.join(', '));
 }
 
 /**
- * Throws an exception if data contains a field not supported
+ * Throws a non-interrupting exception if data contains a field not supported
  * by this embed type.
  * @param {!Object} data
  * @param {!Array<string>} allowedFields
  */
-export function validateData(data, allowedFields) {
+function validateAllowedFields(data, allowedFields) {
   const defaultAvailableFields = {
     width: true,
     height: true,
@@ -260,13 +270,38 @@ export function validateData(data, allowedFields) {
     location: true,
     mode: true,
     consentNotificationId: true,
+    ampSlotIndex: true,
   };
+
   for (const field in data) {
-    if (!data.hasOwnProperty(field) ||
-        field in defaultAvailableFields) {
+    if (!data.hasOwnProperty(field) || field in defaultAvailableFields) {
       continue;
     }
-    user.assert(allowedFields.indexOf(field) != -1,
-        'Unknown attribute for %s: %s.', data.type, field);
+    if (allowedFields.indexOf(field) < 0) {
+      // Throw in a timeout, because we do not want to interrupt execution,
+      // because that would make each removal an instant backward incompatible
+      // change.
+      rethrowAsync(new Error(`Unknown attribute for ${data.type}: ${field}.`));
+    }
   }
 }
+
+/** @private {!Object<string, boolean>} */
+let experimentToggles = {};
+
+/**
+ * Returns true if an experiment is enabled.
+ * @param {string} experimentId
+ * @return {boolean}
+ */
+export function isExperimentOn(experimentId) {
+  return !!experimentToggles[experimentId];
+}
+
+/**
+ * Set experiment toggles.
+ * @param {!Object<string, boolean>} toggles
+ */
+export function setExperimentToggles(toggles) {
+  experimentToggles = toggles;
+}

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.
@@ -60,4 +59,4 @@ Review the [ads/README](../ads/README.md) for further details on ad integration.
 - JavaScript can not be involved with the initiation of font loading.
 - Font loading gets controlled (but not initiated) by [`<amp-font>`](https://github.com/ampproject/amphtml/issues/648).
 - AMP by default does not allow inclusion of external stylesheets, but it is happy to whitelist URL prefixes of font providers for font inclusion via link tags. These link tags and their fonts must be served via HTTPS.
-- If a font provider does referrer based "security" it needs to whitelist the AMP proxy origins before being included in the link tag whitelist. AMP proxy sends the appropriate referrer header such as "https://cdn.ampproject.org".
+- If a font provider does referrer based "security" it needs to whitelist the AMP proxy origins before being included in the link tag whitelist. AMP proxy sends the appropriate referrer header such as "https://cdn.ampproject.org" and "https://amp.cloudflare.com".