Changeset 269129 in webkit

Timestamp:

Oct 28, 2020 5:00:17 PM (21 months ago)

Author:

wilander@apple.com

Message:

PCM: Accept ad click data when the link opens a new window
​https://bugs.webkit.org/show_bug.cgi?id=214176
<rdar://problem/65358005>

Reviewed by Brent Fulgham.

A link with the attribute target="_blank" takes another code path for
navigation which involves the creation of a new window and webpage. That
code path needs to transfer ad click attribution data to the new webpage
Source/WebKit:

where it can be picked up in WebPageProxy::didCommitLoadForFrame().
The ad click attribution data sits in the NavigationAction which is not
available in the completion handler WebPageProxy::createNewPage().
The client, which differs between TestRunner and e.g. Safari, consumes
the NavigationAction. I don't want to risk a regression in e.g. Safari
while still passing the test because TestRunner hasn't regressed.

Test: http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window.html

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::didCommitLoadForFrame):

Now also checks for pending ad click attribution data in its new
member variable m_newPageNavigationAdClickAttribution.

(WebKit::WebPageProxy::createNewPage):

Now forwards optional ad click attribution data to the completion
handler where it can be stored on the newly created webpage in the
new member variable m_newPageNavigationAdClickAttribution.

• UIProcess/WebPageProxy.h:

Added m_newPageNavigationAdClickAttribution.

• WebProcess/WebCoreSupport/WebChromeClient.cpp:

(WebKit::WebChromeClient::createWindow):

Added missing navigationAction.adClickAttribution() copy.

• WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:

(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):

Added missing navigationAction.adClickAttribution() copy.

LayoutTests:

where it can be picked up.

• http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window-expected.txt: Added.
• http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window.html: Added.
• http/tests/adClickAttribution/resources/convertAndPostMessageBack.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window-expected.txt (added)
• LayoutTests/http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window.html (added)
• LayoutTests/http/tests/adClickAttribution/resources/convertAndPostMessageBack.html (added)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (3 diffs)
• Source/WebKit/UIProcess/WebPageProxy.h (modified) (1 diff)
• Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp (modified) (1 diff)
• Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-10-28  John Wilander  <wilander@apple.com>
+
+        PCM: Accept ad click data when the link opens a new window
+        https://bugs.webkit.org/show_bug.cgi?id=214176
+        <rdar://problem/65358005>
+
+        Reviewed by Brent Fulgham.
+
+        A link with the attribute target="_blank" takes another code path for
+        navigation which involves the creation of a new window and webpage. That
+        code path needs to transfer ad click attribution data to the new webpage
+        where it can be picked up.
+
+        * http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window-expected.txt: Added.
+        * http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window.html: Added.
+        * http/tests/adClickAttribution/resources/convertAndPostMessageBack.html: Added.
+
 2020-10-28  Truitt Savell  <tsavell@apple.com>
 

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-10-28  John Wilander  <wilander@apple.com>
+
+        PCM: Accept ad click data when the link opens a new window
+        https://bugs.webkit.org/show_bug.cgi?id=214176
+        <rdar://problem/65358005>
+
+        Reviewed by Brent Fulgham.
+
+        A link with the attribute target="_blank" takes another code path for
+        navigation which involves the creation of a new window and webpage. That
+        code path needs to transfer ad click attribution data to the new webpage
+        where it can be picked up in WebPageProxy::didCommitLoadForFrame().
+        The ad click attribution data sits in the NavigationAction which is not
+        available in the completion handler WebPageProxy::createNewPage().
+        The client, which differs between TestRunner and e.g. Safari, consumes
+        the NavigationAction. I don't want to risk a regression in e.g. Safari
+        while still passing the test because TestRunner hasn't regressed.
+
+        Test: http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window.html
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didCommitLoadForFrame):
+            Now also checks for pending ad click attribution data in its new
+            member variable m_newPageNavigationAdClickAttribution.
+        (WebKit::WebPageProxy::createNewPage):
+            Now forwards optional ad click attribution data to the completion
+            handler where it can be stored on the newly created webpage in the
+            new member variable m_newPageNavigationAdClickAttribution.
+        * UIProcess/WebPageProxy.h:
+            Added m_newPageNavigationAdClickAttribution.
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::createWindow):
+            Added missing navigationAction.adClickAttribution() copy.
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNewWindowAction):
+            Added missing navigationAction.adClickAttribution() copy.
+
 2020-10-28  Said Abou-Hallawa  <said@apple.com>
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -4652 +4652 @@
     frame->didCommitLoad(mimeType, webCertificateInfo, containsPluginDocument);
 
