Amp analytics 3p vendors #9661
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,225 @@ | ||
| /** | ||
| * Copyright 2017 The AMP HTML Authors. All Rights Reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS-IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| import './polyfills'; | ||
| import {tryParseJson} from '../src/json'; | ||
| import {dev, initLogConstructor, setReportError} from '../src/log'; | ||
| import {IframeMessagingClient} from './iframe-messaging-client'; | ||
| import {AMP_ANALYTICS_3P_MESSAGE_TYPE} from '../src/3p-analytics-common'; | ||
|
|
||
| initLogConstructor(); | ||
| // TODO(alanorozco): Refactor src/error.reportError so it does not contain big | ||
| // transitive dependencies and can be included here. | ||
| setReportError(() => {}); | ||
|
|
||
| /** @private @const {string} */ | ||
| const TAG_ = 'ampanalytics-lib'; | ||
|
|
||
| /** | ||
| * Receives messages bound for this cross-domain iframe, from all creatives | ||
| */ | ||
| class AmpAnalytics3pMessageRouter { | ||
|
|
||
| /** @param {!Window} win */ | ||
| constructor(win) { | ||
| /** @private {!Window} */ | ||
| this.win_ = win; | ||
|
|
||
| // The sentinel is required by iframe-messaging-client, and is used to | ||
| // uniquely identify the frame as part of message routing | ||
| /** @const {string} */ | ||
| this.sentinel_ = dev().assertString( | ||
| tryParseJson(this.win_.name, {}).sentinel, | ||
| 'Invalid/missing sentinel on iframe name attribute' + this.win_.name); | ||
| if (!this.sentinel_) { | ||
| return; | ||
| } | ||
|
|
||
| /** | ||
| * Multiple creatives on a page may wish to use the same type of | ||
| * amp-analytics tag. This object provides a mapping between the | ||
| * IDs which identify which amp-analytics tag a message is to/from, | ||
| * with each ID's corresponding AmpAnalytics3pCreativeMessageRouter, | ||
| * which is an object that handles messages to/from a particular creative. | ||
| * @private {!Object<string, !AmpAnalytics3pCreativeMessageRouter>} | ||
| */ | ||
| this.creativeMessageRouters_ = {}; | ||
|
|
||
| /** | ||
| * Handles communication between frames | ||
| * @private {!IframeMessagingClient} | ||
| */ | ||
| this.iframeMessagingClient_ = new IframeMessagingClient(win); | ||
| this.iframeMessagingClient_.setSentinel(this.sentinel_); | ||
| this.iframeMessagingClient_.registerCallback( | ||
| AMP_ANALYTICS_3P_MESSAGE_TYPE.CREATIVE, | ||
| messageContainer => { | ||
| let entries; | ||
| dev().assert(messageContainer.data && | ||
| (entries = Object.entries(messageContainer.data)).length, | ||
| 'Received empty new creative message'); | ||
| entries.forEach(entry => { | ||
| const creativeId = entry[0]; | ||
| const extraData = entry[1]; | ||
| dev().assert(!this.creativeMessageRouters_[creativeId], | ||
| 'Duplicate new creative message for ' + creativeId); | ||
| this.creativeMessageRouters_[creativeId] = | ||
| new AmpAnalytics3pCreativeMessageRouter( | ||
| this.win_, this.iframeMessagingClient_, creativeId, extraData); | ||
| }); | ||
| }); | ||
| this.iframeMessagingClient_.registerCallback( | ||
| AMP_ANALYTICS_3P_MESSAGE_TYPE.EVENT, | ||
| messageContainer => { | ||
| let entries; | ||
| dev().assert(messageContainer.data && | ||
| (entries = Object.entries(messageContainer.data)).length, | ||
| 'Received empty events message'); | ||
| entries.forEach(entry => { | ||
| const creativeId = entry[0]; | ||
| const messages = entry[1]; | ||
| try { | ||
| dev().assert(messages && messages.length, | ||
| 'Received empty events list for' + creativeId); | ||
| dev().assert(this.creativeMessageRouters_[creativeId], | ||
| 'Discarding event message received prior to new creative' + | ||
| ' message for' + creativeId); | ||
| this.creativeMessageRouters_[creativeId] | ||
| .sendMessagesToListener(messages); | ||
| } catch (e) { | ||
| dev().error(TAG_, 'Failed to send message to listener: ' + | ||
|
There was a problem hiding this comment. Using the term "send" is a bit misleading here as one may expect it to mean you postMessaged. Perhaps "Failed to execute listener"...? There was a problem hiding this comment. Re: your above comment which GitHub UI won't let me comment on, I forgot to "git add" the file to test the new Queue classes. Will add that together with addressing your comments here. There was a problem hiding this comment. And re: the comment I'm actually responding to here, done. Changed to "Failed to pass message to event listener" |
||
| e.message); | ||
| } | ||
| }); | ||
| }); | ||
| this.iframeMessagingClient_.sendMessage( | ||
| AMP_ANALYTICS_3P_MESSAGE_TYPE.READY); | ||
|
There was a problem hiding this comment. Can this fit on one line? There was a problem hiding this comment. 81 characters, unless I rename stuff. |
||
| } | ||
| } | ||
|
|
||
| new AmpAnalytics3pMessageRouter(window); | ||
|
|
||
| /** | ||
| * Receives messages bound for this cross-domain iframe, from a particular | ||
| * creative. Also sends response messages from the iframe meant for this | ||
| * particular creative. | ||
| */ | ||
| class AmpAnalytics3pCreativeMessageRouter { | ||
| /** | ||
| * @param {!Window} win The enclosing window object | ||
| * @param {!IframeMessagingClient} iframeMessagingClient Facilitates | ||
| * communication with the frame | ||
| * @param {!string} creativeId The ID of the creative to route messages | ||
| * to/from | ||
| * @param {string=} opt_extraData Extra data to be passed to the frame | ||
| */ | ||
| constructor(win, iframeMessagingClient, creativeId, opt_extraData) { | ||
| /** @private {!Window} */ | ||
| this.win_ = win; | ||
|
|
||
| /** @private {!IframeMessagingClient} */ | ||
| this.iframeMessagingClient_ = iframeMessagingClient; | ||
|
|
||
| /** @private {!string} */ | ||
| this.creativeId_ = creativeId; | ||
|
|
||
| /** @private {?string} */ | ||
| this.extraData_ = opt_extraData; | ||
|
|
||
| /** @private {?function(!Array<!AmpAnalytics3pEvent>)} */ | ||
| this.eventListener_ = null; | ||
|
|
||
| if (this.win_.onNewAmpAnalyticsInstance) { | ||
| try { | ||
| this.win_.onNewAmpAnalyticsInstance(this); | ||
| } catch (e) { | ||
| dev().error(TAG_, 'Exception thrown by onNewAmpAnalyticsInstance in' + | ||
|
There was a problem hiding this comment. Consider moving onNewAmpAnalyticsInstance check prior to creating AmpAnalytics3pCreativeMessageRouter as AmpAnalytics3pMessageRouter essentially expects it to work properly. In other words, it would call router's sendMessagesToListener function even though client is not actually listening |
||
| this.win_.location.href + ': ' + e.message); | ||
| } | ||
| } else { | ||
| dev().error(TAG_, 'Must implement onNewAmpAnalyticsInstance in' + | ||
| this.win_.location.href); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Registers a callback function to be called when AMP Analytics events occur. | ||
| * There may only be one listener. If another function has previously been | ||
| * registered as a listener, it will no longer receive events. | ||
| * @param {!function(!Array<AmpAnalytics3pEvent>)} listener A function | ||
| * that takes an array of event strings, and does something with them. | ||
| */ | ||
| registerAmpAnalytics3pEventsListener(listener) { | ||
| if (this.eventListener_) { | ||
| dev().warn(TAG_, 'Replacing existing eventListener for ' + | ||
| this.creativeId_); | ||
| } | ||
| this.eventListener_ = listener; | ||
| } | ||
|
|
||
| /** | ||
| * Receives message(s) from a creative for the cross-domain iframe | ||
| * and passes them to that iframe's listener, if a listener has been | ||
| * registered | ||
| * @param {!Array<AmpAnalytics3pEvent>} messages The message that was received | ||
| */ | ||
| sendMessagesToListener(messages) { | ||
| if (!messages.length || !this.eventListener_) { | ||
|
There was a problem hiding this comment. Consider adding a dev().warn indicating messages are being lost and registerAmpAnalytics3pEventsListener should be called. We could also try and recover from previous invalid/missing onNewAmpAnalyticsInstance here There was a problem hiding this comment. Given the fix to your previous comment, it is no longer possible to reach here without implementing onNewAmpAnalyticsInstance, as the enclosing AmpAnalytics3pMessageRouter will now not have been created. Unfortunately we can't go and create that for them, so I just made the dev().warn message explicitly say "Attempted to send messages when no listener configured. Be sure to first call registerAmpAnalytics3pEventsListener()" |
||
| return; | ||
| } | ||
| try { | ||
| this.eventListener_(messages); | ||
| } catch (e) { | ||
| dev().error(TAG_, 'Caught exception executing listener for ' + | ||
| this.creativeId_ + ': ' + e.message); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Gets any optional extra data that should be made available to the | ||
| * cross-domain frame, in the context of a particular creative. | ||
| * @returns {?string} | ||
| */ | ||
| getExtraData() { | ||
| return this.extraData_; | ||
| } | ||
|
|
||
| /** | ||
| * Sends a message from the third-party vendor's metrics-collection page back | ||
| * to the creative. | ||
| * @param {!Object} response The response to send. | ||
| */ | ||
| sendMessageToCreative(response) { | ||
| this.iframeMessagingClient_.sendMessage( | ||
| AMP_ANALYTICS_3P_MESSAGE_TYPE.RESPONSE, | ||
| this.buildAmpAnalytics3pResponse_(response)); | ||
| } | ||
|
|
||
| /** | ||
| * Builds an instance of AmpAnalytics3pResponse | ||
| * @param {!Object} response The response from the iframe, which can be | ||
| * any object | ||
| * @returns {AmpAnalytics3pResponse} | ||
| */ | ||
| buildAmpAnalytics3pResponse_(response) { | ||
| const messageObject = /** @type {AmpAnalytics3pResponse} */ ({ | ||
| destination: this.creativeId_, | ||
| data: response, | ||
| }); | ||
| return messageObject; | ||
| } | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <title>Requests Frame</title> | ||
| <script> | ||
| /** | ||
| * To receive analytics events in a third-party frame, you must | ||
| * implement this method, with this signature. Within that method, you | ||
| * must create a function which processes the analytics events, and | ||
| * pass that function to the registerAmpAnalytics3pEventsListener() method | ||
| * of the object which is passed as a parameter to | ||
| * onNewAmpAnalyticsInstance(). | ||
| * @param ampAnalytics Call registerAmpAnalyticsEventListener() on this, | ||
| * passing your function which will receive the analytics events. | ||
| */ | ||
| window.onNewAmpAnalyticsInstance = ampAnalytics => { | ||
| ampAnalytics.registerAmpAnalytics3pEventsListener(events => { | ||
| events.forEach(event => { | ||
| // Now, do something meaningful with the AMP Analytics event | ||
| console.log("Received an event: " + event + | ||
| ", and my extra data is: " + | ||
| ampAnalytics.getExtraData()); | ||
| ampAnalytics.sendMessageToCreative( | ||
| {'status': 'received', 'somethingElse': '42'}); | ||
| }); | ||
| }); | ||
| }; | ||
|
|
||
| // Load the script specified in the iframe’s name attribute: | ||
| script = document.createElement('script'); | ||
| script.src = JSON.parse(window.name).scriptSrc; | ||
|
There was a problem hiding this comment. I fear this opens an XSS exploit for 3p analytic vendors. They shouldn't just trust the script src is to a valid AMP url. Instead we should either provide only the path to the file and hard code the domain or the vendor should have code verifying the domain. See how remote HTML loads the context script: https://github.com/ampproject/amphtml/blob/master/3p/remote.html#L9 There was a problem hiding this comment. This sounds like a good idea. I just ran it and forced the non-local path, and that gave me: So, sure, I could definitely just send everything after "https://3p.ampproject.net", and hard-code that part on the client side. There was a problem hiding this comment. Hmm...though we'll also have to know on that side whether we're local, since I won't want to prepend the "https://3p.ampproject.net" in that case. |
||
| document.head.appendChild(script); | ||
| // The script will be loaded, and will call onNewAmpAnalyticsInstance() | ||
| </script> | ||
| </head> | ||
| <!-- The frame will not be visible, so there is no need for a body tag. --> | ||
| </html> | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| <!doctype html> | ||
| <html ⚡> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>3P AMP Analytics Example</title> | ||
| <link rel="canonical" href="amps.html" > | ||
| <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1"> | ||
| <link href='https://fonts.googleapis.com/css?family=Georgia|Open+Sans|Roboto' rel='stylesheet' type='text/css'> | ||
| <script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js"></script> | ||
| <script async custom-element="amp-ad" src="https://cdn.ampproject.org/v0/amp-ad-0.1.js"></script> | ||
| <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> | ||
| </head> | ||
| <body> | ||
| Here is some text above the ad.<br/> | ||
| <amp-ad width="300" height="400" | ||
| type="fake" | ||
| src="fake_amp_ad_with_3p_analytics.json" | ||
| data-use-a4a="true" fakesig="true"> | ||
| <div placeholder>Loading...</div> | ||
| <div fallback>Could not display the fake ad :(</div> | ||
| </amp-ad> | ||
| <br/> | ||
| Here is some text below the ad.<br/> | ||
| </body> | ||
| </html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "creative": "<!doctype html><html amp4ads><head><meta charset=utf-8><script async src=https://cdn.ampproject.org/v0.js></script><script async custom-element=amp-analytics src=https://cdn.ampproject.org/v0/amp-analytics-0.1.js></script><style amp4ads-boilerplate>body{visibility:hidden}</style><meta name=viewport content=width=device-width,minimum-scale=1></head><body><a href=\"http://localhost/landing.html?testStatus=3PANALYTICS(test,status)&testOther=3PANALYTICS(test,somethingElse)&test2Status=3PANALYTICS(test2,status)&test2Other=3PANALYTICS(test2,somethingElse)\"><amp-img height=300 id=img src=https://upload.wikimedia.org/wikipedia/commons/6/6e/Golde33443.jpg width=256></amp-img> <p class=frob>By Golden Trvs Gol twister (Own work) [CC BY-SA 4.0 (http://creativecommons.org/licenses/by-sa/4.0)], via Wikimedia Commons</p></a><amp-analytics type=\"test\"><script type=application/json>{\"requests\":{\"pageview\":\"viewed=true&url=${canonicalUrl}&title=${title}&acct=${account}\",\"event\":\"eid=click&acct=${account}\"},\"vars\":{\"account\":\"ABC123\"},\"triggers\":{\"defaultPageview\":{\"on\":\"visible\",\"request\":\"pageview\"},\"anchorClicks\":{\"on\":\"click\",\"selector\":\"img\",\"request\":\"event\"}},\"transport\":{\"iframe\":\"http://localhost:8000/examples/analytics-3p-remote-frame.html\",\"extraData\":\"ThisIsExtraData\"}}</script></amp-analytics><script type=\"application/json\" amp-ad-metadata>{\"ampRuntimeUtf16CharOffsets\":[55,222],\"bodyAttributes\":\"\",\"bodyUtf16CharOffsets\":[356,2436],\"customElementExtensions\":[\"amp-analytics\"],\"jsonUtf16CharOffsets\":{\"amp-analytics\":[661,2410]}}</script></body></html>", | ||
| "signature": "ACWMjZE6z6W9JyytdwVJ6FpbUxDxRcyRbYVT9mv/eYZbaUHP4x9nVHPA9B298mIZeqr73aWipv4aldRrb6SWS7SBi9rgKRvB2F6ppS5mXsEQCkZaBxuBgPKrz4hl2F3mElpV60ScN7xX2kt5tGJp+E9sZCU6K9w9j7XKJRhQ6Hc5TZnPvrWeUiT0Pth0O/KREKIBbR3EocM/5TYEHlQ38e2dtEIiyY6B1BgPLBz80uKASIDNS/IgJoaFWqR7VkZkztq7sjrhgKEGPscJiqVSviN9r9beAJnsBzFio4Onu+FMwY0+kkdKvZ7fhiuMvuSmGqcyrwqwLlGSc0dqYiqx7EGhTDKM" | ||
| } | ||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to accelerate this PR, I suggest you split off the code into a 2nd PR. I can see clear boundary between the 2:
PR1: runtime side code
PR2: adding a library for 3p to use.
As long as there're clear API defined, you can easily separate the 2