[Merge-87][SH-Blink] Adding histograms for Shared Highlights.
Adding histograms to text fragment selector generation logic. Also
adding a histogram that indicates if the origin of a link with text
fragment is a known search engine.
For finding out if a given url is a known search engine,
I am separating template_url_prepopulate_data::GetEngineType and deps
into a separate file to use it from blink/renderer/platform.
Otherwise, template_url_prepopulate_data pulls deps that are also
pulled by deps which somehow triggers
AddressSanitizer: odr-violation(one definition rule) on some targets.
See https://ci.chromium.org/p/chromium/builders/try/linux-libfuzzer-asan-rel/569669
Unit tests for histograms in in a followup.
(cherry picked from commit 2fd5742a51183821b2c7fbe139b962c40e8b99c8)
Change-Id: I9d496b883f32041b5bd612b0cf8e447cc79fa68e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2432674
Commit-Queue: Gayane Petrosyan <[email protected]>
Reviewed-by: manuk hovanesian <[email protected]>
Reviewed-by: Dominic Battré <[email protected]>
Reviewed-by: Robert Kaplow <[email protected]>
Reviewed-by: David Bokan <[email protected]>
Reviewed-by: sebsg <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#813242}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2456948
Reviewed-by: Gayane Petrosyan <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#99}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/chrome/browser/prefs/pref_metrics_service.cc b/chrome/browser/prefs/pref_metrics_service.cc
index a6fb962b4..3d900e3c 100644
--- a/chrome/browser/prefs/pref_metrics_service.cc
+++ b/chrome/browser/prefs/pref_metrics_service.cc
@@ -19,7 +19,7 @@
#include "chrome/common/url_constants.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/prefs/pref_service.h"
-#include "components/search_engines/template_url_prepopulate_data.h"
+#include "components/search_engines/search_engine_utils.h"
#include "url/gurl.h"
PrefMetricsService::PrefMetricsService(Profile* profile)
@@ -48,10 +48,9 @@
// pages, e.g. plus.google.com).
if (!homepage_is_ntp) {
if (homepage_url.is_valid()) {
- UMA_HISTOGRAM_ENUMERATION(
- "Settings.HomePageEngineType",
- TemplateURLPrepopulateData::GetEngineType(homepage_url),
- SEARCH_ENGINE_MAX);
+ UMA_HISTOGRAM_ENUMERATION("Settings.HomePageEngineType",
+ SearchEngineUtils::GetEngineType(homepage_url),
+ SEARCH_ENGINE_MAX);
}
}
}
@@ -84,10 +83,9 @@
if (url_list->GetString(i, &url_text)) {
GURL start_url(url_text);
if (start_url.is_valid()) {
- UMA_HISTOGRAM_ENUMERATION(
- "Settings.StartupPageEngineTypes",
- TemplateURLPrepopulateData::GetEngineType(start_url),
- SEARCH_ENGINE_MAX);
+ UMA_HISTOGRAM_ENUMERATION("Settings.StartupPageEngineTypes",
+ SearchEngineUtils::GetEngineType(start_url),
+ SEARCH_ENGINE_MAX);
}
}
}
@@ -100,10 +98,9 @@
for (size_t i = 0; i < startup_tabs.size(); ++i) {
GURL start_url(startup_tabs.at(i).url);
if (start_url.is_valid()) {
- UMA_HISTOGRAM_ENUMERATION(
- "Settings.PinnedTabEngineTypes",
- TemplateURLPrepopulateData::GetEngineType(start_url),
- SEARCH_ENGINE_MAX);
+ UMA_HISTOGRAM_ENUMERATION("Settings.PinnedTabEngineTypes",
+ SearchEngineUtils::GetEngineType(start_url),
+ SEARCH_ENGINE_MAX);
}
}
#endif
diff --git a/components/omnibox/browser/autocomplete_match.cc b/components/omnibox/browser/autocomplete_match.cc
index b7a2bf5..58132b9e 100644
--- a/components/omnibox/browser/autocomplete_match.cc
+++ b/components/omnibox/browser/autocomplete_match.cc
@@ -28,8 +28,8 @@
#include "components/omnibox/browser/omnibox_pedal.h"
#include "components/omnibox/browser/suggestion_answer.h"
#include "components/omnibox/common/omnibox_features.h"
+#include "components/search_engines/search_engine_utils.h"
#include "components/search_engines/template_url.h"
-#include "components/search_engines/template_url_prepopulate_data.h"
#include "components/search_engines/template_url_service.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "ui/gfx/vector_icon_types.h"
@@ -842,7 +842,7 @@
if (template_url) {
SearchEngineType search_engine_type =
match.destination_url.is_valid()
- ? TemplateURLPrepopulateData::GetEngineType(match.destination_url)
+ ? SearchEngineUtils::GetEngineType(match.destination_url)
: SEARCH_ENGINE_OTHER;
UMA_HISTOGRAM_ENUMERATION("Omnibox.SearchEngineType", search_engine_type,
SEARCH_ENGINE_MAX);
diff --git a/components/search_engines/BUILD.gn b/components/search_engines/BUILD.gn
index 1f7e2e69..eb66a2d9 100644
--- a/components/search_engines/BUILD.gn
+++ b/components/search_engines/BUILD.gn
@@ -52,6 +52,7 @@
public_deps = [
":prepopulated_engines",
":search_engine_type",
+ ":search_engine_utils",
"//base",
"//components/google/core/common",
"//components/keyed_service/core",
@@ -100,6 +101,28 @@
}
}
+source_set("search_engine_utils") {
+ sources = [
+ "search_engine_utils.cc",
+ "search_engine_utils.h",
+ ]
+
+ public_deps = [
+ ":prepopulated_engines",
+ ":search_engine_type",
+ "//components/google/core/common",
+ ]
+
+ deps = [ "//url" ]
+
+ if (is_android) {
+ deps += [
+ "//components/search_engines/android:jni_headers",
+ "//url:gurl_android",
+ ]
+ }
+}
+
source_set("search_engine_type") {
sources = [ "search_engine_type.h" ]
}
diff --git a/components/search_engines/search_engine_utils.cc b/components/search_engines/search_engine_utils.cc
new file mode 100644
index 0000000..ecbd1561
--- /dev/null
+++ b/components/search_engines/search_engine_utils.cc
@@ -0,0 +1,60 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/search_engines/search_engine_utils.h"
+
+#include "components/google/core/common/google_util.h"
+#include "components/search_engines/prepopulated_engines.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "url/gurl.h"
+
+namespace SearchEngineUtils {
+
+namespace {
+
+bool SameDomain(const GURL& given_url, const GURL& prepopulated_url) {
+ return prepopulated_url.is_valid() &&
+ net::registry_controlled_domains::SameDomainOrHost(
+ given_url, prepopulated_url,
+ net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
+}
+
+} // namespace
+
+// Global functions -----------------------------------------------------------
+
+SearchEngineType GetEngineType(const GURL& url) {
+ DCHECK(url.is_valid());
+
+ // Check using TLD+1s, in order to more aggressively match search engine types
+ // for data imported from other browsers.
+ //
+ // First special-case Google, because the prepopulate URL for it will not
+ // convert to a GURL and thus won't have an origin. Instead see if the
+ // incoming URL's host is "[*.]google.<TLD>".
+ if (google_util::IsGoogleHostname(url.host(),
+ google_util::DISALLOW_SUBDOMAIN))
+ return TemplateURLPrepopulateData::google.type;
+
+ // Now check the rest of the prepopulate data.
+ for (size_t i = 0; i < TemplateURLPrepopulateData::kAllEnginesLength; ++i) {
+ // First check the main search URL.
+ if (SameDomain(
+ url, GURL(TemplateURLPrepopulateData::kAllEngines[i]->search_url)))
+ return TemplateURLPrepopulateData::kAllEngines[i]->type;
+
+ // Then check the alternate URLs.
+ for (size_t j = 0;
+ j < TemplateURLPrepopulateData::kAllEngines[i]->alternate_urls_size;
+ ++j) {
+ if (SameDomain(url, GURL(TemplateURLPrepopulateData::kAllEngines[i]
+ ->alternate_urls[j])))
+ return TemplateURLPrepopulateData::kAllEngines[i]->type;
+ }
+ }
+
+ return SEARCH_ENGINE_OTHER;
+}
+
+} // namespace SearchEngineUtils
diff --git a/components/search_engines/search_engine_utils.h b/components/search_engines/search_engine_utils.h
new file mode 100644
index 0000000..83e0dd4
--- /dev/null
+++ b/components/search_engines/search_engine_utils.h
@@ -0,0 +1,20 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_SEARCH_ENGINES_SEARCH_ENGINE_UTILS_H_
+#define COMPONENTS_SEARCH_ENGINES_SEARCH_ENGINE_UTILS_H_
+
+#include "components/search_engines/search_engine_type.h"
+
+class GURL;
+
+namespace SearchEngineUtils {
+
+// Like the above, but takes a GURL which is expected to represent a search URL.
+// This may be called on any thread.
+SearchEngineType GetEngineType(const GURL& url);
+
+} // namespace SearchEngineUtils
+
+#endif // COMPONENTS_SEARCH_ENGINES_SEARCH_ENGINE_UTILS_H_
diff --git a/components/search_engines/template_url.cc b/components/search_engines/template_url.cc
index e881a98..2a4c441 100644
--- a/components/search_engines/template_url.cc
+++ b/components/search_engines/template_url.cc
@@ -26,9 +26,9 @@
#include "base/trace_event/memory_usage_estimator.h"
#include "build/build_config.h"
#include "components/google/core/common/google_util.h"
+#include "components/search_engines/search_engine_utils.h"
#include "components/search_engines/search_engines_switches.h"
#include "components/search_engines/search_terms_data.h"
-#include "components/search_engines/template_url_prepopulate_data.h"
#include "components/url_formatter/url_formatter.h"
#include "google_apis/google_api_keys.h"
#include "net/base/escape.h"
@@ -1410,8 +1410,8 @@
const SearchTermsData& search_terms_data) const {
if (engine_type_ == SEARCH_ENGINE_UNKNOWN) {
const GURL url = GenerateSearchURL(search_terms_data);
- engine_type_ = url.is_valid() ?
- TemplateURLPrepopulateData::GetEngineType(url) : SEARCH_ENGINE_OTHER;
+ engine_type_ = url.is_valid() ? SearchEngineUtils::GetEngineType(url)
+ : SEARCH_ENGINE_OTHER;
DCHECK_NE(SEARCH_ENGINE_UNKNOWN, engine_type_);
}
return engine_type_;
diff --git a/components/search_engines/template_url_prepopulate_data.cc b/components/search_engines/template_url_prepopulate_data.cc
index 60c60a4d..c1368c6 100644
--- a/components/search_engines/template_url_prepopulate_data.cc
+++ b/components/search_engines/template_url_prepopulate_data.cc
@@ -8,15 +8,12 @@
#include "base/stl_util.h"
#include "build/build_config.h"
#include "components/country_codes/country_codes.h"
-#include "components/google/core/common/google_util.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/search_engines/prepopulated_engines.h"
#include "components/search_engines/search_engines_pref_names.h"
#include "components/search_engines/template_url_data.h"
#include "components/search_engines/template_url_data_util.h"
-#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
-#include "url/gurl.h"
namespace TemplateURLPrepopulateData {
@@ -1356,13 +1353,6 @@
return t_urls;
}
-bool SameDomain(const GURL& given_url, const GURL& prepopulated_url) {
- return prepopulated_url.is_valid() &&
- net::registry_controlled_domains::SameDomainOrHost(
- given_url, prepopulated_url,
- net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
-}
-
} // namespace
// Global functions -----------------------------------------------------------
@@ -1452,33 +1442,4 @@
: nullptr;
}
-SearchEngineType GetEngineType(const GURL& url) {
- DCHECK(url.is_valid());
-
- // Check using TLD+1s, in order to more aggressively match search engine types
- // for data imported from other browsers.
- //
- // First special-case Google, because the prepopulate URL for it will not
- // convert to a GURL and thus won't have an origin. Instead see if the
- // incoming URL's host is "[*.]google.<TLD>".
- if (google_util::IsGoogleHostname(url.host(),
- google_util::DISALLOW_SUBDOMAIN))
- return google.type;
-
- // Now check the rest of the prepopulate data.
- for (size_t i = 0; i < kAllEnginesLength; ++i) {
- // First check the main search URL.
- if (SameDomain(url, GURL(kAllEngines[i]->search_url)))
- return kAllEngines[i]->type;
-
- // Then check the alternate URLs.
- for (size_t j = 0; j < kAllEngines[i]->alternate_urls_size; ++j) {
- if (SameDomain(url, GURL(kAllEngines[i]->alternate_urls[j])))
- return kAllEngines[i]->type;
- }
- }
-
- return SEARCH_ENGINE_OTHER;
-}
-
} // namespace TemplateURLPrepopulateData
diff --git a/components/search_engines/template_url_prepopulate_data.h b/components/search_engines/template_url_prepopulate_data.h
index 159263ac..682060b 100644
--- a/components/search_engines/template_url_prepopulate_data.h
+++ b/components/search_engines/template_url_prepopulate_data.h
@@ -11,9 +11,6 @@
#include <string>
#include <vector>
-#include "components/search_engines/search_engine_type.h"
-
-class GURL;
class PrefService;
struct TemplateURLData;
@@ -65,10 +62,6 @@
std::unique_ptr<TemplateURLData> GetPrepopulatedDefaultSearch(
PrefService* prefs);
-// Like the above, but takes a GURL which is expected to represent a search URL.
-// This may be called on any thread.
-SearchEngineType GetEngineType(const GURL& url);
-
} // namespace TemplateURLPrepopulateData
#endif // COMPONENTS_SEARCH_ENGINES_TEMPLATE_URL_PREPOPULATE_DATA_H_
diff --git a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc
index f386226e..ad820ba 100644
--- a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc
+++ b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc
@@ -21,6 +21,7 @@
#include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector.h"
#include "third_party/blink/renderer/core/scroll/scroll_alignment.h"
#include "third_party/blink/renderer/core/scroll/scrollable_area.h"
+#include "third_party/blink/renderer/platform/search_engine_utils.h"
namespace blink {
@@ -377,6 +378,7 @@
did_find_match_ = true;
if (first_match_needs_scroll_) {
+ metrics_->SetSearchEngineSource(HasSearchEngineSource());
first_match_needs_scroll_ = false;
PhysicalRect bounding_box(ComputeTextRect(range));
@@ -498,4 +500,13 @@
metrics_->SetTickClockForTesting(tick_clock);
}
+bool TextFragmentAnchor::HasSearchEngineSource() {
+ AtomicString referrer = frame_->GetDocument()->referrer();
+ // TODO(crbug.com/1133823): Add test case for valid referrer.
+ if (!referrer)
+ return false;
+
+ return IsKnownSearchEngine(referrer);
+}
+
} // namespace blink
diff --git a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h
index 22cf488..ee4d075 100644
--- a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h
+++ b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.h
@@ -95,6 +95,8 @@
void FireBeforeMatchEvent(Element* element);
+ bool HasSearchEngineSource();
+
Vector<TextFragmentFinder> text_fragment_finders_;
Member<LocalFrame> frame_;
diff --git a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.cc b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.cc
index 10fdb64..e100957 100644
--- a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.cc
+++ b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.cc
@@ -150,7 +150,7 @@
}
UMA_HISTOGRAM_ENUMERATION("TextFragmentAnchor.Parameters",
- GetParametersForMatch(match));
+ GetParametersForSelector(match.selector));
}
UMA_HISTOGRAM_BOOLEAN("TextFragmentAnchor.AmbiguousMatch", ambiguous_match_);
@@ -182,6 +182,8 @@
time_to_scroll_into_view.InMilliseconds());
}
+ UMA_HISTOGRAM_BOOLEAN("TextFragmentAnchor.LinkOpenSource",
+ has_search_engine_source_);
#ifndef NDEBUG
metrics_reported_ = true;
#endif
@@ -200,26 +202,26 @@
}
TextFragmentAnchorMetrics::TextFragmentAnchorParameters
-TextFragmentAnchorMetrics::GetParametersForMatch(const Match& match) {
+TextFragmentAnchorMetrics::GetParametersForSelector(
+ const TextFragmentSelector& selector) {
TextFragmentAnchorParameters parameters =
TextFragmentAnchorParameters::kUnknown;
- if (match.selector.Type() == TextFragmentSelector::SelectorType::kExact) {
- if (match.selector.Prefix().length() && match.selector.Suffix().length())
+ if (selector.Type() == TextFragmentSelector::SelectorType::kExact) {
+ if (selector.Prefix().length() && selector.Suffix().length())
parameters = TextFragmentAnchorParameters::kExactTextWithContext;
- else if (match.selector.Prefix().length())
+ else if (selector.Prefix().length())
parameters = TextFragmentAnchorParameters::kExactTextWithPrefix;
- else if (match.selector.Suffix().length())
+ else if (selector.Suffix().length())
parameters = TextFragmentAnchorParameters::kExactTextWithSuffix;
else
parameters = TextFragmentAnchorParameters::kExactText;
- } else if (match.selector.Type() ==
- TextFragmentSelector::SelectorType::kRange) {
- if (match.selector.Prefix().length() && match.selector.Suffix().length())
+ } else if (selector.Type() == TextFragmentSelector::SelectorType::kRange) {
+ if (selector.Prefix().length() && selector.Suffix().length())
parameters = TextFragmentAnchorParameters::kTextRangeWithContext;
- else if (match.selector.Prefix().length())
+ else if (selector.Prefix().length())
parameters = TextFragmentAnchorParameters::kTextRangeWithPrefix;
- else if (match.selector.Suffix().length())
+ else if (selector.Suffix().length())
parameters = TextFragmentAnchorParameters::kTextRangeWithSuffix;
else
parameters = TextFragmentAnchorParameters::kTextRange;
@@ -233,4 +235,9 @@
tick_clock_ = tick_clock;
}
+void TextFragmentAnchorMetrics::SetSearchEngineSource(
+ bool has_search_engine_source) {
+ has_search_engine_source_ = has_search_engine_source;
+}
+
} // namespace blink
diff --git a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.h b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.h
index c7b177d1..616beb85 100644
--- a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.h
+++ b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.h
@@ -42,6 +42,9 @@
explicit TextFragmentAnchorMetrics(Document* document);
+ static TextFragmentAnchorParameters GetParametersForSelector(
+ const TextFragmentSelector& selector);
+
void DidCreateAnchor(int selector_count, int directive_length);
void DidFindMatch(Match match);
@@ -65,11 +68,11 @@
void SetTickClockForTesting(const base::TickClock* tick_clock);
+ void SetSearchEngineSource(bool has_search_engine_source);
+
void Trace(Visitor*) const;
private:
- TextFragmentAnchorParameters GetParametersForMatch(const Match& match);
-
Member<Document> document_;
#ifndef NDEBUG
@@ -85,6 +88,7 @@
base::TimeTicks first_scroll_into_view_time_;
bool did_non_zero_scroll_ = false;
bool did_scroll_to_top_ = false;
+ bool has_search_engine_source_ = false;
const base::TickClock* tick_clock_;
};
diff --git a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc
index 5c57efd6..dcde1c1 100644
--- a/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc
+++ b/third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics_test.cc
@@ -137,6 +137,9 @@
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.TableCellMatch", 1);
histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.TableCellMatch", 0,
1);
+ histogram_tester_.ExpectTotalCount("TextFragmentAnchor.LinkOpenSource", 1);
+ histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.LinkOpenSource", 0,
+ 1);
}
// Test UMA metrics collection when there is no match found
@@ -199,6 +202,10 @@
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.ListItemMatch", 0);
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.TableCellMatch", 0);
+
+ histogram_tester_.ExpectTotalCount("TextFragmentAnchor.LinkOpenSource", 1);
+ histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.LinkOpenSource", 0,
+ 1);
}
// Test that we don't collect any metrics when there is no text directive
@@ -245,6 +252,8 @@
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.ListItemMatch", 0);
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.TableCellMatch", 0);
+
+ histogram_tester_.ExpectTotalCount("TextFragmentAnchor.LinkOpenSource", 0);
}
// Test that the correct metrics are collected when we found a match but didn't
@@ -312,6 +321,10 @@
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.TableCellMatch", 1);
histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.TableCellMatch", 0,
1);
+
+ histogram_tester_.ExpectTotalCount("TextFragmentAnchor.LinkOpenSource", 1);
+ histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.LinkOpenSource", 0,
+ 1);
}
// Test that the correct metrics are collected for all possible combinations of
@@ -405,6 +418,10 @@
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.TableCellMatch", 4);
histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.TableCellMatch", 0,
4);
+
+ histogram_tester_.ExpectTotalCount("TextFragmentAnchor.LinkOpenSource", 1);
+ histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.LinkOpenSource", 0,
+ 1);
}
// Test that the correct metrics are collected for all possible combinations of
@@ -510,6 +527,10 @@
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.ListItemMatch", 0);
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.TableCellMatch", 0);
+
+ histogram_tester_.ExpectTotalCount("TextFragmentAnchor.LinkOpenSource", 1);
+ histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.LinkOpenSource", 0,
+ 1);
}
class TextFragmentAnchorScrollMetricsTest
@@ -635,6 +656,10 @@
histogram_tester_.ExpectTotalCount("TextFragmentAnchor.TableCellMatch", 1);
histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.TableCellMatch", 0,
1);
+
+ histogram_tester_.ExpectTotalCount("TextFragmentAnchor.LinkOpenSource", 1);
+ histogram_tester_.ExpectUniqueSample("TextFragmentAnchor.LinkOpenSource", 0,
+ 1);
}
// Test that the user scrolling back to the top of the page reports metrics
diff --git a/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc b/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc
index 19cf8c3..282a10b 100644
--- a/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc
+++ b/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.cc
@@ -4,12 +4,15 @@
#include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/time/default_tick_clock.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
#include "third_party/blink/public/platform/interface_registry.h"
#include "third_party/blink/renderer/core/editing/ephemeral_range.h"
#include "third_party/blink/renderer/core/editing/finder/find_buffer.h"
#include "third_party/blink/renderer/core/editing/iterators/text_iterator.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
+#include "third_party/blink/renderer/core/page/scrolling/text_fragment_anchor_metrics.h"
#include "third_party/blink/renderer/core/page/scrolling/text_fragment_finder.h"
#include "third_party/blink/renderer/platform/text/text_boundaries.h"
@@ -157,6 +160,7 @@
constexpr int kNoContextMinChars = 20;
constexpr int kMaxContextWords = 10;
constexpr int kMaxRangeWords = 10;
+constexpr int kMaxIterationCountToRecord = 10;
void TextFragmentSelectorGenerator::UpdateSelection(
LocalFrame* selection_frame,
@@ -228,6 +232,7 @@
DCHECK(selection_range_);
DCHECK(callback);
+ generation_start_time_ = base::DefaultTickClock::GetInstance()->NowTicks();
pending_generate_selector_callback_ = std::move(callback);
state_ = kNeedsNewCandidate;
step_ = kExact;
@@ -235,8 +240,13 @@
max_available_suffix_ = "";
num_prefix_words_ = 0;
num_suffix_words_ = 0;
+ iteration_ = 0;
CompleteSelection();
+ UMA_HISTOGRAM_COUNTS_1000(
+ "SharedHighlights.LinkGenerated.SelectionLength",
+ PlainText(EphemeralRange(selection_range_)).length());
+
GenerateSelectorCandidate();
}
@@ -274,7 +284,7 @@
void TextFragmentSelectorGenerator::RunTextFinder() {
DCHECK(selector_);
-
+ iteration_++;
// |FindMatch| will call |DidFindMatch| indicating if the match was unique.
TextFragmentFinder finder(*this, *selector_);
finder.FindMatch(*selection_frame_->GetDocument());
@@ -301,6 +311,8 @@
}
void TextFragmentSelectorGenerator::NoMatchFound() {
+ UMA_HISTOGRAM_ENUMERATION("SharedHighlights.LinkGenerated.Error",
+ LinkGenerationError::kIncorrectSelector);
NotifySelectorReady(
TextFragmentSelector(TextFragmentSelector::SelectorType::kInvalid));
}
@@ -308,6 +320,33 @@
void TextFragmentSelectorGenerator::NotifySelectorReady(
const TextFragmentSelector& selector) {
DCHECK(pending_generate_selector_callback_);
+ // TODO(crbug.com/1133823): Add unit tests for all SharedHighlights.*
+ // histograms.
+ UMA_HISTOGRAM_BOOLEAN(
+ "SharedHighlights.LinkGenerated",
+ selector.Type() != TextFragmentSelector::SelectorType::kInvalid);
+
+ if (selector.Type() != TextFragmentSelector::SelectorType::kInvalid) {
+ UMA_HISTOGRAM_COUNTS_1000("SharedHighlights.LinkGenerated.ParamLength",
+ selector.ToString().length());
+
+ UMA_HISTOGRAM_EXACT_LINEAR("SharedHighlights.LinkGenerated.Iterations",
+ iteration_, kMaxIterationCountToRecord);
+ UMA_HISTOGRAM_TIMES("SharedHighlights.LinkGenerated.TimeToGenerate",
+ base::DefaultTickClock::GetInstance()->NowTicks() -
+ generation_start_time_);
+ UMA_HISTOGRAM_ENUMERATION(
+ "SharedHighlights.LinkGenerated.SelectorParameters",
+ TextFragmentAnchorMetrics::GetParametersForSelector(selector));
+ } else {
+ UMA_HISTOGRAM_EXACT_LINEAR(
+ "SharedHighlights.LinkGenerated.Error.Iterations", iteration_,
+ kMaxIterationCountToRecord);
+ UMA_HISTOGRAM_TIMES("SharedHighlights.LinkGenerated.Error.TimeToGenerate",
+ base::DefaultTickClock::GetInstance()->NowTicks() -
+ generation_start_time_);
+ }
+
std::move(pending_generate_selector_callback_).Run(selector.ToString());
}
@@ -351,6 +390,8 @@
String selected_text = PlainText(ephemeral_range).StripWhiteSpace();
if (selected_text.IsEmpty()) {
+ UMA_HISTOGRAM_ENUMERATION("SharedHighlights.LinkGenerated.Error",
+ LinkGenerationError::kEmptySelection);
state_ = kFailure;
return;
}
@@ -411,6 +452,8 @@
// If from middle till end of selection there is no word break, then we
// cannot use it for range end.
if (mid_point == selection_length) {
+ UMA_HISTOGRAM_ENUMERATION("SharedHighlights.LinkGenerated.Error",
+ LinkGenerationError::kNoRange);
state_ = kFailure;
return;
}
@@ -451,6 +494,8 @@
// Give up if context is already too long.
if (num_prefix_words_ == kMaxContextWords ||
num_prefix_words_ == kMaxContextWords) {
+ UMA_HISTOGRAM_ENUMERATION("SharedHighlights.LinkGenerated.Error",
+ LinkGenerationError::kContextLimitReached);
state_ = kFailure;
return;
}
@@ -463,6 +508,8 @@
}
if (max_available_prefix_.IsEmpty() && max_available_suffix_.IsEmpty()) {
+ UMA_HISTOGRAM_ENUMERATION("SharedHighlights.LinkGenerated.Error",
+ LinkGenerationError::kNoContext);
state_ = kFailure;
return;
}
@@ -472,6 +519,8 @@
// Give up if we were unable to get new prefix and suffix.
if (prefix == selector_->Prefix() && suffix == selector_->Suffix()) {
+ UMA_HISTOGRAM_ENUMERATION("SharedHighlights.LinkGenerated.Error",
+ LinkGenerationError::kContextExhausted);
state_ = kFailure;
return;
}
diff --git a/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h b/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h
index 5f7c047..14272be 100644
--- a/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h
+++ b/third_party/blink/renderer/core/page/scrolling/text_fragment_selector_generator.h
@@ -31,6 +31,17 @@
public TextFragmentFinder::Client,
public blink::mojom::blink::TextFragmentSelectorProducer {
public:
+ // Update corresponding |LinkGenerationError| in enums.xml.
+ enum LinkGenerationError {
+ kIncorrectSelector,
+ kNoRange,
+ kNoContext,
+ kContextExhausted,
+ kContextLimitReached,
+ kEmptySelection,
+
+ kMaxValue = kContextLimitReached
+ };
explicit TextFragmentSelectorGenerator() = default;
void BindTextFragmentSelectorProducer(
@@ -140,6 +151,9 @@
int num_range_start_words_ = 0;
int num_range_end_words_ = 0;
+ int iteration_ = 0;
+ base::TimeTicks generation_start_time_;
+
DISALLOW_COPY_AND_ASSIGN(TextFragmentSelectorGenerator);
};
diff --git a/third_party/blink/renderer/platform/BUILD.gn b/third_party/blink/renderer/platform/BUILD.gn
index 6e76f3d..145eb03 100644
--- a/third_party/blink/renderer/platform/BUILD.gn
+++ b/third_party/blink/renderer/platform/BUILD.gn
@@ -1361,6 +1361,8 @@
"privacy_budget/identifiability_digest_helpers.cc",
"privacy_budget/identifiability_digest_helpers.h",
"resolution_units.h",
+ "search_engine_utils.cc",
+ "search_engine_utils.h",
"supplementable.cc",
"supplementable.h",
"text/bidi_character_run.h",
@@ -1607,6 +1609,7 @@
"//base/allocator:buildflags",
"//cc/ipc",
"//components/paint_preview/common",
+ "//components/search_engines:search_engine_utils",
"//components/viz/client",
"//components/viz/common",
"//crypto",
diff --git a/third_party/blink/renderer/platform/DEPS b/third_party/blink/renderer/platform/DEPS
index 8b2054f6..b6e1dc15 100644
--- a/third_party/blink/renderer/platform/DEPS
+++ b/third_party/blink/renderer/platform/DEPS
@@ -97,4 +97,7 @@
"ukm_time_aggregator_test.cc" : [
"+components/ukm/test_ukm_recorder.h"
],
+ "search_engine_utils.cc": [
+ "+components/search_engines/search_engine_utils.h"
+ ],
}
diff --git a/third_party/blink/renderer/platform/search_engine_utils.cc b/third_party/blink/renderer/platform/search_engine_utils.cc
new file mode 100644
index 0000000..02e013a9
--- /dev/null
+++ b/third_party/blink/renderer/platform/search_engine_utils.cc
@@ -0,0 +1,18 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/platform/search_engine_utils.h"
+
+#include "components/search_engines/search_engine_utils.h"
+#include "url/gurl.h"
+
+namespace blink {
+
+bool IsKnownSearchEngine(const AtomicString& url) {
+ GURL gurl(url.GetString().Utf8());
+
+ return SearchEngineUtils::GetEngineType(gurl) > 0;
+}
+
+} // namespace blink
diff --git a/third_party/blink/renderer/platform/search_engine_utils.h b/third_party/blink/renderer/platform/search_engine_utils.h
new file mode 100644
index 0000000..551ead90
--- /dev/null
+++ b/third_party/blink/renderer/platform/search_engine_utils.h
@@ -0,0 +1,18 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_SEARCH_ENGINE_UTILS_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_SEARCH_ENGINE_UTILS_H_
+
+#include "third_party/blink/renderer/platform/platform_export.h"
+#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
+
+namespace blink {
+
+// Returns whether provided string is a URL of a known search engine.
+PLATFORM_EXPORT bool IsKnownSearchEngine(const AtomicString&);
+
+} // namespace blink
+
+#endif // THIRD_PARTY_BLINK_RENDERER_PLATFORM_SEARCH_ENGINE_UTILS_H_
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 3cba4c2..7ac44bcc 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -40194,6 +40194,15 @@
<int value="20" label="kOtherReason"/>
</enum>
+<enum name="LinkGenerationError">
+ <int value="0" label="Incorrect selector"/>
+ <int value="1" label="No range available"/>
+ <int value="2" label="No context available"/>
+ <int value="3" label="Available context exhausted"/>
+ <int value="4" label="Context word limit reached"/>
+ <int value="5" label="Empty selection"/>
+</enum>
+
<enum name="LinkMonitorFailureType">
<int value="0" label="Local MAC Address Not Found"/>
<int value="1" label="Client Startup Failure"/>
@@ -70736,6 +70745,11 @@
<int value="8" label="TextRangeWithContext"/>
</enum>
+<enum name="TextFragmentLinkOpenSource">
+ <int value="0" label="Unknown"/>
+ <int value="1" label="SearchEngine"/>
+</enum>
+
<enum name="TextToSpeechEvent">
<int value="0" label="Start"/>
<int value="1" label="End"/>
diff --git a/tools/metrics/histograms/histograms_xml/others/histograms.xml b/tools/metrics/histograms/histograms_xml/others/histograms.xml
index 9123364..df62208 100644
--- a/tools/metrics/histograms/histograms_xml/others/histograms.xml
+++ b/tools/metrics/histograms/histograms_xml/others/histograms.xml
@@ -12659,6 +12659,96 @@
<summary>Win32 APIs that error out during setup.</summary>
</histogram>
+<histogram name="SharedHighlights.LinkGenerated" units="BooleanSuccess"
+ expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Indicates whether text fragment selector generation was successful.
+ </summary>
+</histogram>
+
+<histogram name="SharedHighlights.LinkGenerated.Error"
+ enum="LinkGenerationError" expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Indicates error that caused text fragment selector generation to fail.
+ </summary>
+</histogram>
+
+<histogram name="SharedHighlights.LinkGenerated.Error.Iterations"
+ units="iterations" expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Indicates number of iterations it took for the unsuccessful text fragment
+ selector generation run.
+ </summary>
+</histogram>
+
+<histogram name="SharedHighlights.LinkGenerated.Error.TimeToGenerate"
+ units="ms" expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Time it took in millisecons for a failed text fragment selector generation
+ attempt.
+ </summary>
+</histogram>
+
+<histogram name="SharedHighlights.LinkGenerated.Iterations" units="iterations"
+ expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Indicates number of iterations it took to successfully generate text
+ fragment selector.
+ </summary>
+</histogram>
+
+<histogram name="SharedHighlights.LinkGenerated.ParamLength" units="characters"
+ expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Number of charaters in successfully generated text fragment selector. For
+ example, for #:=:text=sample%20text, it would be the
+ len("sample%20text") which is 13. Recorded only when selector
+ generation was successful.
+ </summary>
+</histogram>
+
+<histogram name="SharedHighlights.LinkGenerated.SelectionLength"
+ units="characters" expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Number of characters in the user selected text, that text fragment selector
+ is generated for. Recorded regardless of generation success.
+ </summary>
+</histogram>
+
+<histogram name="SharedHighlights.LinkGenerated.SelectorParameters"
+ enum="TextFragmentAnchorParameters" expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Indicates which parameters were specified in a text fragment selector.
+ Recorded only when selector generation was successful.
+ </summary>
+</histogram>
+
+<histogram name="SharedHighlights.LinkGenerated.TimeToGenerate" units="ms"
+ expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ How long it took in milliseconds to successfully generate text fragment
+ selector.
+ </summary>
+</histogram>
+
<histogram name="SharedMemory.MapBlockedForSecurity" enum="BooleanBlocked"
expires_after="M85">
<owner>[email protected]</owner>
@@ -14232,6 +14322,17 @@
</summary>
</histogram>
+<histogram name="TextFragmentAnchor.LinkOpenSource"
+ enum="TextFragmentLinkOpenSource" expires_after="2021-03-07">
+ <owner>[email protected]</owner>
+ <owner>[email protected]</owner>
+ <summary>
+ Recorded for every navigatin to a link with a valid text fragment selector
+ (e.g. #:=:text=SELECTOR). Indicates source type that navigation originated
+ from.
+ </summary>
+</histogram>
+
<histogram name="TextFragmentAnchor.ListItemMatch" units="Boolean"
expires_after="2021-03-07">
<owner>[email protected]</owner>
