Merge branch 'master' into canary

Author: erwinmombay

.travis.yml

@@ -27,6 +27,7 @@ script:
   - gulp test
   # Integration tests with all saucelabs browsers
   - gulp test --saucelabs --integration
+  - gulp test --saucelabs --integration --compiled
   # All unit tests with an old chrome (best we can do right now to pass tests
   # and not start relying on new features).
   - gulp test --saucelabs --oldchrome

3p/README.md

@@ -11,7 +11,7 @@ We highly prefer integrations that do not use iframes. JSONP cannot be used for
 Examples: Youtube, Vimeo videos; Tweets, Instagrams; comment systems; polls; quizzes; document viewers
 
 - Our intent is to provide first class support for all embeds that fulfill the notability guidelines laid out in [CONTRIBUTING.md](../CONTRIBUTING.md).
-- Consider whether a click-to-play solution fits your use case where iframe generation is not done immediately (can be done before user action for instant loading of iframe).
+- Consider whether a iframe-with-placeholder solution fits your use case where iframe generation is not done immediately (can be done before user action for instant loading of iframe).
 - Consider whether all that is needed is some documentation for how to use the embed with `amp-iframe`.
 - Iframes and all sub resources must be served from HTTPS.
 - Avoid client side rendering of iframe content.

build-system/config.js

