Bug 229204 – PCM: Allow measurement of links in nested, cross-site iframes

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Pranay Prabhat]

https://github.com/privacycg/private-click-measurement/issues/7 has highlighted the needs but for majority of publishers , ads are rendered in a secondary domain usually rendered by Google Ad Manager. PCM's click based anchor tags within secondary domain need to be propagated to publisher context in order to register the click for conversion reporting.

[Radar WebKit Bug Importer]

<rdar://problem/82310386>

[John Wilander]

Created attachment 440110 [details]
Patch

[Alex Christensen]

Comment on attachment 440110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440110&action=review

> Source/WebCore/html/HTMLAnchorElement.cpp:446
> +    if (frame->isMainFrame())
> +        mainDocumentRegistrableDomain = RegistrableDomain { document().url() };
> +    else if (auto mainDocument = frame->mainFrame().document())
> +        mainDocumentRegistrableDomain = RegistrableDomain { mainDocument->url() };

I think you can simplify this because Frame::mainFrame() returns itself if it's the main frame.

[John Wilander]

(In reply to Alex Christensen from comment #3)
> Comment on attachment 440110 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440110&action=review
> 
> > Source/WebCore/html/HTMLAnchorElement.cpp:446
> > +    if (frame->isMainFrame())
> > +        mainDocumentRegistrableDomain = RegistrableDomain { document().url() };
> > +    else if (auto mainDocument = frame->mainFrame().document())
> > +        mainDocumentRegistrableDomain = RegistrableDomain { mainDocument->url() };
> 
> I think you can simplify this because Frame::mainFrame() returns itself if
> it's the main frame.

Thanks! Will fix before landing.

[Chris Dumez]

Comment on attachment 440110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440110&action=review

>>> Source/WebCore/html/HTMLAnchorElement.cpp:446
>>> +        mainDocumentRegistrableDomain = RegistrableDomain { mainDocument->url() };
>> 
>> I think you can simplify this because Frame::mainFrame() returns itself if it's the main frame.
> 
> Thanks! Will fix before landing.

You don't even need frames, document().topDocument().url()

[John Wilander]

(In reply to Chris Dumez from comment #5)
> Comment on attachment 440110 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440110&action=review
> 
> >>> Source/WebCore/html/HTMLAnchorElement.cpp:446
> >>> +        mainDocumentRegistrableDomain = RegistrableDomain { mainDocument->url() };
> >> 
> >> I think you can simplify this because Frame::mainFrame() returns itself if it's the main frame.
> > 
> > Thanks! Will fix before landing.
> 
> You don't even need frames, document().topDocument().url()

I looked at the implementation of topDocument() and this part has me worried about potential misuse vectors:

if (backForwardCacheState() == NotInBackForwardCache && !m_renderTreeBeingDestroyed) {
    if (!m_frame)
        return const_cast<Document&>(*this);
    // This should always be non-null.
    Document* mainFrameDocument = m_frame->mainFrame().document();
    return mainFrameDocument ? *mainFrameDocument : const_cast<Document&>(*this);
}

I'll go with if (auto mainDocument = frame->mainFrame().document()).

[John Wilander]

Created attachment 440122 [details]
Patch

[John Wilander]

Created attachment 440215 [details]
Patch

[John Wilander]

Fixed test for iOS.

[EWS]

Committed r283593 (242549@main): <https://commits.webkit.org/242549@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440215 [details].