Allow invalid AMP creatives to get position info

[zombifier]

Allows unregistered iframes (and their children) to receive position info from the inabox host script.

Some AMP creatives will fail validation, meaning it will not be registered with the host script and thus unable to determine its visibility or resize itself. The former functionality should be unrestricted to any frames that request it, valid or not.

[torch2424]

cc @lannka Do you have context on this?

[jeffkaufman]

@torch2424 Some context:

• When an AMP creative fails validation it won't be allowed to talk to the host script, which means if its in a cross-domain iframe on Safari (no IntersectionObserver) it can't determine its visibility. This means it can't determine if it has met the IAB standard for viewability (50% on screen for a second).

• Creatives can fail validation for many reasons, including things like "the creative had 19k of CSS, and then the ad network added 5k of CSS, and so it's over the 20k limit". Of course the creative could also have custom JS or just not be AMP at all.

• While some host script features do need to be restricted to validated AMP creatives (resizing) this isn't the case for position information, so it's ok to open the host script up to supply this information to any creative that requests it.

An alternative implementation of this would be for the ad tag to add all creatives, AMP or not, to the list of frames, and use the new amp-allowed whitelist (PR #18698) to restrict them to position information only. I think the change here is simpler, but could be convinced otherwise.

@zombifier could you include some of this background in your PR description?

[jeffkaufman]

I'm not convinced this works: after running getFrameElement_ above either:

• getFrameElement_ returned null, !iframe was true, we returned false above
• !this.iframeMap_[request['sentinel']].iframe will be false and this code block will never run

Have you tested this locally?

[zombifier]

getFrameElement_ does not return the iframe, it returns the top most measurable frame (measurableFrame), so the two conditions are not mutually exclusive. I had tested this locally and it works on my end.

[jeffkaufman]

I'm still not seeing it, but maybe I'm misreading?

Case 1:

• getFrameElement_ returns null
• iframeMap_[sentinel] is unset
• processMessage will log Ignored message from unmeasurable iframe and return false

Case 2:

• getFrameElement_ returns non-null
• iframeMap_[sentinel] will be set, as will iframeMap_[sentinel].iframe
• !this.iframeMap_[request['sentinel']].iframe will be false
• the if (request['type'] != MessageType.SEND_POSITIONS) {...} block won't ever run

Are you getting Ignored message from untrusted iframe in local testing?

[zombifier]

So I refactored getFrameElement_ so that if the message source is not (a child of any frames) within the registered frames list, its entry with the sentinel would still be logged in the map. In this case iframeMap_[sentinel].iframe will be null (since we don't know, correct me if I'm wrong), but not iframeMap_[sentinel].measurableFrame, which is needed for getting positioning.

I hope this makes sense. Please ask more questions if you are unclear.

[lannka]

Not sure if it's the same thing but I remember we were discussing adding amp-allow attribute to inform host.js what APIs are granted to the ad.

Is that the same thing?

@zombifier @jeffkaufman since you've already written up a description, do you mind moving it to an issue I2I, and have this PR link to it. Just for streamlined tracking and providing more background to the community

[zombifier]

So concerning @jeffkaufman's comment, after a bit of thinking and discussions I believe that the stated alternative of loosening the ad tag's restrictions so all frames are registered with the host script and instead relying on the whitelist for functionality is the better way to do this. As such, I'm closing this PR for now.