Skip to content

Commit

Permalink
Update PublicSuffixStore::topPrivatelyControlledDomain() to take in a…
Browse files Browse the repository at this point in the history
… StringView

https://bugs.webkit.org/show_bug.cgi?id=274480

Reviewed by Darin Adler.

Update PublicSuffixStore::topPrivatelyControlledDomain() to take in a StringView
instead of a String. Most call sites have a StringView and we don't ever need to
construct a String if the host is already in the cache.

* Source/WTF/wtf/HashMap.h:
(WTF::HashMapEnsureTranslatorAdapter::hash):
(WTF::HashMapEnsureTranslatorAdapter::equal):
(WTF::HashMapEnsureTranslatorAdapter::translate):
(WTF::TableTraitsArg>::ensure):
* Source/WTF/wtf/text/StringHash.h:
(WTF::ASCIICaseInsensitiveStringViewHashTranslator::translate):
* Source/WTF/wtf/text/StringImpl.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::urlForBindings const):
* Source/WebCore/page/Quirks.cpp:
(WebCore::isYahooMail):
(WebCore::Quirks::shouldHideSearchFieldResultsButton const):
(WebCore::Quirks::isAmazon const):
(WebCore::Quirks::isGoogleMaps const):
* Source/WebCore/platform/PublicSuffixStore.cpp:
(WebCore::PublicSuffixStore::topPrivatelyControlledDomain const):
* Source/WebCore/platform/PublicSuffixStore.h:
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::resetStateAfterProcessTermination):
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch):
(WebKit::WebProcessProxy::didExceedMemoryFootprintThreshold):
* Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp:
(TestWebKitAPI::TEST(WTF_HashMap, Ensure_Translator)):

Canonical link: https://commits.webkit.org/279092@main
  • Loading branch information