@@ -61,7 +61,10 @@ var karma = {
     configFile: karmaConf,
     singleRun: true,
     client: {
-      captureConsole: false
+      captureConsole: false,
+      amp: {
+        useCompiledJs: false
+      }
     }
   },
   firefox: {

build-system/tasks/presubmit-checks.js

@@ -133,6 +133,7 @@ var forbiddenTerms = {
   'setCookie\\W': {
     message: requiresReviewPrivacy,
     whitelist: [
+      'src/service/cid-impl.js',
       'src/cookies.js',
       'src/experiments.js',
       'tools/experiments/experiments.js',

build-system/tasks/test.js

@@ -84,6 +84,13 @@ gulp.task('test', 'Runs tests in chrome', ['build'], function(done) {
     c.files = config.testPaths;
   }
 
+  if (argv.compiled) {
+    if (!argv.integration) {
+      throw new Error('Compiled tests are only supported for integration tests');
+    }
+    c.client.amp.useCompiledJs = true;
+  }
+
   karma.start(c, done);
 }, {
   options: {
@@ -93,6 +100,8 @@ gulp.task('test', 'Runs tests in chrome', ['build'], function(done) {
     'safari': '  Runs tests in Safari',
     'firefox': '  Runs tests in Firefox',
     'integration': 'Run only integration tests.',
+    'compiled': 'Changes integration tests to use production JS ' +
+        'binaries for execution',
     'oldchrome': 'Runs test with an old chrome. Saucelabs only.',
   }
 });

docs/include_features.md

@@ -14,7 +14,7 @@ Display an iframe in your page using the
 
 `amp-iframe` requirements:
 
-* Must be at least 600px or 75% of the first viewport away from the top (except for `click-to-play` iframes as described below).
+* Must be at least 600px or 75% of the first viewport away from the top (except for iframes implemented with a placeholder, as described below).
 * Can only request resources via HTTPS, and they must not be in the same origin as the container,
 unless they do not specify allow-same-origin.
 

extensions/amp-access/0.1/amp-access.js

@@ -239,7 +239,8 @@ export class AccessService {
       // No consent - an essential part of the access system.
       const consent = Promise.resolve();
       this.readerIdPromise_ = this.cid_.then(cid => {
-        return cid.get('amp-access', consent);
+        return cid.get({scope: 'amp-access', createCookieIfNotPresent: true},
+            consent);
       });
     }
     return this.readerIdPromise_;

extensions/amp-access/0.1/test/test-amp-access.js

@@ -260,30 +260,17 @@ describe('AccessService authorization', () => {
     sandbox = null;
   });
 
-  it('should run authorization flow', () => {
+  function expectGetReaderId(result) {
     cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
-    xhrMock.expects('fetchJson')
-        .withExactArgs('https://acme.com/a?rid=reader1',
-            {credentials: 'include'})
-        .returns(Promise.resolve({access: true}))
+        .withExactArgs(
+            {scope: 'amp-access', createCookieIfNotPresent: true},
+            sinon.match(() => true))
+        .returns(Promise.resolve(result))
         .once();
-    const promise = service.runAuthorization_();
-    expect(document.documentElement).to.have.class('amp-access-loading');
-    return promise.then(() => {
-      expect(document.documentElement).not.to.have.class('amp-access-loading');
-      expect(elementOn).not.to.have.attribute('amp-access-hide');
-      expect(elementOff).to.have.attribute('amp-access-hide');
-    });
-  });
+  }
 
   it('should run authorization flow', () => {
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     xhrMock.expects('fetchJson')
         .withExactArgs('https://acme.com/a?rid=reader1',
             {credentials: 'include'})
@@ -299,10 +286,7 @@ describe('AccessService authorization', () => {
   });
 
   it('should recover from authorization failure', () => {
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     xhrMock.expects('fetchJson')
         .withExactArgs('https://acme.com/a?rid=reader1',
             {credentials: 'include'})
@@ -318,10 +302,7 @@ describe('AccessService authorization', () => {
   });
 
   it('should resolve first-authorization promise after success', () => {
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     xhrMock.expects('fetchJson')
         .withExactArgs('https://acme.com/a?rid=reader1',
             {credentials: 'include'})
@@ -334,10 +315,7 @@ describe('AccessService authorization', () => {
   });
 
   it('should NOT resolve first-authorization promise after failure', () => {
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     xhrMock.expects('fetchJson')
         .withExactArgs('https://acme.com/a?rid=reader1',
             {credentials: 'include'})
@@ -553,6 +531,15 @@ describe('AccessService pingback', () => {
     sandbox = null;
   });
 
+  function expectGetReaderId(result) {
+    cidMock.expects('get')
+        .withExactArgs(
+            {scope: 'amp-access', createCookieIfNotPresent: true},
+            sinon.match(() => true))
+        .returns(Promise.resolve(result))
+        .once();
+  }
+
   it('should register "viewed" signal after timeout', () => {
     service.reportViewToServer_ = sandbox.spy();
     const p = service.reportWhenViewed_();
@@ -685,10 +672,7 @@ describe('AccessService pingback', () => {
   });
 
   it('should send POST pingback', () => {
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     xhrMock.expects('sendSignal')
         .withExactArgs('https://acme.com/p?rid=reader1', sinon.match(init => {
           return (init.method == 'POST' &&
@@ -754,24 +738,27 @@ describe('AccessService login', () => {
     sandbox = null;
   });
 
+  function expectGetReaderId(result) {
+    cidMock.expects('get')
+        .withExactArgs(
+            {scope: 'amp-access', createCookieIfNotPresent: true},
+            sinon.match(() => true))
+        .returns(Promise.resolve(result))
+        .once();
+  }
+
   it('should open dialog in the same microtask', () => {
     service.openLoginDialog_ = sandbox.stub();
     service.openLoginDialog_.returns(Promise.resolve());
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     service.login();
     expect(service.openLoginDialog_.callCount).to.equal(1);
     expect(service.openLoginDialog_.firstCall.args[0].then).to.exist;
   });
 
   it('should succeed login with success=true', () => {
     service.runAuthorization_ = sandbox.spy();
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     let urlPromise = null;
     serviceMock.expects('openLoginDialog_')
         .withExactArgs(sinon.match(arg => {
@@ -792,10 +779,7 @@ describe('AccessService login', () => {
 
   it('should fail login with success=no', () => {
     service.runAuthorization_ = sandbox.spy();
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     serviceMock.expects('openLoginDialog_')
         .withExactArgs(sinon.match(arg => !!arg.then))
         .returns(Promise.resolve('#success=no'))
@@ -808,10 +792,7 @@ describe('AccessService login', () => {
 
   it('should fail login with empty response', () => {
     service.runAuthorization_ = sandbox.spy();
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     serviceMock.expects('openLoginDialog_')
         .withExactArgs(sinon.match(arg => !!arg.then))
         .returns(Promise.resolve(''))
@@ -824,10 +805,7 @@ describe('AccessService login', () => {
 
   it('should fail login with aborted dialog', () => {
     service.runAuthorization_ = sandbox.spy();
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     serviceMock.expects('openLoginDialog_')
         .withExactArgs(sinon.match(arg => !!arg.then))
         .returns(Promise.reject('abort'))
@@ -841,10 +819,7 @@ describe('AccessService login', () => {
 
   it('should run login only once at a time', () => {
     service.runAuthorization_ = sandbox.spy();
-    cidMock.expects('get')
-        .withExactArgs('amp-access', sinon.match(() => true))
-        .returns(Promise.resolve('reader1'))
-        .once();
+    expectGetReaderId('reader1');
     serviceMock.expects('openLoginDialog_')
         .withExactArgs(sinon.match(arg => !!arg.then))
         .returns(new Promise(() => {}))

extensions/amp-iframe/amp-iframe.md

@@ -90,7 +90,7 @@ Here are some factors that affect how fast the resize will be executed:
 - Whether the resize is requested for a currently active IFrame;
 - Whether the resize is requested for an IFrame below the viewport or above the viewport.
 
-#### Iframe Click-To-Play
+#### Iframe with Placeholder
 It is possible to have an `amp-iframe` appear on the top of a document when the `amp-iframe` has a `placeholder` element as shown in the example below.
 
 ```html

gulpfile.js

@@ -324,9 +324,10 @@ function buildExamples(watch) {
   buildExample('user-notification.amp.html');
   buildExample('vine.amp.html');
 
-  // Examples are also copied into `c/` directory for AMP-proxy testing.
-  fs.copy('examples.build/', 'c/', {clobber: true},
-      copyHandler.bind(null, 'examples.build to c folder'));
+  // TODO(dvoytenko, #1393): Enable for proxy-testing.
+  // // Examples are also copied into `c/` directory for AMP-proxy testing.
+  // fs.copy('examples.build/', 'c/', {clobber: true},
+  //     copyHandler.bind(null, 'examples.build to c folder'));
 
   function copyHandler(name, err) {
     if (err) {