Skip to content

Commit

Permalink
Report PreloadingTriggeringOutcome to UMA.
Browse files Browse the repository at this point in the history
A new histogram "Preloading.{PreloadingType}.Attempt
{PreloadingPredictor}.TriggeringOutcome" is added to track the terminal
status of preloading events, for which we can derive the number of
prefetches, prefetches success rate etc.

PreloadingPredictor is changed from enum class to a normal class so that
it holds both the UKM enum as well as the string description. With such,
- ContentPreloadingPredictor / ChromePreloadingPredictor are removed,
  along with all the associated ToPreloadingPredictor cast methods.
- {Chrome|Content}PreloadingPredictor::PredictorXYZ is re-expressed
  with {chrome|content}_preloading_predictor::PredictorXYZ namespace.
- PreloadingPredictor content layer enum boundaries are removed. The
  enum validity check is placed into PreloadingDataImpl, just before
  the predictions and attempts are reported to UKM.


Note: the triggering outcome metrics can be derived from the Precog UKMs
but a UMA histogram can answer them much more directly.

Bug: 1406238
Change-Id: If1742b1e3a0cf2803c614d2c4c7d0cbe610057ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4149348
Reviewed-by: Sreeja Kamishetty <sreejakshetty@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Simon Pelchat <spelchat@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: William Liu <liuwilliam@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1095788}
  • Loading branch information