cdumez committed May 22, 2024
1 parent 0a96881 commit 3a59245
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 19 deletions.
27 changes: 23 additions & 4 deletions Source/WTF/wtf/HashMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ class HashMap final {
MappedTakeType take(iterator);
MappedTakeType takeFirst();

// An alternate version of find() that finds the object by hashing and comparing
// with some other type, to avoid the cost of type conversion. HashTranslator
// must have the following function members:
// Alternate versions of find() / contains() / get() / remove() that find the object
// by hashing and comparing with some other type, to avoid the cost of type conversion.
// HashTranslator must have the following function members:
// static unsigned hash(const T&);
// static bool equal(const ValueType&, const T&);
template<typename HashTranslator, typename T> iterator find(const T&);
Expand All @@ -180,13 +180,14 @@ class HashMap final {
template<typename HashTranslator, typename T> MappedPeekType inlineGet(const T&) const;
template<typename HashTranslator, typename T> bool remove(const T&);

// An alternate version of add() that finds the object by hashing and comparing
// Alternate versions of add() / ensure() that find the object by hashing and comparing
// with some other type, to avoid the cost of type conversion if the object is already
// in the table. HashTranslator must have the following function members:
// static unsigned hash(const T&);
// static bool equal(const ValueType&, const T&);
// static translate(ValueType&, const T&, unsigned hashCode);
template<typename HashTranslator, typename K, typename V> AddResult add(K&&, V&&);
template<typename HashTranslator, typename K, typename Functor> AddResult ensure(K&&, Functor&&);

// Overloads for smart pointer keys that take the raw pointer type as the parameter.
template<typename K = KeyType> typename std::enable_if<IsSmartPtr<K>::value, iterator>::type find(std::add_const_t<typename GetPtrHelper<K>::UnderlyingType>*);
Expand Down Expand Up @@ -268,6 +269,17 @@ struct HashMapTranslatorAdapter {
}
};

template<typename ValueTraits, typename Translator>
struct HashMapEnsureTranslatorAdapter {
template<typename T> static unsigned hash(const T& key) { return Translator::hash(key); }
template<typename T, typename U> static bool equal(const T& a, const U& b) { return Translator::equal(a, b); }
template<typename T, typename U, typename Functor> static void translate(T& location, U&& key, Functor&& functor, unsigned hashCode)
{
Translator::translate(location.key, key, hashCode);
location.value = functor();
}
};

template<typename T, typename U, typename V, typename W, typename X, typename Y>
inline void HashMap<T, U, V, W, X, Y>::swap(HashMap& other)
{
Expand Down Expand Up @@ -432,6 +444,13 @@ auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg, TableTra
return inlineSet(WTFMove(key), std::forward<T>(mapped));
}

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg, typename TableTraitsArg>
template<typename HashTranslator, typename K, typename Functor>
auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg, TableTraitsArg>::ensure(K&& key, Functor&& functor) -> AddResult
{
return m_impl.template addPassingHashCode<HashMapEnsureTranslatorAdapter<KeyValuePairTraits, HashTranslator>>(std::forward<K>(key), std::forward<Functor>(functor));
}

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg, typename TableTraitsArg>
template<typename HashTranslator, typename K, typename V>
auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg, TableTraitsArg>::add(K&& key, V&& value) -> AddResult
Expand Down
5 changes: 5 additions & 0 deletions Source/WTF/wtf/text/StringHash.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,11 @@ namespace WTF {
{
return equalIgnoringASCIICaseCommon(a, b);
}

static void translate(String& location, StringView view, unsigned)
{
location = view.toString();
}
};

struct HashTranslatorASCIILiteral {
Expand Down
2 changes: 2 additions & 0 deletions Source/WTF/wtf/text/StringImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ namespace WTF {
class SymbolImpl;
class SymbolRegistry;

struct ASCIICaseInsensitiveStringViewHashTranslator;
struct HashedUTF8CharactersTranslator;
struct HashTranslatorASCIILiteral;
struct LCharBufferTranslator;
Expand Down Expand Up @@ -185,6 +186,7 @@ class StringImpl : private StringImplShape {
friend class SymbolImpl;
friend class ExternalStringImpl;

friend struct WTF::ASCIICaseInsensitiveStringViewHashTranslator;
friend struct WTF::HashedUTF8CharactersTranslator;
friend struct WTF::HashTranslatorASCIILiteral;
friend struct WTF::LCharBufferTranslator;
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/dom/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4108,8 +4108,8 @@ const URL& Document::urlForBindings() const

auto areSameSiteIgnoringPublicSuffix = [](StringView domain, StringView otherDomain) {
auto& publicSuffixStore = PublicSuffixStore::singleton();
auto domainString = publicSuffixStore.topPrivatelyControlledDomain(domain.toStringWithoutCopying());
auto otherDomainString = publicSuffixStore.topPrivatelyControlledDomain(otherDomain.toStringWithoutCopying());
auto domainString = publicSuffixStore.topPrivatelyControlledDomain(domain);
auto otherDomainString = publicSuffixStore.topPrivatelyControlledDomain(otherDomain);
auto substringToSeparator = [](const String& string) -> String {
auto indexOfFirstSeparator = string.find('.');
if (indexOfFirstSeparator == notFound)
Expand Down
8 changes: 4 additions & 4 deletions Source/WebCore/page/Quirks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ static HashMap<RegistrableDomain, String>& updatableStorageAccessUserAgentString
static inline bool isYahooMail(Document& document)
{
auto host = document.topDocument().url().host();
return host.startsWith("mail."_s) && PublicSuffixStore::singleton().topPrivatelyControlledDomain(host.toString()).startsWith("yahoo."_s);
return host.startsWith("mail."_s) && PublicSuffixStore::singleton().topPrivatelyControlledDomain(host).startsWith("yahoo."_s);
}
#endif

Expand Down Expand Up @@ -242,7 +242,7 @@ bool Quirks::shouldHideSearchFieldResultsButton() const
if (!needsQuirks())
return false;

if (PublicSuffixStore::singleton().topPrivatelyControlledDomain(m_document->topDocument().url().host().toString()).startsWith("google."_s))
if (PublicSuffixStore::singleton().topPrivatelyControlledDomain(m_document->topDocument().url().host()).startsWith("google."_s))
return true;
#endif
return false;
Expand Down Expand Up @@ -419,13 +419,13 @@ bool Quirks::shouldDisableElementFullscreenQuirk() const
#if ENABLE(TOUCH_EVENTS)
bool Quirks::isAmazon() const
{
return PublicSuffixStore::singleton().topPrivatelyControlledDomain(m_document->topDocument().url().host().toString()).startsWith("amazon."_s);
return PublicSuffixStore::singleton().topPrivatelyControlledDomain(m_document->topDocument().url().host()).startsWith("amazon."_s);
}

bool Quirks::isGoogleMaps() const
{
auto& url = m_document->topDocument().url();
return PublicSuffixStore::singleton().topPrivatelyControlledDomain(url.host().toString()).startsWith("google."_s) && startsWithLettersIgnoringASCIICase(url.path(), "/maps/"_s);
return PublicSuffixStore::singleton().topPrivatelyControlledDomain(url.host()).startsWith("google."_s) && startsWithLettersIgnoringASCIICase(url.path(), "/maps/"_s);
}

// rdar://49124313
Expand Down
9 changes: 4 additions & 5 deletions Source/WebCore/platform/PublicSuffixStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,18 @@ String PublicSuffixStore::publicSuffix(const URL& url) const
return { };
}

String PublicSuffixStore::topPrivatelyControlledDomain(const String& host) const
String PublicSuffixStore::topPrivatelyControlledDomain(StringView host) const
{
// FIXME: if host is a URL, we could drop these checks.
if (host.isEmpty())
return { };

if (!host.containsOnlyASCII())
return host;
return host.toString();

Locker locker { m_HostTopPrivatelyControlledDomainCacheLock };
auto hostCopy = crossThreadCopy(host);
auto result = m_hostTopPrivatelyControlledDomainCache.ensure(hostCopy, [&] {
const auto lowercaseHost = hostCopy.convertToASCIILowercase();
auto result = m_hostTopPrivatelyControlledDomainCache.ensure<ASCIICaseInsensitiveStringViewHashTranslator>(host, [&] {
const auto lowercaseHost = host.convertToASCIILowercase();
if (lowercaseHost == "localhost"_s || URL::hostIsIPAddress(lowercaseHost))
return lowercaseHost;

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/PublicSuffixStore.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class PublicSuffixStore {
// https://url.spec.whatwg.org/#host-public-suffix
WEBCORE_EXPORT bool isPublicSuffix(StringView domain) const;
WEBCORE_EXPORT String publicSuffix(const URL&) const;
WEBCORE_EXPORT String topPrivatelyControlledDomain(const String& host) const;
WEBCORE_EXPORT String topPrivatelyControlledDomain(StringView host) const;
WEBCORE_EXPORT void clearHostTopPrivatelyControlledDomainCache();

#if PLATFORM(COCOA)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9740,7 +9740,7 @@ void WebPageProxy::resetStateAfterProcessTermination(ProcessTerminationReason re

#if PLATFORM(IOS_FAMILY)
if (m_process->isUnderMemoryPressure()) {
auto domain = WebCore::PublicSuffixStore::singleton().topPrivatelyControlledDomain(URL({ }, currentURL()).host().toString());
auto domain = WebCore::PublicSuffixStore::singleton().topPrivatelyControlledDomain(URL({ }, currentURL()).host());
if (!domain.isEmpty())
logDiagnosticMessageWithEnhancedPrivacy(WebCore::DiagnosticLoggingKeys::domainCausingJetsamKey(), domain, WebCore::ShouldSample::No);
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/WebProcessProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,7 @@ void WebProcessProxy::processDidTerminateOrFailedToLaunch(ProcessTerminationReas
// FIXME: Perhaps this should consider ProcessTerminationReasons ExceededMemoryLimit, ExceededCPULimit, Unresponsive as well.
if (pages.size() == 1 && reason == ProcessTerminationReason::Crash) {
auto& page = pages[0];
auto domain = PublicSuffixStore::singleton().topPrivatelyControlledDomain(URL({ }, page->currentURL()).host().toString());
auto domain = PublicSuffixStore::singleton().topPrivatelyControlledDomain(URL({ }, page->currentURL()).host());
if (!domain.isEmpty())
page->logDiagnosticMessageWithEnhancedPrivacy(WebCore::DiagnosticLoggingKeys::domainCausingCrashKey(), domain, WebCore::ShouldSample::No);
}
Expand Down Expand Up @@ -2015,7 +2015,7 @@ void WebProcessProxy::didExceedMemoryFootprintThreshold(size_t footprint)
bool hasAllowedToRunInTheBackgroundActivity = false;

for (auto& page : this->pages()) {
auto pageDomain = PublicSuffixStore::singleton().topPrivatelyControlledDomain(URL({ }, page->currentURL()).host().toString());
auto pageDomain = PublicSuffixStore::singleton().topPrivatelyControlledDomain(URL({ }, page->currentURL()).host());
if (domain.isEmpty())
domain = WTFMove(pageDomain);
else if (domain != pageDomain)
Expand Down
24 changes: 24 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,30 @@ TEST(WTF_HashMap, Clear_Reenter)
EXPECT_TRUE(map.isEmpty());
}

TEST(WTF_HashMap, Ensure_Translator)
{
HashMap<String, unsigned> map;
auto addResult = map.ensure<StringViewHashTranslator>(StringView { "foo"_s }, [] { return 1u; });
EXPECT_TRUE(addResult.isNewEntry);
EXPECT_TRUE(map.contains<StringViewHashTranslator>(StringView { "foo"_s }));
EXPECT_EQ(map.size(), 1u);
unsigned existingValue = map.get<StringViewHashTranslator>(StringView { "foo"_s });
EXPECT_EQ(existingValue, 1u);
existingValue = map.get<StringViewHashTranslator>("foo"_str);
EXPECT_EQ(existingValue, 1u);
addResult = map.ensure<StringViewHashTranslator>(StringView { "foo"_s }, [] {
EXPECT_TRUE(false);
return 2u;
});
EXPECT_FALSE(addResult.isNewEntry);
EXPECT_EQ(map.size(), 1u);
existingValue = map.get<StringViewHashTranslator>("foo"_str);
EXPECT_EQ(existingValue, 1u);
bool didRemove = map.remove<StringViewHashTranslator>(StringView { "foo"_s });
EXPECT_TRUE(didRemove);
EXPECT_EQ(map.size(), 0u);
}

TEST(WTF_HashMap, GetOptional)
{
{
Expand Down

0 comments on commit 3a59245

Please sign in to comment.