Install CID service in core runtime instead of amp-analytics extensio…

…n. (#10074) * Install CID service in core runtime instead of amp-analytics extension. * Validator update for amp-user-notification * fix lint * fix test * fix lint * address comment * fix validator test
ampproject · Jun 29, 2017 · 8dcf893 · 8dcf893
1 parent abdb14d
commit 8dcf893
Show file tree

Hide file tree

Showing 11 changed files with 12 additions and 29 deletions.
diff --git a/examples/experiment.amp.html b/examples/experiment.amp.html
@@ -6,7 +6,6 @@
   <link rel="canonical" href="amps.html" >
   <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
   <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
-  <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js"></script>
   <script async custom-element="amp-experiment" src="https://cdn.ampproject.org/v0/amp-experiment-0.1.js"></script>
   <script async custom-element="amp-user-notification" src="https://cdn.ampproject.org/v0/amp-user-notification-0.1.js"></script>
   <script async src="https://cdn.ampproject.org/v0.js"></script>

diff --git a/examples/user-notification.amp.html b/examples/user-notification.amp.html
@@ -193,7 +193,6 @@
       animation: fadeIn ease-in 1s 1 forwards;
     }
   </style>
-  <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js"></script>
   <script async custom-element="amp-user-notification" src="https://cdn.ampproject.org/v0/amp-user-notification-0.1.js"></script>
   <script async custom-element="amp-ad" src="https://cdn.ampproject.org/v0/amp-ad-0.1.js"></script>
   <script async custom-element="amp-lightbox" src="https://cdn.ampproject.org/v0/amp-lightbox-0.1.js"></script>

diff --git a/extensions/amp-analytics/0.1/amp-analytics.js b/extensions/amp-analytics/0.1/amp-analytics.js
@@ -29,7 +29,6 @@ import {toggle} from '../../../src/style';
 import {isEnumValue} from '../../../src/types';
 import {parseJson} from '../../../src/json';
 import {Activity} from './activity-impl';
