0

[Code Health Rotation] Spanification of VisitedHashLink

This CL converts the const char* canonical_url and its size into a
std::string_view throughout the stack, from the blink/ layer all the
way up to chrome/ and components/.

This change should be a no-op, since it just changes the way data
is sent through the stack instead of making any logic changes.

More context on Spanification: https://docs.google.com/document/d/1rV6zYT5l5oUeCcF149eTjIG3E4ukecvoUQpiMtB3wk0/edit?resourcekey=0-sHZDaXR51dCRy-3XiTbiGg&tab=t.0

Bug: 1490484
Change-Id: Iaed5ae06bf4bb95202c015af5af14a4f397967ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5169058
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Dibyajyoti Pal <dibyapal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1244847}
This commit is contained in:
Dibyajyoti Pal
2024-01-09 20:36:34 +00:00
committed by Chromium LUCI CQ
parent 38780ad3c5
commit 0ddc941b4c
17 changed files with 64 additions and 53 deletions

@ -5,6 +5,7 @@
#include "android_webview/renderer/aw_content_renderer_client.h"
#include <memory>
#include <string_view>
#include <vector>
#include "android_webview/common/aw_switches.h"
@ -208,9 +209,9 @@ void AwContentRendererClient::PrepareErrorPage(
android_system_error_page::PopulateErrorPageHtml(error, error_html);
}
uint64_t AwContentRendererClient::VisitedLinkHash(const char* canonical_url,
size_t length) {
return visited_link_reader_->ComputeURLFingerprint(canonical_url, length);
uint64_t AwContentRendererClient::VisitedLinkHash(
std::string_view canonical_url) {
return visited_link_reader_->ComputeURLFingerprint(canonical_url);
}
bool AwContentRendererClient::IsLinkVisited(uint64_t link_hash) {

@ -7,6 +7,7 @@
#include <memory>
#include <string>
#include <string_view>
#include "android_webview/common/mojom/render_message_filter.mojom.h"
#include "android_webview/renderer/aw_render_thread_observer.h"
@ -51,7 +52,7 @@ class AwContentRendererClient : public content::ContentRendererClient,
content::mojom::AlternativeErrorPageOverrideInfoPtr
alternative_error_page_info,
std::string* error_html) override;
uint64_t VisitedLinkHash(const char* canonical_url, size_t length) override;
uint64_t VisitedLinkHash(std::string_view canonical_url) override;
bool IsLinkVisited(uint64_t link_hash) override;
void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override;
void GetSupportedKeySystems(media::GetSupportedKeySystemsCB cb) override;

@ -6,6 +6,7 @@
#include <functional>
#include <memory>
#include <string_view>
#include <utility>
#include "base/check_op.h"
@ -1476,10 +1477,10 @@ bool ChromeContentRendererClient::IsPrefetchOnly(
return prerender::NoStatePrefetchHelper::IsPrefetching(render_frame);
}
uint64_t ChromeContentRendererClient::VisitedLinkHash(const char* canonical_url,
size_t length) {
uint64_t ChromeContentRendererClient::VisitedLinkHash(
std::string_view canonical_url) {
return chrome_observer_->visited_link_reader()->ComputeURLFingerprint(
canonical_url, length);
canonical_url);
}
bool ChromeContentRendererClient::IsLinkVisited(uint64_t link_hash) {

@ -11,6 +11,7 @@
#include <memory>
#include <set>
#include <string>
#include <string_view>
#include <vector>
#include "base/gtest_prod_util.h"
@ -150,7 +151,7 @@ class ChromeContentRendererClient
const url::Origin* initiator_origin,
GURL* new_url) override;
bool IsPrefetchOnly(content::RenderFrame* render_frame) override;
uint64_t VisitedLinkHash(const char* canonical_url, size_t length) override;
uint64_t VisitedLinkHash(std::string_view canonical_url) override;
bool IsLinkVisited(uint64_t link_hash) override;
std::unique_ptr<blink::WebPrescientNetworking> CreatePrescientNetworking(
content::RenderFrame* render_frame) override;

@ -6,6 +6,7 @@
#include <memory>
#include <string>
#include <string_view>
#include <utility>
#include "base/functional/bind.h"
@ -44,7 +45,7 @@ class TestChromeContentRendererClient : public ChromeContentRendererClient {
~TestChromeContentRendererClient() override {}
// Since visited_link_reader_ in ChromeContentRenderClient never get
// initiated, overrides VisitedLinkedHash() function to prevent crashing.
uint64_t VisitedLinkHash(const char* canonical_url, size_t length) override {
uint64_t VisitedLinkHash(std::string_view canonical_url) override {
return 0;
}
};

@ -5,6 +5,7 @@
#include "components/safe_browsing/content/renderer/phishing_classifier/phishing_dom_feature_extractor.h"
#include <memory>
#include <string_view>
#include <unordered_map>
#include "base/functional/bind.h"
@ -132,7 +133,7 @@ class TestChromeContentRendererClient : public ChromeContentRendererClient {
~TestChromeContentRendererClient() override {}
// Since visited_link_reader_ in ChromeContentRenderClient never get
// initiated, overrides VisitedLinkedHash() function to prevent crashing.
uint64_t VisitedLinkHash(const char* canonical_url, size_t length) override {
uint64_t VisitedLinkHash(std::string_view canonical_url) override {
return 0;
}
};

@ -9,6 +9,7 @@
#include <algorithm>
#include <memory>
#include <string_view>
#include <utility>
#include "base/files/file_util.h"
@ -328,8 +329,7 @@ VisitedLinkWriter::Hash VisitedLinkWriter::TryToAddURL(const GURL& url) {
if (!url.is_valid())
return null_hash_; // Don't add invalid URLs.
Fingerprint fingerprint =
ComputeURLFingerprint(url.spec().data(), url.spec().size(), salt_);
Fingerprint fingerprint = ComputeURLFingerprint(url.spec(), salt_);
// If the table isn't loaded the table will be rebuilt and after
// that accumulated fingerprints will be applied to the table.
if (table_builder_.get() || table_is_loading_from_file_) {
@ -424,8 +424,7 @@ void VisitedLinkWriter::DeleteURLs(URLIterator* urls) {
if (!url.is_valid())
continue;
Fingerprint fingerprint =
ComputeURLFingerprint(url.spec().data(), url.spec().size(), salt_);
Fingerprint fingerprint = ComputeURLFingerprint(url.spec(), salt_);
deleted_since_rebuild_.insert(fingerprint);
// If the URL was just added and now we're deleting it, it may be in the
@ -450,8 +449,7 @@ void VisitedLinkWriter::DeleteURLs(URLIterator* urls) {
const GURL& url(urls->NextURL());
if (!url.is_valid())
continue;
deleted_fingerprints.insert(
ComputeURLFingerprint(url.spec().data(), url.spec().size(), salt_));
deleted_fingerprints.insert(ComputeURLFingerprint(url.spec(), salt_));
}
DeleteFingerprintsFromCurrentTable(deleted_fingerprints);
}
@ -760,16 +758,14 @@ void VisitedLinkWriter::OnTableLoadComplete(
// Also add anything that was added while we were asynchronously
// loading the table.
for (const GURL& url : added_since_load_) {
Fingerprint fingerprint =
ComputeURLFingerprint(url.spec().data(), url.spec().size(), salt_);
Fingerprint fingerprint = ComputeURLFingerprint(url.spec(), salt_);
AddFingerprint(fingerprint, false);
}
added_since_load_.clear();
// Now handle deletions.
for (const GURL& url : deleted_since_load_) {
Fingerprint fingerprint =
ComputeURLFingerprint(url.spec().data(), url.spec().size(), salt_);
Fingerprint fingerprint = ComputeURLFingerprint(url.spec(), salt_);
DeleteFingerprint(fingerprint, false);
}
deleted_since_load_.clear();
@ -1160,8 +1156,8 @@ void VisitedLinkWriter::TableBuilder::DisownWriter() {
void VisitedLinkWriter::TableBuilder::OnURL(const GURL& url) {
if (!url.is_empty()) {
fingerprints_.push_back(VisitedLinkWriter::ComputeURLFingerprint(
url.spec().data(), url.spec().length(), salt_));
fingerprints_.push_back(
VisitedLinkWriter::ComputeURLFingerprint(url.spec(), salt_));
}
}

@ -7,6 +7,7 @@
#include <string.h> // for memset()
#include <ostream>
#include <string_view>
#include "base/bit_cast.h"
#include "base/check.h"
@ -27,17 +28,18 @@ VisitedLinkCommon::~VisitedLinkCommon() = default;
// FIXME: this uses linear probing, it should be replaced with quadratic
// probing or something better. See VisitedLinkWriter::AddFingerprint
bool VisitedLinkCommon::IsVisited(const char* canonical_url,
size_t url_len) const {
if (url_len == 0)
bool VisitedLinkCommon::IsVisited(std::string_view canonical_url) const {
if (canonical_url.size() == 0) {
return false;
if (!hash_table_ || table_length_ == 0)
}
if (!hash_table_ || table_length_ == 0) {
return false;
return IsVisited(ComputeURLFingerprint(canonical_url, url_len));
}
return IsVisited(ComputeURLFingerprint(canonical_url));
}
bool VisitedLinkCommon::IsVisited(const GURL& url) const {
return IsVisited(url.spec().data(), url.spec().size());
return IsVisited(url.spec());
}
bool VisitedLinkCommon::IsVisited(Fingerprint fingerprint) const {
@ -77,16 +79,15 @@ bool VisitedLinkCommon::IsVisited(Fingerprint fingerprint) const {
// static
VisitedLinkCommon::Fingerprint VisitedLinkCommon::ComputeURLFingerprint(
const char* canonical_url,
size_t url_len,
std::string_view canonical_url,
const uint8_t salt[LINK_SALT_LENGTH]) {
DCHECK(url_len > 0) << "Canonical URLs should not be empty";
DCHECK(canonical_url.size() > 0) << "Canonical URLs should not be empty";
base::MD5Context ctx;
base::MD5Init(&ctx);
base::MD5Update(&ctx, base::StringPiece(reinterpret_cast<const char*>(salt),
LINK_SALT_LENGTH));
base::MD5Update(&ctx, base::StringPiece(canonical_url, url_len));
base::MD5Update(&ctx, canonical_url);
base::MD5Digest digest;
base::MD5Final(&digest, &ctx);

@ -8,6 +8,7 @@
#include <stddef.h>
#include <stdint.h>
#include <string_view>
#include <vector>
#include "base/memory/raw_ptr.h"
@ -66,15 +67,14 @@ class VisitedLinkCommon {
virtual ~VisitedLinkCommon();
// Returns the fingerprint for the given URL.
Fingerprint ComputeURLFingerprint(const char* canonical_url,
size_t url_len) const {
return ComputeURLFingerprint(canonical_url, url_len, salt_);
Fingerprint ComputeURLFingerprint(std::string_view canonical_url) const {
return ComputeURLFingerprint(canonical_url, salt_);
}
// Looks up the given key in the table. The fingerprint for the URL is
// computed if you call one with the string argument. Returns true if found.
// Does not modify the hastable.
bool IsVisited(const char* canonical_url, size_t url_len) const;
bool IsVisited(const std::string_view canonical_url) const;
bool IsVisited(const GURL& url) const;
bool IsVisited(Fingerprint fingerprint) const;
@ -119,8 +119,7 @@ class VisitedLinkCommon {
// pass the salt as a parameter. See the non-static version above if you
// want to use the current class' salt.
static Fingerprint ComputeURLFingerprint(
const char* canonical_url,
size_t url_len,
std::string_view canonical_url,
const uint8_t salt[LINK_SALT_LENGTH]);
// Computes the hash value of the given fingerprint, this is used as a lookup

@ -9,6 +9,7 @@
#include <map>
#include <memory>
#include <string>
#include <string_view>
#include "base/notreached.h"
#include "base/threading/platform_thread.h"
@ -61,8 +62,8 @@ blink::WebSandboxSupport* PpapiBlinkPlatformImpl::GetSandboxSupport() {
#endif
}
uint64_t PpapiBlinkPlatformImpl::VisitedLinkHash(const char* canonical_url,
size_t length) {
uint64_t PpapiBlinkPlatformImpl::VisitedLinkHash(
std::string_view canonical_url) {
NOTREACHED();
return 0;
}

@ -8,6 +8,7 @@
#include <stddef.h>
#include <memory>
#include <string_view>
#include "build/build_config.h"
#include "content/child/blink_platform_impl.h"
@ -28,7 +29,7 @@ class PpapiBlinkPlatformImpl : public BlinkPlatformImpl {
// BlinkPlatformImpl methods:
blink::WebSandboxSupport* GetSandboxSupport() override;
uint64_t VisitedLinkHash(const char* canonical_url, size_t length) override;
uint64_t VisitedLinkHash(std::string_view canonical_url) override;
bool IsLinkVisited(uint64_t link_hash) override;
blink::WebString DefaultLocale() override;

@ -4,6 +4,8 @@
#include "content/public/renderer/content_renderer_client.h"
#include <string_view>
#include "base/command_line.h"
#include "base/task/sequenced_task_runner.h"
#include "base/task/single_thread_task_runner.h"
@ -135,8 +137,8 @@ bool ContentRendererClient::IsPrefetchOnly(RenderFrame* render_frame) {
return false;
}
uint64_t ContentRendererClient::VisitedLinkHash(const char* canonical_url,
size_t length) {
uint64_t ContentRendererClient::VisitedLinkHash(
std::string_view canonical_url) {
return 0;
}

@ -10,6 +10,7 @@
#include <map>
#include <memory>
#include <string>
#include <string_view>
#include <vector>
#include "base/files/file_path.h"
@ -260,7 +261,7 @@ class CONTENT_EXPORT ContentRendererClient {
virtual bool IsPrefetchOnly(RenderFrame* render_frame);
// See blink::Platform.
virtual uint64_t VisitedLinkHash(const char* canonical_url, size_t length);
virtual uint64_t VisitedLinkHash(std::string_view canonical_url);
virtual bool IsLinkVisited(uint64_t link_hash);
// Creates a WebPrescientNetworking instance for |render_frame|. The returned

@ -6,6 +6,7 @@
#include <algorithm>
#include <memory>
#include <string_view>
#include <utility>
#include <vector>
@ -279,9 +280,9 @@ bool RendererBlinkPlatformImpl::sandboxEnabled() {
switches::kSingleProcess);
}
uint64_t RendererBlinkPlatformImpl::VisitedLinkHash(const char* canonical_url,
size_t length) {
return GetContentClient()->renderer()->VisitedLinkHash(canonical_url, length);
uint64_t RendererBlinkPlatformImpl::VisitedLinkHash(
std::string_view canonical_url) {
return GetContentClient()->renderer()->VisitedLinkHash(canonical_url);
}
bool RendererBlinkPlatformImpl::IsLinkVisited(uint64_t link_hash) {

@ -10,6 +10,7 @@
#include <memory>
#include <string>
#include <string_view>
#include "base/containers/id_map.h"
#include "base/memory/raw_ptr.h"
@ -78,7 +79,7 @@ class CONTENT_EXPORT RendererBlinkPlatformImpl : public BlinkPlatformImpl {
// blink::Platform implementation.
blink::WebSandboxSupport* GetSandboxSupport() override;
virtual bool sandboxEnabled();
uint64_t VisitedLinkHash(const char* canonicalURL, size_t length) override;
uint64_t VisitedLinkHash(std::string_view canonical_url) override;
bool IsLinkVisited(uint64_t linkHash) override;
blink::WebString UserAgent() override;
blink::UserAgentMetadata UserAgentMetadata() override;

@ -33,6 +33,7 @@
#include <memory>
#include <string>
#include <string_view>
#include <tuple>
#include <vector>
@ -223,9 +224,7 @@ class BLINK_PLATFORM_EXPORT Platform {
// Returns the hash for the given canonicalized URL for use in visited
// link coloring.
virtual uint64_t VisitedLinkHash(const char* canonical_url, size_t length) {
return 0;
}
virtual uint64_t VisitedLinkHash(std::string_view canonical_url) { return 0; }
// Returns whether the given link hash is in the user's history. The
// hash must have been generated by calling VisitedLinkHash().

@ -30,10 +30,11 @@
#include "third_party/blink/renderer/platform/link_hash.h"
#include <string_view>
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"
#include "third_party/blink/renderer/platform/wtf/text/string_utf8_adaptor.h"
#include "url/url_util.h"
namespace blink {
@ -62,7 +63,9 @@ LinkHash VisitedLinkHash(const KURL& base, const AtomicString& relative) {
url::RawCanonOutput<2048> buffer;
if (!ResolveRelative(base, relative.GetString(), &buffer))
return 0;
return Platform::Current()->VisitedLinkHash(buffer.data(), buffer.length());
return Platform::Current()->VisitedLinkHash(
std::string_view(buffer.data(), buffer.length()));
}
} // namespace blink