-    if (navigation && frame->isMainFrame()) {
-        if (auto& adClickAttribution = navigation->adClickAttribution()) {
+    if (frame->isMainFrame()) {
+        Optional<WebCore::AdClickAttribution> adClickAttribution;
+        if (m_newPageNavigationAdClickAttribution)
+            adClickAttribution = m_newPageNavigationAdClickAttribution;
+        else if (navigation && navigation->adClickAttribution())
+            adClickAttribution = navigation->adClickAttribution();
+        if (adClickAttribution) {
             if (adClickAttribution->destination().matches(frame->url()))
                 websiteDataStore().networkProcess().send(Messages::NetworkProcess::StoreAdClickAttribution(m_websiteDataStore->sessionID(), *adClickAttribution), 0);
         }
     }
+    m_newPageNavigationAdClickAttribution.reset();
 
     if (frame->isMainFrame()) {
@@ -5519 +5525 @@
     auto originatingFrameInfo = API::FrameInfo::create(WTFMove(originatingFrameInfoData), originatingPage);
     auto mainFrameURL = m_mainFrame ? m_mainFrame->url() : URL();
-    auto completionHandler = [this, protectedThis = makeRef(*this), mainFrameURL, request, reply = WTFMove(reply)] (RefPtr<WebPageProxy> newPage) mutable {
+    auto completionHandler = [this, protectedThis = makeRef(*this), mainFrameURL, request, reply = WTFMove(reply), adClickAttribution = navigationActionData.adClickAttribution] (RefPtr<WebPageProxy> newPage) mutable {
         if (!newPage) {
             reply(WTF::nullopt, WTF::nullopt);
@@ -5531 +5537 @@
         newPage->m_shouldSuppressAppLinksInNextNavigationPolicyDecision = mainFrameURL.host() == request.url().host();
 
+        newPage->m_newPageNavigationAdClickAttribution = adClickAttribution;
 #if HAVE(APP_SSO)
         newPage->m_shouldSuppressSOAuthorizationInNextNavigationPolicyDecision = true;

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -2886 +2886 @@
 
     size_t m_suspendMediaPlaybackCounter { 0 };
+
+    Optional<WebCore::AdClickAttribution> m_newPageNavigationAdClickAttribution;
 };
 

trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp

@@ -268 +268 @@
     navigationActionData.shouldOpenExternalURLsPolicy = navigationAction.shouldOpenExternalURLsPolicy();
     navigationActionData.downloadAttribute = navigationAction.downloadAttribute();
+    navigationActionData.adClickAttribution = navigationAction.adClickAttribution();
 
     WebFrame* webFrame = WebFrame::fromCoreFrame(frame);

trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

@@ -880 +880 @@
     navigationActionData.shouldOpenExternalURLsPolicy = navigationAction.shouldOpenExternalURLsPolicy();
     navigationActionData.downloadAttribute = navigationAction.downloadAttribute();
+    navigationActionData.adClickAttribution = navigationAction.adClickAttribution();
 
     webPage->send(Messages::WebPageProxy::DecidePolicyForNewWindowAction(m_frame->frameID(), m_frame->info(), identifier, navigationActionData, request,