WebKit Bugzilla
Attachment 361029 Details for
Bug 194104
: Parse and handle Ad Click Attribution attributes in HTMLAnchorElement::handleClick()
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-194104-20190203141844.patch (text/plain), 18.53 KB, created by
John Wilander
on 2019-02-03 14:18:45 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
John Wilander
Created:
2019-02-03 14:18:45 PST
Size:
18.53 KB
patch
obsolete
>Subversion Revision: 240906 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 98128cdcd1a884195fe29eddb4c521d5870aee26..984bd9cf410d6478d473506757b9d07ae50af46f 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,29 @@ >+2019-02-03 John Wilander <wilander@apple.com> >+ >+ Parse and handle Ad Click Attribution attributes in HTMLAnchorElement::handleClick() >+ https://bugs.webkit.org/show_bug.cgi?id=194104 >+ <rdar://problem/47649991> >+ >+ Reviewed by Chris Dumez, Daniel Bates, and Darin Adler. >+ >+ Test: http/tests/adClickAttribution/anchor-tag-attributes-validation.html >+ >+ This patch adds parsing and validation of the two new Ad Click Attribution >+ attributes in anchor elements: adcampaignid and addestination. The data is >+ not yet forwarded into the loader. >+ >+ * html/HTMLAnchorElement.cpp: >+ (WebCore::HTMLAnchorElement::parseAdClickAttribution const): >+ (WebCore::HTMLAnchorElement::handleClick): >+ Now calls HTMLAnchorElement::parseAdClickAttribution(). >+ * html/HTMLAnchorElement.h: >+ * loader/AdClickAttribution.h: >+ Made WebCore::AdClickAttribution copyable since it's needed to have it be >+ WTF::Optional. Also made AdClickAttribution::MaxEntropy public. Changed >+ numeric types from unsigned short to uint32_t. >+ (WebCore::AdClickAttribution::Campaign::isValid const): >+ (WebCore::AdClickAttribution::Conversion::isValid const): >+ > 2019-02-03 Antti Koivisto <antti@apple.com> > > Don't include ScrollCoordinator.h from Element.h >diff --git a/Source/WebCore/html/HTMLAnchorElement.cpp b/Source/WebCore/html/HTMLAnchorElement.cpp >index e7e167c95b4177513432f96c6f41e385afb62e44..071870c472a08da10ef2b891fc389cd0f7c3ee33 100644 >--- a/Source/WebCore/html/HTMLAnchorElement.cpp >+++ b/Source/WebCore/html/HTMLAnchorElement.cpp >@@ -24,6 +24,7 @@ > #include "config.h" > #include "HTMLAnchorElement.h" > >+#include "AdClickAttribution.h" > #include "DOMTokenList.h" > #include "ElementIterator.h" > #include "EventHandler.h" >@@ -50,8 +51,10 @@ > #include "SecurityPolicy.h" > #include "Settings.h" > #include "URLUtils.h" >+#include "UserGestureIndicator.h" > #include <wtf/IsoMallocInlines.h> > #include <wtf/text/StringBuilder.h> >+#include <wtf/text/StringConcatenateNumbers.h> > > namespace WebCore { > >@@ -394,6 +397,52 @@ bool HTMLAnchorElement::isSystemPreviewLink() const > } > #endif > >+Optional<AdClickAttribution> HTMLAnchorElement::parseAdClickAttribution() const >+{ >+ using Campaign = AdClickAttribution::Campaign; >+ using Source = AdClickAttribution::Source; >+ using Destination = AdClickAttribution::Destination; >+ >+ if (!RuntimeEnabledFeatures::sharedFeatures().adClickAttributionEnabled() || !UserGestureIndicator::processingUserGesture()) >+ return WTF::nullopt; >+ >+ if (!hasAttributeWithoutSynchronization(adcampaignidAttr) && !hasAttributeWithoutSynchronization(addestinationAttr)) >+ return WTF::nullopt; >+ >+ auto adCampaignIDAttr = attributeWithoutSynchronization(adcampaignidAttr); >+ auto adDestinationAttr = attributeWithoutSynchronization(addestinationAttr); >+ >+ if (adCampaignIDAttr.isEmpty() || adDestinationAttr.isEmpty()) { >+ document().addConsoleMessage(MessageSource::Other, MessageLevel::Warning, "Both adcampaignid and addestination need to be set for Ad Click Attribution to work."_s); >+ return WTF::nullopt; >+ } >+ >+ RefPtr<Frame> frame = document().frame(); >+ if (!frame || !frame->isMainFrame()) { >+ document().addConsoleMessage(MessageSource::Other, MessageLevel::Warning, "Ad Click Attribution is only supported in the main frame."_s); >+ return WTF::nullopt; >+ } >+ >+ auto adCampaignID = parseHTMLNonNegativeInteger(adCampaignIDAttr); >+ if (!adCampaignID) { >+ document().addConsoleMessage(MessageSource::Other, MessageLevel::Warning, "adcampaignid can not be converted to a non-negative integer which is required for Ad Click Attribution."_s); >+ return WTF::nullopt; >+ } >+ >+ if (adCampaignID.value() >= AdClickAttribution::MaxEntropy) { >+ document().addConsoleMessage(MessageSource::Other, MessageLevel::Warning, makeString("adcampaignid must have a non-negative value less than ", AdClickAttribution::MaxEntropy, " for Ad Click Attribution.")); >+ return WTF::nullopt; >+ } >+ >+ URL adDestinationURL { URL(), adDestinationAttr }; >+ if (!adDestinationURL.isValid() || !adDestinationURL.protocolIsInHTTPFamily()) { >+ document().addConsoleMessage(MessageSource::Other, MessageLevel::Warning, "adddestination could not be converted to a valid HTTP-family URL."_s); >+ return WTF::nullopt; >+ } >+ >+ return AdClickAttribution { Campaign(adCampaignID.value()), Source(document().domain()), Destination(adDestinationURL.host().toString()) }; >+} >+ > void HTMLAnchorElement::handleClick(Event& event) > { > event.setDefaultHandled(); >@@ -438,6 +487,13 @@ void HTMLAnchorElement::handleClick(Event& event) > else if (hasRel(Relation::NoOpener) || (RuntimeEnabledFeatures::sharedFeatures().blankAnchorTargetImpliesNoOpenerEnabled() && equalIgnoringASCIICase(effectiveTarget, "_blank"))) > newFrameOpenerPolicy = NewFrameOpenerPolicy::Suppress; > >+ auto adClickAttribution = parseAdClickAttribution(); >+ // FIXME: The adClickAttribution should be forwarded to the loader and handled down the pipe. See >+ // rdar://problem/47650118 >+ // A matching conversion event needs to happen before the complete ad click attributionURL can be >+ // created. Thus, it should be empty for now. >+ ASSERT_UNUSED(adClickAttribution, !adClickAttribution || adClickAttribution->url().isNull()); >+ > frame->loader().urlSelected(completedURL, effectiveTarget, &event, LockHistory::No, LockBackForwardList::No, shouldSendReferrer, document().shouldOpenExternalURLsPolicyToPropagate(), newFrameOpenerPolicy, downloadAttribute, systemPreviewInfo); > > sendPings(completedURL); >diff --git a/Source/WebCore/html/HTMLAnchorElement.h b/Source/WebCore/html/HTMLAnchorElement.h >index 6b02c9ab4d462685036e170396e45b81e8d329e8..61315ced17fa4d6e06526cd90bb588aadfa2af97 100644 >--- a/Source/WebCore/html/HTMLAnchorElement.h >+++ b/Source/WebCore/html/HTMLAnchorElement.h >@@ -28,9 +28,11 @@ > #include "SharedStringHash.h" > #include "URLUtils.h" > #include <wtf/OptionSet.h> >+#include <wtf/Optional.h> > > namespace WebCore { > >+class AdClickAttribution; > class DOMTokenList; > > // Link relation bitmask values. >@@ -95,6 +97,8 @@ private: > > void sendPings(const URL& destinationURL); > >+ Optional<AdClickAttribution> parseAdClickAttribution() const; >+ > void handleClick(Event&); > > enum EventType { >diff --git a/Source/WebCore/loader/AdClickAttribution.h b/Source/WebCore/loader/AdClickAttribution.h >index 7df05ee5eb3ea87f9f9a2640538196e610d54733..fe06a1bf2faea81a0ab55ba266095ac348c61c9c 100644 >--- a/Source/WebCore/loader/AdClickAttribution.h >+++ b/Source/WebCore/loader/AdClickAttribution.h >@@ -37,14 +37,13 @@ class URL; > > namespace WebCore { > >-constexpr unsigned short maxEntropy = 64; >- > class AdClickAttribution { >- WTF_MAKE_NONCOPYABLE(AdClickAttribution); > public: >- using CampaignId = unsigned short; >- using ConversionData = unsigned short; >- using PriorityValue = unsigned short; >+ using CampaignId = uint32_t; >+ using ConversionData = uint32_t; >+ using PriorityValue = uint32_t; >+ >+ static constexpr uint32_t MaxEntropy = 64; > > struct Campaign { > explicit Campaign(CampaignId id) >@@ -54,7 +53,7 @@ public: > > bool isValid() const > { >- return id < maxEntropy; >+ return id < MaxEntropy; > } > > CampaignId id; >@@ -104,7 +103,7 @@ public: > > bool isValid() const > { >- return data < maxEntropy && priority < maxEntropy; >+ return data < MaxEntropy && priority < MaxEntropy; > } > > ConversionData data; >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index c0ae8eb2d5fa6fb49c035aacc3af4e9f28577d4a..103927854cdb7d94c8278451fe5d81883a542c7b 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,15 @@ >+2019-02-03 John Wilander <wilander@apple.com> >+ >+ Parse and handle Ad Click Attribution attributes in HTMLAnchorElement::handleClick() >+ https://bugs.webkit.org/show_bug.cgi?id=194104 >+ <rdar://problem/47649991> >+ >+ Reviewed by Chris Dumez, Daniel Bates, and Darin Adler. >+ >+ * TestWebKitAPI/Tests/WebCore/AdClickAttribution.cpp: >+ (TestWebKitAPI::TEST): >+ Changed numeric types from unsigned short to uint32_t. >+ > 2019-02-02 David Kilzer <ddkilzer@apple.com> > > Leak of NSArray (4.25 Kbytes) in com.apple.WebKit.WebContent running WebKit layout tests on iOS Simulator >diff --git a/Tools/TestWebKitAPI/Tests/WebCore/AdClickAttribution.cpp b/Tools/TestWebKitAPI/Tests/WebCore/AdClickAttribution.cpp >index 57ce28170f566fd939af8c363d7c386040729540..a9ca75d71ef380d52f48c5c01472d8af26083faf 100644 >--- a/Tools/TestWebKitAPI/Tests/WebCore/AdClickAttribution.cpp >+++ b/Tools/TestWebKitAPI/Tests/WebCore/AdClickAttribution.cpp >@@ -33,8 +33,8 @@ using namespace WebCore; > > namespace TestWebKitAPI { > >-constexpr unsigned short min6BitValue { 0 }; >-constexpr unsigned short max6BitValue { 63 }; >+constexpr uint32_t min6BitValue { 0 }; >+constexpr uint32_t max6BitValue { 63 }; > > // Positive test cases. > >@@ -52,8 +52,8 @@ TEST(AdClickAttribution, ValidMinValues) > > TEST(AdClickAttribution, ValidMidValues) > { >- AdClickAttribution attribution(AdClickAttribution::Campaign((unsigned short)12), AdClickAttribution::Source("webkit.org"), AdClickAttribution::Destination("example.com")); >- attribution.setConversion(AdClickAttribution::Conversion((unsigned short)44, AdClickAttribution::Priority((unsigned short)22))); >+ AdClickAttribution attribution(AdClickAttribution::Campaign((uint32_t)12), AdClickAttribution::Source("webkit.org"), AdClickAttribution::Destination("example.com")); >+ attribution.setConversion(AdClickAttribution::Conversion((uint32_t)44, AdClickAttribution::Priority((uint32_t)22))); > > auto attributionURL = attribution.url(); > auto referrerURL = attribution.referrer(); >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 3c0ced349ead9d67f764d7fed4974f95c944cf38..b5c77e9284c453b21de9297b6e325b0d6a6476ac 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,18 @@ >+2019-02-03 John Wilander <wilander@apple.com> >+ >+ Parse and handle Ad Click Attribution attributes in HTMLAnchorElement::handleClick() >+ https://bugs.webkit.org/show_bug.cgi?id=194104 >+ <rdar://problem/47649991> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ This test case makes sure invalid data triggers console warnings. >+ >+ * http/tests/adClickAttribution/anchor-tag-attributes-validation-expected.txt: Added. >+ * http/tests/adClickAttribution/anchor-tag-attributes-validation.html: Added. >+ * platform/ios-wk2/http/tests/adClickAttribution/anchor-tag-attributes-validation-expected.txt: Added. >+ Console line numbers are not emitted when running iOS tests so this -expected.txt file doesn't have them. >+ > 2019-02-03 Wenson Hsieh <wenson_hsieh@apple.com> > > Unable to move selection into editable roots with 0 height >diff --git a/LayoutTests/http/tests/adClickAttribution/anchor-tag-attributes-validation-expected.txt b/LayoutTests/http/tests/adClickAttribution/anchor-tag-attributes-validation-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..311084e6d34ff2348d5dd98a7a4ec29e833e2921 >--- /dev/null >+++ b/LayoutTests/http/tests/adClickAttribution/anchor-tag-attributes-validation-expected.txt >@@ -0,0 +1,33 @@ >+CONSOLE MESSAGE: line 107: adcampaignid must have a non-negative value less than 64 for Ad Click Attribution. >+CONSOLE MESSAGE: line 107: adcampaignid must have a non-negative value less than 64 for Ad Click Attribution. >+CONSOLE MESSAGE: line 107: adcampaignid can not be converted to a non-negative integer which is required for Ad Click Attribution. >+CONSOLE MESSAGE: line 107: adcampaignid can not be converted to a non-negative integer which is required for Ad Click Attribution. >+CONSOLE MESSAGE: line 107: adcampaignid can not be converted to a non-negative integer which is required for Ad Click Attribution. >+CONSOLE MESSAGE: line 107: adddestination could not be converted to a valid HTTP-family URL. >+CONSOLE MESSAGE: line 107: adddestination could not be converted to a valid HTTP-family URL. >+CONSOLE MESSAGE: line 107: adddestination could not be converted to a valid HTTP-family URL. >+CONSOLE MESSAGE: line 107: Both adcampaignid and addestination need to be set for Ad Click Attribution to work. >+CONSOLE MESSAGE: line 107: Both adcampaignid and addestination need to be set for Ad Click Attribution to work. >+Test for validity of ad click attribution attributes on anchor tags. >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+Link1 >+Link2 >+Link3 >+Link4 >+Link5 >+Link6 >+Link7 >+Link8 >+Link9 >+Link10 >+Link11 >+Link12 >+Link13 >+Link14 >+ >+PASS successfullyParsed is true >+ >+TEST COMPLETE >+ >diff --git a/LayoutTests/http/tests/adClickAttribution/anchor-tag-attributes-validation.html b/LayoutTests/http/tests/adClickAttribution/anchor-tag-attributes-validation.html >new file mode 100644 >index 0000000000000000000000000000000000000000..1da45788c7d0c41e02e7224e2e2d9828f0852d13 >--- /dev/null >+++ b/LayoutTests/http/tests/adClickAttribution/anchor-tag-attributes-validation.html >@@ -0,0 +1,79 @@ >+<!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/js-test.js"></script> >+ <script src="/js-test-resources/ui-helper.js"></script> >+</head> >+<body onload="runAllTests()"> >+<div id="description"></div> >+<div id="output"></div><br> >+<div id="console"></div> >+<script> >+ description("Test for validity of ad click attribution attributes on anchor tags."); >+ jsTestIsAsync = true; >+ >+ function createAdClickAttributionAnchorElement(elementID, adCampaignID, adDestination) { >+ let anchorElement = document.createElement("a"); >+ anchorElement.id = elementID; >+ anchorElement.adcampaignid = adCampaignID; >+ anchorElement.addestination = adDestination; >+ anchorElement.href = "#"; >+ anchorElement.innerText = "Link" + currentTest; >+ return anchorElement; >+ } >+ >+ function activateElement(elementID, callback) { >+ 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 () { >+ callback(); >+ }, >+ function () { >+ testFailed("Promise rejected."); >+ finishJSTest(); >+ } >+ ); >+ } >+ >+ let currentTest = 0; >+ function runOneTest(adCampaignID, adDestination, callback) { >+ const currentElementID = "test" + currentTest++; >+ const anchorElement = createAdClickAttributionAnchorElement(currentElementID, adCampaignID, adDestination); >+ output.appendChild(anchorElement); >+ const brElement = document.createElement("br"); >+ output.appendChild(brElement); >+ activateElement(currentElementID, callback); >+ } >+ >+ const validAdCampaignID = "03"; >+ const validAdDestination = "http://webkit.org"; >+ const testCases = [ >+ [ validAdCampaignID, validAdDestination ], >+ [ "100", validAdDestination ], // Too many characters. >+ [ "1", validAdDestination ], // Too few characters. >+ [ "98", validAdDestination ], // Too high value. >+ [ "-1", validAdDestination ], // Negative value. >+ [ "ab", validAdDestination ], // Non-digits. >+ [ "1", validAdDestination ], // Non-ASCII. >+ [ " 1", validAdDestination ], // Leading space. >+ [ "1 ", validAdDestination ], // Trailing space. >+ [ validAdCampaignID, "webkit.org" ], // Missing protocol. >+ [ validAdCampaignID, "://webkit.org" ], // Partially missing protocol. >+ [ validAdCampaignID, "" ], // Non-ASCII characters as destination. >+ [ "", validAdDestination ], // Empty campaign ID. >+ [ validAdCampaignID, "" ] // Empty destination. >+ ]; >+ >+ function runAllTests() { >+ if (currentTest < testCases.length) >+ runOneTest(testCases[currentTest][0], testCases[currentTest][1], runAllTests); >+ else >+ finishJSTest(); >+ } >+</script> >+</body> >+</html> >diff --git a/LayoutTests/platform/ios-wk2/http/tests/adClickAttribution/anchor-tag-attributes-validation-expected.txt b/LayoutTests/platform/ios-wk2/http/tests/adClickAttribution/anchor-tag-attributes-validation-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..2ca10c55eec5190d4f8a48a812925903dacb1182 >--- /dev/null >+++ b/LayoutTests/platform/ios-wk2/http/tests/adClickAttribution/anchor-tag-attributes-validation-expected.txt >@@ -0,0 +1,33 @@ >+CONSOLE MESSAGE: adcampaignid must have a non-negative value less than 64 for Ad Click Attribution. >+CONSOLE MESSAGE: adcampaignid must have a non-negative value less than 64 for Ad Click Attribution. >+CONSOLE MESSAGE: adcampaignid can not be converted to a non-negative integer which is required for Ad Click Attribution. >+CONSOLE MESSAGE: adcampaignid can not be converted to a non-negative integer which is required for Ad Click Attribution. >+CONSOLE MESSAGE: adcampaignid can not be converted to a non-negative integer which is required for Ad Click Attribution. >+CONSOLE MESSAGE: adddestination could not be converted to a valid HTTP-family URL. >+CONSOLE MESSAGE: adddestination could not be converted to a valid HTTP-family URL. >+CONSOLE MESSAGE: adddestination could not be converted to a valid HTTP-family URL. >+CONSOLE MESSAGE: Both adcampaignid and addestination need to be set for Ad Click Attribution to work. >+CONSOLE MESSAGE: Both adcampaignid and addestination need to be set for Ad Click Attribution to work. >+Test for validity of ad click attribution attributes on anchor tags. >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+Link1 >+Link2 >+Link3 >+Link4 >+Link5 >+Link6 >+Link7 >+Link8 >+Link9 >+Link10 >+Link11 >+Link12 >+Link13 >+Link14 >+ >+PASS successfullyParsed is true >+ >+TEST COMPLETE >+
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194104
:
360752
|
360754
|
360783
|
360790
|
360803
|
360816
|
360820
|
360894
|
360900
|
360904
|
361029
|
361031