William Liu authored and Chromium LUCI CQ committed Jan 23, 2023
1 parent 718d899 commit 238a9e5
Show file tree
Hide file tree
Showing 46 changed files with 536 additions and 266 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ void AnchorElementPreloader::MaybePreconnect(const GURL& target) {
// likely compute the confidence by looking at different factors (e.g. anchor
// element dimensions, last time since scroll, etc.).
preloading_data->AddPreloadingPrediction(
ToPreloadingPredictor(ChromePreloadingPredictor::kPointerDownOnAnchor),
chrome_preloading_predictor::kPointerDownOnAnchor,
/*confidence=*/100, match_callback);
content::PreloadingAttempt* attempt = preloading_data->AddPreloadingAttempt(
ToPreloadingPredictor(ChromePreloadingPredictor::kPointerDownOnAnchor),
chrome_preloading_predictor::kPointerDownOnAnchor,
content::PreloadingType::kPreconnect, match_callback);

if (content::PreloadingEligibility eligibility =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ class AnchorElementPreloaderBrowserTest
test_ukm_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
ukm_entry_builder_ =
std::make_unique<content::test::PreloadingAttemptUkmEntryBuilder>(
ToPreloadingPredictor(
ChromePreloadingPredictor::kPointerDownOnAnchor));
chrome_preloading_predictor::kPointerDownOnAnchor);
test_timer_ = std::make_unique<base::ScopedMockElapsedTimersForTest>();
ASSERT_TRUE(loading_predictor);
loading_predictor->preconnect_manager()->SetObserverForTesting(this);
Expand Down
9 changes: 3 additions & 6 deletions chrome/browser/predictors/autocomplete_action_predictor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,7 @@ void AutocompleteActionPredictor::StartPrerendering(
// this prerendering attempt for Prerender.
content::PreloadingAttempt* preloading_attempt =
preloading_data->AddPreloadingAttempt(
ToPreloadingPredictor(
ChromePreloadingPredictor::kOmniboxDirectURLInput),
chrome_preloading_predictor::kOmniboxDirectURLInput,
content::PreloadingType::kPrerender, std::move(same_url_matcher));

PrerenderManager::CreateForWebContents(&web_contents);
Expand All @@ -227,8 +226,7 @@ void AutocompleteActionPredictor::StartPrerendering(
// this preloading attempt for NoStatePrefetch.
content::PreloadingAttempt* preloading_attempt =
preloading_data->AddPreloadingAttempt(
ToPreloadingPredictor(
ChromePreloadingPredictor::kOmniboxDirectURLInput),
chrome_preloading_predictor::kOmniboxDirectURLInput,
content::PreloadingType::kNoStatePrefetch,
std::move(same_url_matcher));

Expand Down Expand Up @@ -318,8 +316,7 @@ AutocompleteActionPredictor::RecommendAction(
// We multiply confidence by 100 to pass the percentage and cast it into int
// for logs.
preloading_data->AddPreloadingPrediction(
ToPreloadingPredictor(
ChromePreloadingPredictor::kOmniboxDirectURLInput),
chrome_preloading_predictor::kOmniboxDirectURLInput,
static_cast<int64_t>(confidence * 100), std::move(same_url_matcher));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ class AutocompleteActionPredictorTest : public testing::Test {
profile_.get(), nullptr);
ukm_entry_builder_ =
std::make_unique<content::test::PreloadingPredictionUkmEntryBuilder>(
ToPreloadingPredictor(
ChromePreloadingPredictor::kOmniboxDirectURLInput));
chrome_preloading_predictor::kOmniboxDirectURLInput);
test_ukm_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
test_timer_ = std::make_unique<base::ScopedMockElapsedTimersForTest>();

Expand Down
8 changes: 1 addition & 7 deletions chrome/browser/preloading/chrome_preloading.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@
// found in the LICENSE file.

#include "chrome/browser/preloading/chrome_preloading.h"

#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "components/search_engines/template_url.h"
#include "components/search_engines/template_url_service.h"
#include "content/public/browser/browser_context.h"
#include "url/gurl.h"

using content::PreloadingPredictor;

namespace {

bool IsSideSearch(content::BrowserContext* browser_context, const GURL& url) {
Expand All @@ -31,11 +30,6 @@ bool IsSideSearch(content::BrowserContext* browser_context, const GURL& url) {

} // namespace

content::PreloadingPredictor ToPreloadingPredictor(
ChromePreloadingPredictor predictor) {
return static_cast<content::PreloadingPredictor>(predictor);
}

content::PreloadingEligibility ToPreloadingEligibility(
ChromePreloadingEligibility eligibility) {
return static_cast<content::PreloadingEligibility>(eligibility);
Expand Down
103 changes: 53 additions & 50 deletions chrome/browser/preloading/chrome_preloading.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,63 +13,66 @@
#include "url/gurl.h"

class TemplateURLService;
using content::PreloadingPredictor;

// If you change any of the following emums, please follow the process in
// go/preloading-dashboard-updates to update the mapping reflected in
// dashboard, or if you are not a Googler, please file an FYI bug on
// https://crbug.new with component Internals>Preload.
// If you change any of the following enums or static variables, please follow
// the process in go/preloading-dashboard-updates to update the mapping
// reflected in dashboard, or if you are not a Googler, please file an FYI bug
// on https://crbug.new with component Internals>Preload.

// Defines various embedder triggering mechanisms which triggers different
// preloading operations mentioned in //content/public/browser/preloading.h.

// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class ChromePreloadingPredictor {
// Numbering starts from `kPreloadingPredictorContentEnd` defined in
// //content/browser/public/preloading.h . Advance numbering by +1 when adding
// a new element.

// When the preloading URL is predicted from the Omnibox Direct URL Input
// (DUI). This is used to perform various preloading operations like prefetch
// and prerender to load Omnibox predicted URLs faster.
kOmniboxDirectURLInput =
static_cast<int>(PreloadingPredictor::kPreloadingPredictorContentEnd),

// When a pointerdown (e.g. mousedown or touchstart) event happens on an
// anchor element with an href value pointing to an HTTP(S) origin, we may
// attempt to preload the link.
kPointerDownOnAnchor =
static_cast<int>(PreloadingPredictor::kPreloadingPredictorContentEnd) + 1,

// When the preloading URL is predicted from the default search suggest
// service for faster search page loads.
kDefaultSearchEngine =
static_cast<int>(PreloadingPredictor::kPreloadingPredictorContentEnd) + 2,

// When the preloading URL is predicted from the default search suggest due to
// change in Omnibox selection.
kOmniboxSearchPredictor =
static_cast<int>(PreloadingPredictor::kPreloadingPredictorContentEnd) + 3,

// When the preloading URL is predicted from the default search suggest due to
// mouse being pressed down on a Omnibox Search suggestion.
kOmniboxMousePredictor =
static_cast<int>(PreloadingPredictor::kPreloadingPredictorContentEnd) + 4,

// When the default match in omnibox has the search prefetch or prerender
// hint.
kOmniboxSearchSuggestDefaultMatch =
static_cast<int>(PreloadingPredictor::kPreloadingPredictorContentEnd) + 5,

// TODO(crbug.com/1309934): Integrate more Preloading predictors with
// Preloading logging APIs.
};

// Helper method to convert ChromePreloadingPredictor to
// content::PreloadingPredictor to avoid casting.
content::PreloadingPredictor ToPreloadingPredictor(
ChromePreloadingPredictor predictor);
//
// Advance numbering by +1 when adding a new element.
//
// Please make sure Chrome `PreloadingPredictor` are defined after 100
// (inclusive) as 99 and below are reserved for content-public and
// content-internal definitions. Both the value and the name should be unique
// across all the namespaces.
namespace chrome_preloading_predictor {
// When the preloading URL is predicted from the Omnibox Direct URL Input
// (DUI). This is used to perform various preloading operations like prefetch
// and prerender to load Omnibox predicted URLs faster.
static constexpr content::PreloadingPredictor kOmniboxDirectURLInput(
100,
"OmniboxDirectURLInput");

// When a pointerdown (e.g. mousedown or touchstart) event happens on an
// anchor element with an href value pointing to an HTTP(S) origin, we may
// attempt to preload the link.
static constexpr content::PreloadingPredictor kPointerDownOnAnchor(
101,
"PointerDownOnAnchor");

// When the preloading URL is predicted from the default search suggest
// service for faster search page loads.
static constexpr content::PreloadingPredictor kDefaultSearchEngine(
102,
"DefaultSearchEngine");

// When the preloading URL is predicted from the default search suggest due to
// change in Omnibox selection.
static constexpr content::PreloadingPredictor kOmniboxSearchPredictor(
103,
"OmniboxSearchPredictor");

// When the preloading URL is predicted from the default search suggest due to
// mouse being pressed down on a Omnibox Search suggestion.
static constexpr content::PreloadingPredictor kOmniboxMousePredictor(
104,
"OmniboxMousePredictor");

// When the default match in omnibox has the search prefetch or prerender
// hint.
static constexpr content::PreloadingPredictor kOmniboxSearchSuggestDefaultMatch(
105,
"OmniboxSearchSuggestDefaultMatch");

// TODO(crbug.com/1309934): Integrate more Preloading predictors with
// Preloading logging APIs.
} // namespace chrome_preloading_predictor

// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,10 @@ class NoStatePrefetchBrowserTest
test_ukm_recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
omnibox_attempt_entry_builder_ =
std::make_unique<content::test::PreloadingAttemptUkmEntryBuilder>(
ToPreloadingPredictor(
ChromePreloadingPredictor::kOmniboxDirectURLInput));
chrome_preloading_predictor::kOmniboxDirectURLInput);
link_rel_attempt_entry_builder_ =
std::make_unique<content::test::PreloadingAttemptUkmEntryBuilder>(
content::PreloadingPredictor::kLinkRel);
content::preloading_predictor::kLinkRel);
test_timer_ = std::make_unique<base::ScopedMockElapsedTimersForTest>();
host_resolver()->AddRule("*", "127.0.0.1");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/task/single_thread_task_runner.h"
#include "chrome/browser/devtools/devtools_window.h"
#include "chrome/browser/devtools/devtools_window_testing.h"
#include "chrome/browser/preloading/chrome_preloading.h"
#include "chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_request.h"
#include "chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service.h"
#include "chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_service_factory.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,14 @@ bool SearchPrefetchService::MaybePrefetchURL(
const GURL& url,
content::WebContents* web_contents) {
return MaybePrefetchURL(url, /*navigation_prefetch=*/false, web_contents,
ChromePreloadingPredictor::kDefaultSearchEngine);
chrome_preloading_predictor::kDefaultSearchEngine);
}

bool SearchPrefetchService::MaybePrefetchURL(
const GURL& url,
bool navigation_prefetch,
content::WebContents* web_contents,
ChromePreloadingPredictor predictor) {
content::PreloadingPredictor predictor) {
if (!SearchPrefetchServicePrefetchingIsEnabled())
return false;

Expand Down Expand Up @@ -242,8 +242,7 @@ bool SearchPrefetchService::MaybePrefetchURL(
// this DefaultSearchEngine or OmniboxSearchPredictor prefetch attempt when
// |navigation_prefetch| is true.
attempt = preloading_data->AddPreloadingAttempt(
ToPreloadingPredictor(predictor), content::PreloadingType::kPrefetch,
same_url_matcher);
predictor, content::PreloadingType::kPrefetch, same_url_matcher);

if (!search_with_terms) {
recorder.reason_ =
Expand Down Expand Up @@ -648,8 +647,8 @@ void SearchPrefetchService::OnResultChanged(content::WebContents* web_contents,

// Create PreloadingPrediction for this match.
preloading_data->AddPreloadingPrediction(
ToPreloadingPredictor(ChromePreloadingPredictor::kDefaultSearchEngine),
confidence, std::move(same_url_matcher));
chrome_preloading_predictor::kDefaultSearchEngine, confidence,
std::move(same_url_matcher));

// Record a prediction for default match prefetch suggest predictions.
if (result.default_match() == &match) {
Expand All @@ -662,8 +661,7 @@ void SearchPrefetchService::OnResultChanged(content::WebContents* web_contents,

// Create PreloadingPrediction for this match.
preloading_data->AddPreloadingPrediction(
ToPreloadingPredictor(
ChromePreloadingPredictor::kOmniboxSearchSuggestDefaultMatch),
chrome_preloading_predictor::kOmniboxSearchSuggestDefaultMatch,
confidence, std::move(same_url_matcher));
} else if (OnlyAllowDefaultMatchPreloading()) {
// Only prefetch default match when in the experiment.
Expand Down Expand Up @@ -781,17 +779,17 @@ void SearchPrefetchService::OnNavigationLikely(
[](NavigationPredictor navigation_predictor) {
switch (navigation_predictor) {
case NavigationPredictor::kMouseDown:
return ChromePreloadingPredictor::kOmniboxMousePredictor;
return chrome_preloading_predictor::kOmniboxMousePredictor;
case NavigationPredictor::kUpOrDownArrowButton:
return ChromePreloadingPredictor::kOmniboxSearchPredictor;
return chrome_preloading_predictor::kOmniboxSearchPredictor;
}
};
auto predictor = navigation_likely_event_to_predictor(navigation_predictor);

// Create PreloadingPrediction for this match. We set the confidence to 100 as
// when the user changed the selected match, we always trigger prefetch.
preloading_data->AddPreloadingPrediction(ToPreloadingPredictor(predictor),
100, std::move(same_url_matcher));
preloading_data->AddPreloadingPrediction(predictor, 100,
std::move(same_url_matcher));
MaybePrefetchURL(preload_url,
/*navigation_prefetch=*/true, web_contents, predictor);
}
Expand Down Expand Up @@ -998,8 +996,7 @@ void SearchPrefetchService::CoordinatePrefetchWithPrerender(
content::PreloadingData::GetOrCreateForWebContents(web_contents);
content::PreloadingAttempt* preloading_attempt =
preloading_data->AddPreloadingAttempt(
ToPreloadingPredictor(
ChromePreloadingPredictor::kDefaultSearchEngine),
chrome_preloading_predictor::kDefaultSearchEngine,
content::PreloadingType::kPrerender, same_url_matcher);

auto prefetch_request_iter = prefetches_.find(canonical_search_url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
#include "base/scoped_observation.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "chrome/browser/preloading/chrome_preloading.h"
#include "chrome/browser/preloading/prefetch/search_prefetch/search_prefetch_request.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/omnibox/browser/autocomplete_match.h"
#include "components/omnibox/browser/omnibox.mojom-shared.h"
#include "components/search_engines/template_url_data.h"
#include "components/search_engines/template_url_service.h"
#include "components/search_engines/template_url_service_observer.h"
#include "content/public/browser/preloading.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -211,7 +211,7 @@ class SearchPrefetchService : public KeyedService,
bool MaybePrefetchURL(const GURL& url,
bool navigation_prefetch,
content::WebContents* web_contents,
ChromePreloadingPredictor predictor);
content::PreloadingPredictor predictor);

// Adds |this| as an observer of |template_url_service| if not added already.
void ObserveTemplateURLService(TemplateURLService* template_url_service);
Expand Down

0 comments on commit 238a9e5

Please sign in to comment.