-import {Cid} from '../../../src/service/cid-impl';
 import {
     InstrumentationService,
     instrumentationServicePromiseForDoc,
@@ -47,7 +46,6 @@ import {SANDBOX_AVAILABLE_VARS} from './sandbox-vars-whitelist';
 AMP.registerServiceForDoc(
     'amp-analytics-instrumentation', InstrumentationService);
 AMP.registerServiceForDoc('activity', Activity);
-AMP.registerServiceForDoc('cid', Cid);
 
 installVariableService(AMP.win);
 

diff --git a/extensions/amp-user-notification/0.1/test/validator-amp-user-notification.html b/extensions/amp-user-notification/0.1/test/validator-amp-user-notification.html
@@ -26,7 +26,6 @@
   <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
   <script async src="https://cdn.ampproject.org/v0.js"></script>
   <script async custom-element="amp-user-notification" src="https://cdn.ampproject.org/v0/amp-user-notification-0.1.js"></script>
-  <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js"></script>
 </head>
 <body>
   <!-- Example of valid amp-user-notification -->

diff --git a/extensions/amp-user-notification/0.1/test/validator-amp-user-notification.out b/extensions/amp-user-notification/0.1/test/validator-amp-user-notification.out
@@ -1,2 +1,2 @@
 FAIL
-amp-user-notification/0.1/test/validator-amp-user-notification.html:71:2 The specified layout 'CONTAINER' is not supported by tag 'amp-user-notification'. (see https://www.ampproject.org/docs/reference/components/amp-user-notification) [AMP_LAYOUT_PROBLEM]
+amp-user-notification/0.1/test/validator-amp-user-notification.html:70:2 The specified layout 'CONTAINER' is not supported by tag 'amp-user-notification'. (see https://www.ampproject.org/docs/reference/components/amp-user-notification) [AMP_LAYOUT_PROBLEM]
diff --git a/extensions/amp-user-notification/validator-amp-user-notification.protoascii b/extensions/amp-user-notification/validator-amp-user-notification.protoascii
@@ -31,9 +31,7 @@ tags: {  # <amp-user-notification>
   tag_name: "AMP-USER-NOTIFICATION"
   disallowed_ancestor: "AMP-SIDEBAR"
   requires_extension: "amp-user-notification"
-  requires_extension: "amp-analytics"
   attr_lists: "extended-amp-global"
-  spec_url: "https://www.ampproject.org/docs/reference/components/amp-user-notification"
   amp_layout: {
     supported_layouts: NODISPLAY
   }

diff --git a/src/ad-cid.js b/src/ad-cid.js
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import {cidForDocOrNull, timerFor} from './services';
+import {cidForDoc, timerFor} from './services';
 import {adConfig} from '../ads/_config';
 import {dev} from '../src/log';
 
@@ -41,7 +41,7 @@ export function getAdCid(adElement) {
  */
 export function getOrCreateAdCid(
     ampDoc, clientIdScope, opt_clientIdCookieName) {
-  const cidPromise = cidForDocOrNull(ampDoc).then(cidService => {
+  const cidPromise = cidForDoc(ampDoc).then(cidService => {
     if (!cidService) {
       return;
     }

diff --git a/src/runtime.js b/src/runtime.js
@@ -51,6 +51,7 @@ import {
   hasRenderDelayingServices,
 } from './render-delaying-services';
 import {installActionServiceForDoc} from './service/action-impl';
+import {installCidService} from './service/cid-impl';
 import {installCryptoService} from './service/crypto-impl';
 import {installDocumentInfoServiceForDoc} from './service/document-info-impl';
 import {installGlobalClickListenerForDoc} from './service/document-click';
@@ -124,6 +125,7 @@ export function installRuntimeServices(global) {
  * @param {!Object<string, string>=} opt_initParams
  */
 export function installAmpdocServices(ampdoc, opt_initParams) {
+  installCidService(ampdoc);
   installDocumentInfoServiceForDoc(ampdoc);
   installViewerServiceForDoc(ampdoc, opt_initParams);
   installViewportServiceForDoc(ampdoc);

diff --git a/src/service/cid-impl.js b/src/service/cid-impl.js
@@ -440,6 +440,12 @@ function getEntropy(win) {
       win.Math.random() + win.screen.width + win.screen.height);
 }
 
+/**
+ * @param {!./ampdoc-impl.AmpDoc} ampdoc
+ */
+export function installCidService(ampdoc) {
+  return registerServiceBuilderForDoc(ampdoc, 'cid', Cid);
+}
 
 /**
  * @param {!./ampdoc-impl.AmpDoc} ampdoc

diff --git a/src/services.js b/src/services.js
@@ -94,18 +94,7 @@ export function bindForDoc(nodeOrDoc) {
  */
 export function cidForDoc(nodeOrDoc) {
   return /** @type {!Promise<!./service/cid-impl.Cid>} */ ( // eslint-disable-line max-len
-      getElementServiceForDoc(nodeOrDoc, 'cid', 'amp-analytics'));
-}
-
-/**
- * Returns a promise for the CID service or a promise for null if the service
- * is not available on the current page.
- * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc
- * @return {!Promise<!./service/cid-impl.Cid>}
- */
-export function cidForDocOrNull(nodeOrDoc) {
-  return /** @type {!Promise<!./service/cid-impl.Cid>} */ ( // eslint-disable-line max-len
-      getElementServiceIfAvailableForDoc(nodeOrDoc, 'cid', 'amp-analytics'));
+      getServicePromiseForDoc(nodeOrDoc, 'cid'));
 }
 
 /**

diff --git a/test/functional/test-ad-cid.js b/test/functional/test-ad-cid.js
@@ -23,7 +23,6 @@ import {installDocService} from '../../src/service/ampdoc-impl';
 import {installTimerService} from '../../src/service/timer-impl';
 import {getAdCid} from '../../src/ad-cid';
 import {timerFor} from '../../src/services';
-import {resetServiceForTesting} from '../../src/service';
 import * as lolex from 'lolex';
 
 describes.realWin('ad-cid', {}, env => {
@@ -115,10 +114,4 @@ describes.realWin('ad-cid', {}, env => {
     });
     return expect(getAdCid(adElement)).to.eventually.be.undefined;
   });
-
-  it('should return null if cid service not available', () => {
-    resetServiceForTesting(win, 'cid');
-    config.clientIdScope = cidScope;
-    return expect(getAdCid(adElement)).to.eventually.be.undefined;
-  });
 });