Commit d553d20c authored by Etienne Pierre-doray's avatar Etienne Pierre-doray Committed by Chromium LUCI CQ

[jank]: Add lookalike url trace events and uma histogram.

We're seeing jank in LookalikeUrlService::OnFetchEngagedSites from uma
sampling profiler. There's a few possible culprits:
- First call to GetDomainInfo causes icu loading
- url_formatter::GetSkeletons is expensive and called on many urls
- IsEngagementAtLeast is expensive and called on many urls

To better understand the problem, this CL adds relevant trace events and
records count of engaged sites as UMA histogram.

Bug: 1135279
Change-Id: Ie6ee4ed2dbeeed26e4bbdd72726a0ad48e9bc4bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2615322Reviewed-by: default avatarEtienne Bergeron <etienneb@chromium.org>
Reviewed-by: default avatarWeilun Shi <sweilun@chromium.org>
Reviewed-by: default avatarJoe DeBlasio <jdeblasio@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844109}
parent c553f203
......@@ -13,6 +13,7 @@
#include "base/memory/singleton.h"
#include "base/task/post_task.h"
#include "base/time/default_clock.h"
#include "base/trace_event/trace_event.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/engagement/site_engagement_service_factory.h"
#include "chrome/browser/profiles/incognito_helpers.h"
......@@ -75,11 +76,15 @@ class LookalikeUrlServiceFactory : public BrowserContextKeyedServiceFactory {
std::vector<DomainInfo> UpdateEngagedSitesOnWorkerThread(
base::Time now,
scoped_refptr<HostContentSettingsMap> map) {
TRACE_EVENT0("navigation",
"LookalikeUrlService UpdateEngagedSitesOnWorkerThread");
std::vector<DomainInfo> new_engaged_sites;
auto details =
site_engagement::SiteEngagementService::GetAllDetailsInBackground(now,
map);
TRACE_EVENT1("navigation", "LookalikeUrlService SiteEngagementService",
"site_count", details.size());
for (const site_engagement::mojom::SiteEngagementDetails& detail : details) {
if (!detail.origin.SchemeIsHTTPOrHTTPS()) {
continue;
......@@ -121,6 +126,7 @@ bool LookalikeUrlService::EngagedSitesNeedUpdating() const {
void LookalikeUrlService::ForceUpdateEngagedSites(
EngagedSitesCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
TRACE_EVENT0("navigation", "LookalikeUrlService::ForceUpdateEngagedSites");
// Queue an update on a worker thread if necessary.
if (!update_in_progress_) {
......@@ -156,7 +162,9 @@ void LookalikeUrlService::SetClockForTesting(base::Clock* clock) {
void LookalikeUrlService::OnUpdateEngagedSitesCompleted(
std::vector<DomainInfo> new_engaged_sites) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(update_in_progress_);
TRACE_EVENT0("navigation",
"LookalikeUrlService::OnUpdateEngagedSitesCompleted");
engaged_sites_.swap(new_engaged_sites);
last_engagement_fetch_time_ = clock_->Now();
update_in_progress_ = false;
......
......@@ -23,6 +23,7 @@
#include "base/task/post_task.h"
#include "base/task/thread_pool.h"
#include "base/time/default_clock.h"
#include "base/trace_event/trace_event.h"
#include "base/values.h"
#include "components/lookalikes/core/features.h"
#include "components/security_interstitials/core/pref_names.h"
......@@ -381,6 +382,7 @@ DomainInfo::~DomainInfo() = default;
DomainInfo::DomainInfo(const DomainInfo&) = default;
DomainInfo GetDomainInfo(const std::string& hostname) {
TRACE_EVENT0("navigation", "GetDomainInfo");
if (net::HostStringIsLocalhost(hostname) ||
net::IsHostnameNonUnique(hostname)) {
return DomainInfo(std::string(), std::string(), std::string(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment