Attachment 368588 Details for Bug 197332 – Patch

[patch] Patch

bug-197332-20190430123129.patch (text/plain), 35.15 KB, created by John Wilander on 2019-04-30 12:31:30 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: John Wilander

Created: 2019-04-30 12:31:30 PDT

Size: 35.15 KB

patch

obsolete

>Subversion Revision: 244756
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 83599a922aed400447c99120798be0e01a1fbdee..181d898366fcb1e7e268f2560795f5d706f584c0 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-04-30  John Wilander  <wilander@apple.com>
>+
>+        Add logging of Ad Click Attribution errors and events to a dedicated channel
>+        https://bugs.webkit.org/show_bug.cgi?id=197332
>+        <rdar://problem/49918800>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added missing RELEASE_LOG_INFO and RELEASE_LOG_INFO_IF dummies
>+        for RELEASE_LOG_DISABLED.
>+
>+        * wtf/Assertions.h:
>+
> 2019-04-29  Alex Christensen  <achristensen@webkit.org>
> 
>         <rdar://problem/50299396> Fix internal High Sierra build
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f1044ee448837df865d569e12e4f2b8f3bd35466..ee676a517d80809e249d144f9537dc348fcae55a 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-04-30  John Wilander  <wilander@apple.com>
>+
>+        Add logging of Ad Click Attribution errors and events to a dedicated channel
>+        https://bugs.webkit.org/show_bug.cgi?id=197332
>+        <rdar://problem/49918800>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch adds an experimental Ad Click Attribution debug mode which
>+        logs information.
>+
>+        No new tests.
>+
>+        * loader/AdClickAttribution.cpp:
>+        (WebCore::AdClickAttribution::parseConversionRequest):
>+        (WebCore::AdClickAttribution::debugModeEnabled):
>+        * loader/AdClickAttribution.h:
>+        * page/RuntimeEnabledFeatures.h:
>+        (WebCore::RuntimeEnabledFeatures::adClickAttributionDebugModeEnabled const):
>+        (WebCore::RuntimeEnabledFeatures::setAdClickAttributionDebugModeEnabled):
>+        * platform/Logging.h:
>+
> 2019-04-29  Alex Christensen  <achristensen@webkit.org>
> 
>         <rdar://problem/50299396> Fix internal High Sierra build
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 94f9865af63e09a158191ac41bdbc85f0cc521e9..a8ac34bc97736b85f8811b6167a1ad9326790cee 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,57 @@
>+2019-04-30  John Wilander  <wilander@apple.com>
>+
>+        Add logging of Ad Click Attribution errors and events to a dedicated channel
>+        https://bugs.webkit.org/show_bug.cgi?id=197332
>+        <rdar://problem/49918800>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch adds an experimental Ad Click Attribution debug mode which
>+        logs information. Most changes are just log output in the various
>+        functions in WebKit::AdClickAttributionManager.
>+
>+        The constructor to WebKit::AdClickAttributionManager now takes a
>+        PAL::SessionID so that the log functions can make sure they don't
>+        output anything in ephemeral sessions.
>+
>+        WebProcessPool::platformInitializeNetworkProcess() now picks up the
>+        debug mode setting from the incoming
>+        WebKit::NetworkProcessCreationParameters object.
>+
>+        NetworkResourceLoader::handleAdClickAttributionConversion() was
>+        moved to AdClickAttributionManager::handleConversion() to keep all
>+        the logging in one file.
>+
>+        * NetworkProcess/AdClickAttributionManager.cpp:
>+        (WebKit::AdClickAttributionManager::storeUnconverted):
>+        (WebKit::AdClickAttributionManager::handleConversion):
>+        (WebKit::AdClickAttributionManager::convert):
>+        (WebKit::AdClickAttributionManager::fireConversionRequest):
>+        (WebKit::AdClickAttributionManager::firePendingConversionRequests):
>+        (WebKit::AdClickAttributionManager::clearExpired):
>+        (WebKit::AdClickAttributionManager::debugModeEnabled const):
>+        * NetworkProcess/AdClickAttributionManager.h:
>+        (WebKit::AdClickAttributionManager::AdClickAttributionManager):
>+        (WebKit::AdClickAttributionManager::m_sessionID):
>+        * NetworkProcess/NetworkProcess.cpp:
>+        (WebKit::NetworkProcess::initializeNetworkProcess):
>+        * NetworkProcess/NetworkProcessCreationParameters.cpp:
>+        (WebKit::NetworkProcessCreationParameters::encode const):
>+        (WebKit::NetworkProcessCreationParameters::decode):
>+        * NetworkProcess/NetworkProcessCreationParameters.h:
>+        * NetworkProcess/NetworkResourceLoader.cpp:
>+        (WebKit::NetworkResourceLoader::continueWillSendRedirectedRequest):
>+        (WebKit::NetworkResourceLoader::handleAdClickAttributionConversion): Deleted.
>+        * NetworkProcess/NetworkSession.cpp:
>+        (WebKit::NetworkSession::NetworkSession):
>+        (WebKit::NetworkSession::handleAdClickAttributionConversion):
>+        (WebKit::NetworkSession::convertAdClickAttribution): Deleted.
>+        * NetworkProcess/NetworkSession.h:
>+        * Platform/Logging.h:
>+        * Shared/WebPreferences.yaml:
>+        * UIProcess/Cocoa/WebProcessPoolCocoa.mm:
>+        (WebKit::WebProcessPool::platformInitializeNetworkProcess):
>+
> 2019-04-29  Alex Christensen  <achristensen@webkit.org>
> 
>         <rdar://problem/50299396> Fix internal High Sierra build
>diff --git a/Source/WTF/wtf/Assertions.h b/Source/WTF/wtf/Assertions.h
>index ac1a077459537bb951cb8cd5ab1d24fb1fa5643f..dd64976328400efbf5d89bbb29aba5c73c4b9ed5 100644
>--- a/Source/WTF/wtf/Assertions.h
>+++ b/Source/WTF/wtf/Assertions.h
>@@ -490,9 +490,11 @@ WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithSecurityImplication(v
> #define RELEASE_LOG(channel, ...) ((void)0)
> #define RELEASE_LOG_ERROR(channel, ...) LOG_ERROR(__VA_ARGS__)
> #define RELEASE_LOG_FAULT(channel, ...) LOG_ERROR(__VA_ARGS__)
>+#define RELEASE_LOG_INFO(channel, ...) ((void)0)
> 
> #define RELEASE_LOG_IF(isAllowed, channel, ...) ((void)0)
> #define RELEASE_LOG_ERROR_IF(isAllowed, channel, ...) do { if (isAllowed) RELEASE_LOG_ERROR(channel, __VA_ARGS__); } while (0)
>+#define RELEASE_LOG_INFO_IF(isAllowed, channel, ...) ((void)0)
> 
> #define RELEASE_LOG_WITH_LEVEL(channel, level, ...) ((void)0)
> #define RELEASE_LOG_WITH_LEVEL_IF(isAllowed, channel, level, ...) do { if (isAllowed) RELEASE_LOG_WITH_LEVEL(channel, level, __VA_ARGS__); } while (0)
>diff --git a/Source/WebCore/loader/AdClickAttribution.cpp b/Source/WebCore/loader/AdClickAttribution.cpp
>index 5810094f80664e87c431c82eea38a399d1a94f3e..65f7f5466f9d2a395d7413b441ac2d0140b27af3 100644
>--- a/Source/WebCore/loader/AdClickAttribution.cpp
>+++ b/Source/WebCore/loader/AdClickAttribution.cpp
>@@ -26,6 +26,8 @@
> #include "config.h"
> #include "AdClickAttribution.h"
> 
>+#include "Logging.h"
>+#include "RuntimeEnabledFeatures.h"
> #include <wtf/RandomNumber.h>
> #include <wtf/URL.h>
> #include <wtf/text/StringBuilder.h>
>@@ -50,34 +52,45 @@ bool AdClickAttribution::isValid() const
> 
> Optional<AdClickAttribution::Conversion> AdClickAttribution::parseConversionRequest(const URL& redirectURL)
> {
>-    if (!redirectURL.protocolIs("https"_s) || redirectURL.hasUsername() || redirectURL.hasPassword() || redirectURL.hasQuery() || redirectURL.hasFragment())
>+    if (!redirectURL.protocolIs("https"_s) || redirectURL.hasUsername() || redirectURL.hasPassword() || redirectURL.hasQuery() || redirectURL.hasFragment()) {
>+        RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Conversion was not accepted because the URL: %{public}s is not HTTPS or contains one or more of username, password, query string, and fragment.", redirectURL.string().utf8().data());
>         return { };
>+    }
> 
>     auto path = StringView(redirectURL.string()).substring(redirectURL.pathStart(), redirectURL.pathEnd() - redirectURL.pathStart());
>-    if (path.isEmpty() || !path.startsWith(adClickAttributionPathPrefix))
>+    if (path.isEmpty() || !path.startsWith(adClickAttributionPathPrefix)) {
>+        RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Conversion was not accepted because the URL path: %{public}s did not start with %{public}s.", path.utf8().data(), adClickAttributionPathPrefix);
>         return { };
>+    }
> 
>     auto prefixLength = sizeof(adClickAttributionPathPrefix) - 1;
>     if (path.length() == prefixLength + adClickConversionDataPathSegmentSize) {
>         auto conversionDataUInt64 = path.substring(prefixLength, adClickConversionDataPathSegmentSize).toUInt64Strict();
>-        if (!conversionDataUInt64 || *conversionDataUInt64 > MaxEntropy)
>+        if (!conversionDataUInt64 || *conversionDataUInt64 > MaxEntropy) {
>+            RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Conversion was not accepted because the conversion data could not be parsed or was higher than the allowed maximum of %{public}u.", MaxEntropy);
>             return { };
>+        }
> 
>         return Conversion { static_cast<uint32_t>(*conversionDataUInt64), Priority { 0 } };
>     }
>     
>     if (path.length() == prefixLength + adClickConversionDataPathSegmentSize + 1 + adClickPriorityPathSegmentSize) {
>         auto conversionDataUInt64 = path.substring(prefixLength, adClickConversionDataPathSegmentSize).toUInt64Strict();
>-        if (!conversionDataUInt64 || *conversionDataUInt64 > MaxEntropy)
>+        if (!conversionDataUInt64 || *conversionDataUInt64 > MaxEntropy) {
>+            RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Conversion was not accepted because the conversion data could not be parsed or was higher than the allowed maximum of %{public}u.", MaxEntropy);
>             return { };
>+        }
> 
>         auto conversionPriorityUInt64 = path.substring(prefixLength + adClickConversionDataPathSegmentSize + 1, adClickPriorityPathSegmentSize).toUInt64Strict();
>-        if (!conversionPriorityUInt64 || *conversionPriorityUInt64 > MaxEntropy)
>+        if (!conversionPriorityUInt64 || *conversionPriorityUInt64 > MaxEntropy) {
>+            RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Conversion was not accepted because the priority could not be parsed or was higher than the allowed maximum of %{public}u.", MaxEntropy);
>             return { };
>+        }
> 
>         return Conversion { static_cast<uint32_t>(*conversionDataUInt64), Priority { static_cast<uint32_t>(*conversionPriorityUInt64) } };
>     }
> 
>+    RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Conversion was not accepted because the URL path: %{public}s contained unrecognized parts.", path.utf8().data());
>     return { };
> }
> 
>@@ -212,4 +225,9 @@ String AdClickAttribution::toString() const
>     return builder.toString();
> }
> 
>+bool AdClickAttribution::debugModeEnabled()
>+{
>+    return RuntimeEnabledFeatures::sharedFeatures().adClickAttributionDebugModeEnabled();
>+}
>+
> }
>diff --git a/Source/WebCore/loader/AdClickAttribution.h b/Source/WebCore/loader/AdClickAttribution.h
>index 3e1ac745926dfb8e1be77f96f95937ab530a140f..38b947d8a40e8fc67e5352dce56a8ff64370cc1e 100644
>--- a/Source/WebCore/loader/AdClickAttribution.h
>+++ b/Source/WebCore/loader/AdClickAttribution.h
>@@ -262,6 +262,7 @@ public:
> 
> private:
>     bool isValid() const;
>+    static bool debugModeEnabled();
> 
>     Campaign m_campaign;
>     Source m_source;
>diff --git a/Source/WebCore/page/RuntimeEnabledFeatures.h b/Source/WebCore/page/RuntimeEnabledFeatures.h
>index c61c8fd197f672e97ab43001f388670402cea19e..86545a54c92e84e27237d21368166bdba3bb4c35 100644
>--- a/Source/WebCore/page/RuntimeEnabledFeatures.h
>+++ b/Source/WebCore/page/RuntimeEnabledFeatures.h
>@@ -341,6 +341,8 @@ public:
> 
>     bool adClickAttributionEnabled() const { return m_adClickAttributionEnabled; }
>     void setAdClickAttributionEnabled(bool isEnabled) { m_adClickAttributionEnabled = isEnabled; }
>+    bool adClickAttributionDebugModeEnabled() const { return m_adClickAttributionDebugModeEnabled; }
>+    void setAdClickAttributionDebugModeEnabled(bool isEnabled) { m_adClickAttributionDebugModeEnabled = isEnabled; }
> 
> #if ENABLE(TOUCH_EVENTS)
>     bool mouseEventsSimulationEnabled() const { return m_mouseEventsSimulationEnabled; }
>@@ -525,6 +527,7 @@ private:
>     bool m_CSSLogicalEnabled { false };
> 
>     bool m_adClickAttributionEnabled { false };
>+    bool m_adClickAttributionDebugModeEnabled { false };
> 
> #if ENABLE(TOUCH_EVENTS)
>     bool m_mouseEventsSimulationEnabled { false };
>diff --git a/Source/WebCore/platform/Logging.h b/Source/WebCore/platform/Logging.h
>index abf079defa28a34dce806607df56af095cf712ad..0171229bdcd8faf3dc2319b1b0b81d0073f86a8f 100644
>--- a/Source/WebCore/platform/Logging.h
>+++ b/Source/WebCore/platform/Logging.h
>@@ -38,6 +38,7 @@ namespace WebCore {
> #endif
> 
> #define WEBCORE_LOG_CHANNELS(M) \
>+    M(AdClickAttribution) \
>     M(Animations) \
>     M(ApplePay) \
>     M(Archives) \
>diff --git a/Source/WebKit/NetworkProcess/AdClickAttributionManager.cpp b/Source/WebKit/NetworkProcess/AdClickAttributionManager.cpp
>index 08b00fb9262396fbc41f60441fa507140aa8987b..933b9bdf30769bf0e645acf5bfb791290368dfd9 100644
>--- a/Source/WebKit/NetworkProcess/AdClickAttributionManager.cpp
>+++ b/Source/WebKit/NetworkProcess/AdClickAttributionManager.cpp
>@@ -26,12 +26,14 @@
> #include "config.h"
> #include "AdClickAttributionManager.h"
> 
>+#include "Logging.h"
> #include <WebCore/FetchOptions.h>
> #include <WebCore/FormData.h>
> #include <WebCore/ResourceError.h>
> #include <WebCore/ResourceRequest.h>
> #include <WebCore/ResourceResponse.h>
> #include <WebCore/RuntimeApplicationChecks.h>
>+#include <WebCore/RuntimeEnabledFeatures.h>
> #include <wtf/text/StringBuilder.h>
> #include <wtf/text/StringHash.h>
> 
>@@ -43,13 +45,37 @@ using Destination = AdClickAttribution::Destination;
> using DestinationMap = HashMap<Destination, AdClickAttribution>;
> using Conversion = AdClickAttribution::Conversion;
> 
>+constexpr Seconds debugModeSecondsUntilSend { 60_s };
>+
> void AdClickAttributionManager::storeUnconverted(AdClickAttribution&& attribution)
> {
>     clearExpired();
>-    
>+
>+    RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Storing an ad click from source: %{public}s to destination: %{public}s.", attribution.source().registrableDomain.string().utf8().data(), attribution.destination().registrableDomain.string().utf8().data());
>     m_unconvertedAdClickAttributionMap.set(std::make_pair(attribution.source(), attribution.destination()), WTFMove(attribution));
> }
> 
>+void AdClickAttributionManager::handleConversion(Conversion&& conversion, const URL& requestURL, const WebCore::ResourceRequest& redirectRequest)
>+{
>+    if (m_sessionID.isEphemeral())
>+        return;
>+
>+    RegistrableDomain redirectDomain { redirectRequest.url() };
>+    auto& firstPartyURL = redirectRequest.firstPartyForCookies();
>+
>+    if (!redirectDomain.matches(requestURL)) {
>+        RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Conversion was not accepted because the HTTP redirect from: %{public}s to: %{public}s was not same-site.", redirectDomain.string().utf8().data(), requestURL.string().utf8().data());
>+        return;
>+    }
>+
>+    if (redirectDomain.matches(firstPartyURL)) {
>+        RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Conversion was not accepted because it was requested in an HTTP redirect that is same-site as the first-party.");
>+        return;
>+    }
>+
>+    convert(AdClickAttribution::Source { WTFMove(redirectDomain) }, AdClickAttribution::Destination { firstPartyURL }, WTFMove(conversion));
>+}
>+
> void AdClickAttributionManager::startTimer(Seconds seconds)
> {
>     m_firePendingConversionRequestsTimer.startOneShot(m_isRunningTest ? 0_s : seconds);
>@@ -59,8 +85,17 @@ void AdClickAttributionManager::convert(const Source& source, const Destination&
> {
>     clearExpired();
> 
>-    if (!conversion.isValid())
>+    if (!conversion.isValid()) {
>+        RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Got an invalid conversion.");
>         return;
>+    }
>+
>+#if !RELEASE_LOG_DISABLED
>+    auto& conversionData = conversion.data;
>+    auto& conversionPriority = conversion.priority;
>+#endif
>+
>+    RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Got a conversion for source: %{public}s and destination: %{public}s with conversion data: %{public}u and priority: %{public}u.", source.registrableDomain.string().utf8().data(), destination.registrableDomain.string().utf8().data(), conversionData, conversionPriority);
> 
>     auto secondsUntilSend = Seconds::infinity();
> 
>@@ -73,17 +108,22 @@ void AdClickAttributionManager::convert(const Source& source, const Destination&
>         if (auto optionalSecondsUntilSend = previouslyUnconvertedAttribution.convertAndGetEarliestTimeToSend(WTFMove(conversion))) {
>             secondsUntilSend = *optionalSecondsUntilSend;
>             ASSERT(secondsUntilSend != Seconds::infinity());
>+            RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Converted a stored ad click for source: %{public}s and destination: %{public}s with conversion data: %{public}u and priority: %{public}u.", source.registrableDomain.string().utf8().data(), destination.registrableDomain.string().utf8().data(), conversionData, conversionPriority);
>         }
>-        // If there is no previously converted attribution for this pair, add the new one.
>-        // If the newly converted attribution has higher priority, replace the old one.
>-        if (previouslyConvertedAttributionIter == m_convertedAdClickAttributionMap.end()
>-            || previouslyUnconvertedAttribution.hasHigherPriorityThan(previouslyConvertedAttributionIter->value))
>+
>+        if (previouslyConvertedAttributionIter == m_convertedAdClickAttributionMap.end())
>+            m_convertedAdClickAttributionMap.add(pair, WTFMove(previouslyUnconvertedAttribution));
>+        else if (previouslyUnconvertedAttribution.hasHigherPriorityThan(previouslyConvertedAttributionIter->value)) {
>+            // If the newly converted attribution has higher priority, replace the old one.
>             m_convertedAdClickAttributionMap.set(pair, WTFMove(previouslyUnconvertedAttribution));
>+            RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Replaced a previously converted ad click for source: %{public}s and destination: %{public}s with a new one with conversion data: %{public}u and priority: %{public}u because it had higher priority.", source.registrableDomain.string().utf8().data(), destination.registrableDomain.string().utf8().data(), conversionData, conversionPriority);
>+        }
>     } else if (previouslyConvertedAttributionIter != m_convertedAdClickAttributionMap.end()) {
>         // If we have no newly converted attribution, re-convert the old one to respect the new priority.
>         if (auto optionalSecondsUntilSend = previouslyConvertedAttributionIter->value.convertAndGetEarliestTimeToSend(WTFMove(conversion))) {
>             secondsUntilSend = *optionalSecondsUntilSend;
>             ASSERT(secondsUntilSend != Seconds::infinity());
>+            RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Re-converted an ad click for source: %{public}s and destination: %{public}s with a new one with conversion data: %{public}u and priority: %{public}u because it had higher priority.", source.registrableDomain.string().utf8().data(), destination.registrableDomain.string().utf8().data(), conversionData, conversionPriority);
>         }
>     }
> 
>@@ -92,7 +132,12 @@ void AdClickAttributionManager::convert(const Source& source, const Destination&
>     
>     if (m_firePendingConversionRequestsTimer.isActive() && m_firePendingConversionRequestsTimer.nextFireInterval() < secondsUntilSend)
>         return;
>-    
>+
>+    if (debugModeEnabled()) {
>+        RELEASE_LOG_INFO(AdClickAttribution, "Setting timer for firing conversion requests to the debug mode timeout of %{public}f seconds where the regular timeout would have been %{public}f seconds.", debugModeSecondsUntilSend.seconds(), secondsUntilSend.seconds());
>+        secondsUntilSend = debugModeSecondsUntilSend;
>+    }
>+
>     startTimer(secondsUntilSend);
> }
> 
>@@ -133,10 +178,14 @@ void AdClickAttributionManager::fireConversionRequest(const AdClickAttribution&
>     loadParameters.mainDocumentURL = WTFMove(conversionReferrerURL);
> #endif
> 
>-    m_pingLoadFunction(WTFMove(loadParameters), [](const WebCore::ResourceError& error, const WebCore::ResourceResponse& response) {
>-        // FIXME: Add logging of errors to a dedicated channel.
>+    RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "About to fire an attribution request for a conversion with source: %{public}s and destination: %{public}s.", attribution.source().registrableDomain.string().utf8().data(), attribution.destination().registrableDomain.string().utf8().data());
>+
>+    m_pingLoadFunction(WTFMove(loadParameters), [this, weakThis = makeWeakPtr(*this), sourceStr = attribution.source().registrableDomain.string(), destinationStr = attribution.destination().registrableDomain.string()](const WebCore::ResourceError& error, const WebCore::ResourceResponse& response) {
>+        if (!weakThis)
>+            return;
>+
>+        RELEASE_LOG_INFO_IF(debugModeEnabled() && !error.isNull(), AdClickAttribution, "Received error: '%{public}s' for attribution request with source: %{public}s and destination: %{public}s.", error.localizedDescription().utf8().data(), sourceStr.utf8().data(), destinationStr.utf8().data());
>         UNUSED_PARAM(response);
>-        UNUSED_PARAM(error);
>     });
> }
> 
>@@ -155,7 +204,7 @@ void AdClickAttributionManager::firePendingConversionRequests()
>         }
> 
>         auto now = WallTime::now();
>-        if (*earliestTimeToSend <= now || m_isRunningTest) {
>+        if (*earliestTimeToSend <= now || m_isRunningTest || debugModeEnabled()) {
>             fireConversionRequest(attribution);
>             attribution.markConversionAsSent();
>             continue;
>@@ -193,8 +242,14 @@ void AdClickAttributionManager::clearForRegistrableDomain(const RegistrableDomai
> 
> void AdClickAttributionManager::clearExpired()
> {
>-    m_unconvertedAdClickAttributionMap.removeIf([](auto& keyAndValue) {
>-        return keyAndValue.value.hasExpired();
>+    m_unconvertedAdClickAttributionMap.removeIf([this, weakThis = makeWeakPtr(*this)](auto& keyAndValue) {
>+        if (!weakThis)
>+            return false;
>+
>+        auto hasExpired = keyAndValue.value.hasExpired();
>+        if (hasExpired)
>+            RELEASE_LOG_INFO_IF(debugModeEnabled(), AdClickAttribution, "Removing expired ad click from source: %{public}s to destination: %{public}s.", keyAndValue.value.source().registrableDomain.string().utf8().data(), keyAndValue.value.destination().registrableDomain.string().utf8().data());
>+        return hasExpired;
>     });
> }
> 
>@@ -247,4 +302,9 @@ void AdClickAttributionManager::markAllUnconvertedAsExpiredForTesting()
>         attribution.markAsExpired();
> }
> 
>+bool AdClickAttributionManager::debugModeEnabled() const
>+{
>+    return RuntimeEnabledFeatures::sharedFeatures().adClickAttributionDebugModeEnabled() && !m_sessionID.isEphemeral();
>+}
>+
> } // namespace WebKit
>diff --git a/Source/WebKit/NetworkProcess/AdClickAttributionManager.h b/Source/WebKit/NetworkProcess/AdClickAttributionManager.h
>index 250748c058455999e7291b09811cdea7df79db7e..b62f12e182f21eb8d51074cb254d7b212e0ced19 100644
>--- a/Source/WebKit/NetworkProcess/AdClickAttributionManager.h
>+++ b/Source/WebKit/NetworkProcess/AdClickAttributionManager.h
>@@ -33,11 +33,12 @@
> #include <WebCore/Timer.h>
> #include <wtf/CompletionHandler.h>
> #include <wtf/HashMap.h>
>+#include <wtf/WeakPtr.h>
> #include <wtf/text/WTFString.h>
> 
> namespace WebKit {
> 
>-class AdClickAttributionManager {
>+class AdClickAttributionManager : public CanMakeWeakPtr<AdClickAttributionManager> {
> public:
> 
>     using RegistrableDomain = WebCore::RegistrableDomain;
>@@ -46,17 +47,18 @@ public:
>     using Destination = WebCore::AdClickAttribution::Destination;
>     using Conversion = WebCore::AdClickAttribution::Conversion;
> 
>-    AdClickAttributionManager()
>+    explicit AdClickAttributionManager(PAL::SessionID sessionID)
>         : m_firePendingConversionRequestsTimer(*this, &AdClickAttributionManager::firePendingConversionRequests)
>         , m_pingLoadFunction([](NetworkResourceLoadParameters&& params, CompletionHandler<void(const WebCore::ResourceError&, const WebCore::ResourceResponse&)>&& completionHandler) {
>             UNUSED_PARAM(params);
>             completionHandler(WebCore::ResourceError(), WebCore::ResourceResponse());
>         })
>+        , m_sessionID(sessionID)
>     {
>     }
> 
>     void storeUnconverted(AdClickAttribution&&);
>-    void convert(const Source&, const Destination&, Conversion&&);
>+    void handleConversion(Conversion&&, const URL& requestURL, const WebCore::ResourceRequest& redirectRequest);
>     void clear();
>     void clearForRegistrableDomain(const RegistrableDomain&);
>     void toString(CompletionHandler<void(String)>&&) const;
>@@ -67,9 +69,11 @@ public:
> 
> private:
>     void startTimer(Seconds);
>+    void convert(const Source&, const Destination&, Conversion&&);
>     void fireConversionRequest(const AdClickAttribution&);
>     void firePendingConversionRequests();
>     void clearExpired();
>+    bool debugModeEnabled() const;
> 
>     HashMap<std::pair<Source, Destination>, AdClickAttribution> m_unconvertedAdClickAttributionMap;
>     HashMap<std::pair<Source, Destination>, AdClickAttribution> m_convertedAdClickAttributionMap;
>@@ -77,6 +81,7 @@ private:
>     Function<void(NetworkResourceLoadParameters&&, CompletionHandler<void(const WebCore::ResourceError&, const WebCore::ResourceResponse&)>&&)> m_pingLoadFunction;
>     bool m_isRunningTest { false };
>     Optional<URL> m_conversionBaseURLForTesting;
>+    PAL::SessionID m_sessionID;
> };
>     
> } // namespace WebKit
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>index 69226f0063ba4e37afbac685f92067ce67854b59..6ea9e226d896c094a0c863d2465aea2806878d8f 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>@@ -306,6 +306,8 @@ void NetworkProcess::initializeNetworkProcess(NetworkProcessCreationParameters&&
>     WebCore::RuntimeEnabledFeatures::sharedFeatures().setIsITPDatabaseEnabled(parameters.shouldEnableITPDatabase);
>     WebCore::RuntimeEnabledFeatures::sharedFeatures().setIsITPFirstPartyWebsiteDataRemovalEnabled(parameters.isITPFirstPartyWebsiteDataRemovalEnabled);
> 
>+    WebCore::RuntimeEnabledFeatures::sharedFeatures().setAdClickAttributionDebugModeEnabled(parameters.enableAdClickAttributionDebugMode);
>+
>     SandboxExtension::consumePermanently(parameters.defaultDataStoreParameters.networkSessionParameters.resourceLoadStatisticsDirectoryExtensionHandle);
> 
>     auto sessionID = parameters.defaultDataStoreParameters.networkSessionParameters.sessionID;
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp b/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp
>index ed319975d86d53ceca78ff563848622bbddf5f50..40593199ae4353bf1601699bd5ce12c0827a6c0d 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.cpp
>@@ -91,6 +91,7 @@ void NetworkProcessCreationParameters::encode(IPC::Encoder& encoder) const
>     encoder << shouldEnableITPDatabase;
>     encoder << downloadMonitorSpeedMultiplier;
>     encoder << isITPFirstPartyWebsiteDataRemovalEnabled;
>+    encoder << enableAdClickAttributionDebugMode;
> }
> 
> bool NetworkProcessCreationParameters::decode(IPC::Decoder& decoder, NetworkProcessCreationParameters& result)
>@@ -220,6 +221,9 @@ bool NetworkProcessCreationParameters::decode(IPC::Decoder& decoder, NetworkProc
>     if (!decoder.decode(result.isITPFirstPartyWebsiteDataRemovalEnabled))
>         return false;
> 
>+    if (!decoder.decode(result.enableAdClickAttributionDebugMode))
>+        return false;
>+
>     return true;
> }
> 
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h b/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h
>index 0f1f4407107e8d3368f320326cf7c9e410b3b2c4..203be69ddac0160a63901faeeed3d3ec3794c829 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h
>+++ b/Source/WebKit/NetworkProcess/NetworkProcessCreationParameters.h
>@@ -109,6 +109,7 @@ struct NetworkProcessCreationParameters {
>     bool shouldEnableITPDatabase { false };
>     bool isITPFirstPartyWebsiteDataRemovalEnabled { true };
>     uint32_t downloadMonitorSpeedMultiplier { 1 };
>+    bool enableAdClickAttributionDebugMode { false };
> };
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 048f077f90bbceb276d452c3e90e0ebaf2e373aa..5ac46e5ffc596d6ee8024430e99d45bb6668b06f 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -600,18 +600,6 @@ Optional<Seconds> NetworkResourceLoader::validateCacheEntryForMaxAgeCapValidatio
>     return WTF::nullopt;
> }
> 
>-void NetworkResourceLoader::handleAdClickAttributionConversion(AdClickAttribution::Conversion&& conversion, const URL& requestURL, const WebCore::ResourceRequest& redirectRequest)
>-{
>-    ASSERT(!sessionID().isEphemeral());
>-
>-    RegistrableDomain redirectDomain { redirectRequest.url() };
>-    auto& firstPartyURL = redirectRequest.firstPartyForCookies();
>-    NetworkSession* networkSession = nullptr;
>-    // The redirect has to be done by the same registrable domain and it has to be a third-party request.
>-    if (redirectDomain.matches(requestURL) && !redirectDomain.matches(firstPartyURL) && (networkSession = m_connection->networkProcess().networkSession(sessionID())))
>-        networkSession->convertAdClickAttribution(AdClickAttribution::Source { WTFMove(redirectDomain) }, AdClickAttribution::Destination { firstPartyURL }, WTFMove(conversion));
>-}
>-
> void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse)
> {
>     ++m_redirectCount;
>@@ -672,8 +660,10 @@ void NetworkResourceLoader::continueWillSendRedirectedRequest(ResourceRequest&&
>         return;
>     }
> 
>-    if (adClickConversion)
>-        handleAdClickAttributionConversion(WTFMove(*adClickConversion), request.url(), redirectRequest);
>+    NetworkSession* networkSession = nullptr;
>+    if (adClickConversion && (networkSession = m_connection->networkProcess().networkSession(sessionID())))
>+        networkSession->handleAdClickAttributionConversion(WTFMove(*adClickConversion), request.url(), redirectRequest);
>+
>     send(Messages::WebResourceLoader::WillSendRequest(redirectRequest, sanitizeResponseIfPossible(WTFMove(redirectResponse), ResourceResponse::SanitizationType::Redirection)));
> }
> 
>diff --git a/Source/WebKit/NetworkProcess/NetworkSession.cpp b/Source/WebKit/NetworkProcess/NetworkSession.cpp
>index 5b442bf282f15cb2e99bb28490772f08fdb97141..a185976817c491374c67b4e250def9b24999f5c8 100644
>--- a/Source/WebKit/NetworkProcess/NetworkSession.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkSession.cpp
>@@ -74,7 +74,7 @@ NetworkStorageSession& NetworkSession::networkStorageSession() const
> NetworkSession::NetworkSession(NetworkProcess& networkProcess, PAL::SessionID sessionID)
>     : m_sessionID(sessionID)
>     , m_networkProcess(networkProcess)
>-    , m_adClickAttribution(makeUniqueRef<AdClickAttributionManager>())
>+    , m_adClickAttribution(makeUniqueRef<AdClickAttributionManager>(sessionID))
> {
>     m_adClickAttribution->setPingLoadFunction([this, weakThis = makeWeakPtr(this)](NetworkResourceLoadParameters&& loadParameters, CompletionHandler<void(const WebCore::ResourceError&, const WebCore::ResourceResponse&)>&& completionHandler) {
>         if (!weakThis)
>@@ -149,9 +149,9 @@ void NetworkSession::storeAdClickAttribution(WebCore::AdClickAttribution&& adCli
>     m_adClickAttribution->storeUnconverted(WTFMove(adClickAttribution));
> }
> 
>-void NetworkSession::convertAdClickAttribution(const WebCore::AdClickAttribution::Source& source, const WebCore::AdClickAttribution::Destination& destination, WebCore::AdClickAttribution::Conversion&& conversion)
>+void NetworkSession::handleAdClickAttributionConversion(AdClickAttribution::Conversion&& conversion, const URL& requestURL, const WebCore::ResourceRequest& redirectRequest)
> {
>-    m_adClickAttribution->convert(source, destination, WTFMove(conversion));
>+    m_adClickAttribution->handleConversion(WTFMove(conversion), requestURL, redirectRequest);
> }
> 
> void NetworkSession::dumpAdClickAttribution(CompletionHandler<void(String)>&& completionHandler)
>diff --git a/Source/WebKit/NetworkProcess/NetworkSession.h b/Source/WebKit/NetworkProcess/NetworkSession.h
>index 7099db815dfdd455061b0a4443c44a09b1500fc6..f61ee26da5c2979cf81b559fddf60693742a6a3d 100644
>--- a/Source/WebKit/NetworkProcess/NetworkSession.h
>+++ b/Source/WebKit/NetworkProcess/NetworkSession.h
>@@ -81,7 +81,7 @@ public:
>     void notifyPageStatisticsTelemetryFinished(unsigned totalPrevalentResources, unsigned totalPrevalentResourcesWithUserInteraction, unsigned top3SubframeUnderTopFrameOrigins);
> #endif
>     void storeAdClickAttribution(WebCore::AdClickAttribution&&);
>-    void convertAdClickAttribution(const WebCore::AdClickAttribution::Source&, const WebCore::AdClickAttribution::Destination&, WebCore::AdClickAttribution::Conversion&&);
>+    void handleAdClickAttributionConversion(WebCore::AdClickAttribution::Conversion&&, const URL& requestURL, const WebCore::ResourceRequest& redirectRequest);
>     void dumpAdClickAttribution(CompletionHandler<void(String)>&&);
>     void clearAdClickAttribution();
>     void clearAdClickAttributionForRegistrableDomain(WebCore::RegistrableDomain&&);
>diff --git a/Source/WebKit/Platform/Logging.h b/Source/WebKit/Platform/Logging.h
>index 4c5cc67441f2b01ba49eadab4d41f3e9081f897d..9d5a42d34f02dd29accca043614ab89cd0d1afb6 100644
>--- a/Source/WebKit/Platform/Logging.h
>+++ b/Source/WebKit/Platform/Logging.h
>@@ -40,6 +40,7 @@ extern "C" {
> #endif
> 
> #define WEBKIT2_LOG_CHANNELS(M) \
>+    M(AdClickAttribution) \
>     M(Automation) \
>     M(ActivityState) \
>     M(BackForward) \
>diff --git a/Source/WebKit/Shared/WebPreferences.yaml b/Source/WebKit/Shared/WebPreferences.yaml
>index ff9da3b51ae89d7bf38f5f9cd8116fb81b711c1c..c4fccfcf18b6b5e2e5fd461622d3f5532a0ec7ef 100644
>--- a/Source/WebKit/Shared/WebPreferences.yaml
>+++ b/Source/WebKit/Shared/WebPreferences.yaml
>@@ -1412,6 +1412,14 @@ AdClickAttributionEnabled:
>     webcoreBinding: RuntimeEnabledFeatures
>     category: experimental
> 
>+AdClickAttributionDebugModeEnabled:
>+    type: bool
>+    defaultValue: false
>+    humanReadableName: "Ad Click Attribution Debug Mode"
>+    humanReadableDescription: "Enable Ad Click Attribution Debug Mode"
>+    webcoreBinding: RuntimeEnabledFeatures
>+    category: experimental
>+
> # For internal features:
> # The type should be boolean.
> # You must provide a humanReadableName and humanReadableDescription for all debug features. They
>diff --git a/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm b/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm
>index ef92bad356359c4fdec1ffc3a7dad81820b25f4b..807632ff57c963eedf50bcbb59c23c27d1af386b 100644
>--- a/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm
>@@ -304,6 +304,8 @@ void WebProcessPool::platformInitializeNetworkProcess(NetworkProcessCreationPara
>     auto isITPFirstPartyWebsiteDataRemovalEnabledStr = [defaults stringForKey:[NSString stringWithFormat:@"Experimental%@", WebPreferencesKey::isITPFirstPartyWebsiteDataRemovalEnabledKey().createCFString().get()]];
>     if ([isITPFirstPartyWebsiteDataRemovalEnabledStr isEqual:@"0"])
>         parameters.isITPFirstPartyWebsiteDataRemovalEnabled = false;
>+
>+    parameters.enableAdClickAttributionDebugMode = [defaults boolForKey:[NSString stringWithFormat:@"Experimental%@", WebPreferencesKey::adClickAttributionDebugModeEnabledKey().createCFString().get()]];
> }
> 
> void WebProcessPool::platformInvalidateContext()

Actions: View | Formatted Diff | Diff

Attachments on bug 197332: 368365 | 368371 | 368375 | 368386 | 368509 | 368576 | 368585 | 368588 | 368589 | 368645