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.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@269129 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8a06a3d..c8fa9c3 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,20 @@
+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>
Rebase for fast/forms/input-appearance-spinbutton.html on Catalina after changes in r269036
diff --git a/LayoutTests/http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window-expected.txt b/LayoutTests/http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window-expected.txt
new file mode 100644
index 0000000..38436b0
--- /dev/null
+++ b/LayoutTests/http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window-expected.txt
@@ -0,0 +1,12 @@
+Tests triggering of ad click attribution conversions in a new window.
+
+
+Converted Ad Click Attributions:
+WebCore::AdClickAttribution 1
+Source: 127.0.0.1
+Destination: localhost
+Campaign ID: 3
+Conversion data: 12
+Conversion priority: 3
+Conversion earliest time to send: Within 24-48 hours
+Conversion request sent: false
diff --git a/LayoutTests/http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window.html b/LayoutTests/http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window.html
new file mode 100644
index 0000000..989dcd1
--- /dev/null
+++ b/LayoutTests/http/tests/adClickAttribution/attribution-conversion-through-image-redirect-in-new-window.html
@@ -0,0 +1,59 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AdClickAttributionEnabled=true ] -->
+<html lang="en">
+<head>
+ <meta charset="UTF-8">
+ <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+ <script src="/js-test-resources/ui-helper.js"></script>
+ <script src="resources/util.js"></script>
+</head>
+<body onload="setTimeout(runTest, 0)">
+<div id="description">Tests triggering of ad click attribution conversions in a new window.</div>
+<a target="_blank" rel="opener" id="targetLink" href="http://localhost:8000/adClickAttribution/resources/convertAndPostMessageBack.html" adcampaignid="3" addestination="http://localhost:8000">Link</a><br>
+<div id="output"></div>
+<script>
+ prepareTest();
+
+ function activateElement(elementID) {
+ var element = document.getElementById(elementID);
+ var centerX = element.offsetLeft + element.offsetWidth / 2;
+ var centerY = element.offsetTop + element.offsetHeight / 2;
+ UIHelper.activateAt(centerX, centerY).then(
+ function () {
+ },
+ function () {
+ document.getElementById("output").innerText = "FAIL Promise rejected.";
+ tearDownAndFinish();
+ }
+ );
+ }
+
+ function receiveMessage(event) {
+ if (event.origin === "http://localhost:8000") {
+ if (event.data === "Done") {
+ testRunner.dumpAdClickAttribution();
+ document.body.removeChild(document.getElementById("targetLink"));
+ tearDownAndFinish();
+ } else {
+ testFailed("Received unknown message: " + event.data);
+ tearDownAndFinish();
+ }
+ } else {
+ testFailed("Received a message from an unexpected origin: " + event.origin);
+ tearDownAndFinish();
+ }
+ }
+
+ window.addEventListener("message", receiveMessage, false);
+
+ function runTest() {
+ if (window.testRunner) {
+ testRunner.setCanOpenWindows();
+ activateElement("targetLink");
+ } else {
+ document.getElementById("output").innerText = "FAIL No testRunner.";
+ tearDownAndFinish();
+ }
+ }
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/adClickAttribution/resources/convertAndPostMessageBack.html b/LayoutTests/http/tests/adClickAttribution/resources/convertAndPostMessageBack.html
new file mode 100644
index 0000000..24835a7
--- /dev/null
+++ b/LayoutTests/http/tests/adClickAttribution/resources/convertAndPostMessageBack.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+ <meta charset="UTF-8">
+ <script>
+ function fireConversionPixel() {
+ let imageElement = document.createElement("img");
+ imageElement.src = "https://127.0.0.1:8443/adClickAttribution/resources/redirectToConversion.php?conversionData=12&priority=03";
+ imageElement.id = "pixel";
+ imageElement.onerror = function() {
+ document.body.removeChild(document.getElementById("pixel"));
+ postMessageBack();
+ };
+ document.body.appendChild(imageElement);
+ }
+
+ function postMessageBack() {
+ if (window.opener) {
+ window.opener.postMessage("Done", "http://127.0.0.1:8000");
+ window.close();
+ }
+ }
+ </script>
+</head>
+<body onload="setTimeout(fireConversionPixel, 0)">
+</body>
+</html>
\ No newline at end of file
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index ad67a67..acf88b4 100644
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,40 @@
+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>
[GPU Process] Eagerly flush the PutImageData item to the GPU Process
diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
index 9a2aa80..2617fdf 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -4651,12 +4651,18 @@
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()) {
m_mainFrameHasCustomContentProvider = frameHasCustomContentProvider;
@@ -5518,7 +5524,7 @@
auto* originatingPage = m_process->webPage(originatingPageID);
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);
return;
@@ -5530,6 +5536,7 @@
newPage->m_shouldSuppressAppLinksInNextNavigationPolicyDecision = mainFrameURL.host() == request.url().host();
+ newPage->m_newPageNavigationAdClickAttribution = adClickAttribution;
#if HAVE(APP_SSO)
newPage->m_shouldSuppressSOAuthorizationInNextNavigationPolicyDecision = true;
#endif
diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
index d4694f0..bdf11ba 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.h
+++ b/Source/WebKit/UIProcess/WebPageProxy.h
@@ -2885,6 +2885,8 @@
bool m_canUseCredentialStorage { true };
size_t m_suspendMediaPlaybackCounter { 0 };
+
+ Optional<WebCore::AdClickAttribution> m_newPageNavigationAdClickAttribution;
};
} // namespace WebKit
diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
index 2979ae0..1cd7ad7 100644
--- a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
@@ -267,6 +267,7 @@
navigationActionData.canHandleRequest = m_page.canHandleRequest(navigationAction.resourceRequest());
navigationActionData.shouldOpenExternalURLsPolicy = navigationAction.shouldOpenExternalURLsPolicy();
navigationActionData.downloadAttribute = navigationAction.downloadAttribute();
+ navigationActionData.adClickAttribution = navigationAction.adClickAttribution();
WebFrame* webFrame = WebFrame::fromCoreFrame(frame);
diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
index 66afdfc..975fd89 100644
--- a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
@@ -879,6 +879,7 @@
navigationActionData.canHandleRequest = webPage->canHandleRequest(request);
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,
frameName, listenerID, UserData(WebProcess::singleton().transformObjectsToHandles(userData.get()).get())));
