Skip to content

Commit

Permalink
Enable UKM testing from within web tests (wpt_internal)
Browse files Browse the repository at this point in the history
Expose internals.initializeUKMRecorder(), a function that starts
a TestUkmRecorder and registers it to the document. This UKM recorder
is exposed to the test, currently with one function:

getMetrics(entryName, metricNames[])

This can be further extended with the features of TestUkmRecorder.

Note that this is limited to the UKMs collected by the document,
as a first iteration.

Bug: 1425863
Bug: 1380257
Change-Id: I5edc56701a02465481a46b491b9b8070920f474c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4352828
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Noam Rosenthal <nrosenthal@chromium.org>
Reviewed-by: Yi Xu <yiyix@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1123703}
  • Loading branch information
noamr authored and Chromium LUCI CQ committed Mar 29, 2023
1 parent 501d2da commit 962432a
Show file tree
Hide file tree
Showing 17 changed files with 179 additions and 113 deletions.
2 changes: 2 additions & 0 deletions third_party/blink/renderer/bindings/generated_in_core.gni
Original file line number Diff line number Diff line change
Expand Up @@ -1909,6 +1909,8 @@ generated_interface_sources_for_testing_in_core = [
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_internal_settings_generated.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_internals.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_internals.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_internals_ukm_recorder.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_internals_ukm_recorder.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_origin_trials_test.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_origin_trials_test.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_record_test.cc",
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/bindings/idl_in_core.gni
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,7 @@ static_idl_files_in_core_for_testing = get_path_info(
"//third_party/blink/renderer/core/testing/internals_delete_all_cookies.idl",
"//third_party/blink/renderer/core/testing/internals_get_all_cookies.idl",
"//third_party/blink/renderer/core/testing/internals_get_named_cookie.idl",
"//third_party/blink/renderer/core/testing/internals_ukm_recorder.idl",
"//third_party/blink/renderer/core/testing/origin_trials_test.idl",
"//third_party/blink/renderer/core/testing/origin_trials_test_dictionary.idl",
"//third_party/blink/renderer/core/testing/origin_trials_test_partial.idl",
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,8 @@ source_set("testing") {
"testing/internals_get_all_cookies.h",
"testing/internals_get_named_cookie.cc",
"testing/internals_get_named_cookie.h",
"testing/internals_ukm_recorder.cc",
"testing/internals_ukm_recorder.h",
"testing/intersection_observer_test_helper.h",
"testing/mock_clipboard_host.cc",
"testing/mock_clipboard_host.h",
Expand Down
30 changes: 22 additions & 8 deletions third_party/blink/renderer/core/dom/document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/mojom/base/text_direction.mojom-blink.h"
#include "services/metrics/public/cpp/delegating_ukm_recorder.h"
#include "services/metrics/public/cpp/metrics_utils.h"
#include "services/metrics/public/cpp/mojo_ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/public/cpp/web_sandbox_flags.h"
#include "services/network/public/mojom/trust_tokens.mojom-blink.h"
Expand Down Expand Up @@ -913,6 +915,9 @@ Document::~Document() {
DCHECK(!ax_object_cache_);

InstanceCounters::DecrementCounter(InstanceCounters::kDocumentCounter);
if (WebTestSupport::IsRunningWebTest() && ukm_recorder_) {
ukm::DelegatingUkmRecorder::Get()->RemoveDelegate(ukm_recorder_.get());
}
}

Range* Document::CreateRangeAdjustedToTreeScope(const TreeScope& tree_scope,
Expand Down Expand Up @@ -7718,15 +7723,24 @@ bool Document::AllowedToUseDynamicMarkUpInsertion(
}

ukm::UkmRecorder* Document::UkmRecorder() {
if (ukm_recorder_)
return ukm_recorder_.get();

mojo::PendingRemote<ukm::mojom::UkmRecorderInterface> recorder;
Platform::Current()->GetBrowserInterfaceBroker()->GetInterface(
recorder.InitWithNewPipeAndPassReceiver());
ukm_recorder_ = std::make_unique<ukm::MojoUkmRecorder>(std::move(recorder));
if (!ukm_recorder_) {
mojo::PendingRemote<ukm::mojom::UkmRecorderInterface> recorder;
Platform::Current()->GetBrowserInterfaceBroker()->GetInterface(
recorder.InitWithNewPipeAndPassReceiver());
std::unique_ptr<ukm::MojoUkmRecorder> mojo_recorder =
std::make_unique<ukm::MojoUkmRecorder>(std::move(recorder));
if (WebTestSupport::IsRunningWebTest()) {
ukm::DelegatingUkmRecorder::Get()->AddDelegate(
mojo_recorder->GetWeakPtr());
}
ukm_recorder_ = std::move(mojo_recorder);
}

return ukm_recorder_.get();
if (WebTestSupport::IsRunningWebTest()) {
return ukm::DelegatingUkmRecorder::Get();
} else {
return ukm_recorder_.get();
}
}

ukm::SourceId Document::UkmSourceID() const {
Expand Down
44 changes: 16 additions & 28 deletions third_party/blink/renderer/core/dom/document_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1009,30 +1009,26 @@ TEST_F(DocumentTest, PrefersColorSchemeChanged) {
}

TEST_F(DocumentTest, FindInPageUkm) {
// TODO(https://crbug.com/1380257): Find a better way than swapping the UKM
// recorder.
GetDocument().ukm_recorder_ = std::make_unique<ukm::TestUkmRecorder>();
auto* recorder =
static_cast<ukm::TestUkmRecorder*>(GetDocument().UkmRecorder());
ukm::TestAutoSetUkmRecorder recorder;

EXPECT_EQ(recorder->entries_count(), 0u);
EXPECT_EQ(recorder.entries_count(), 0u);
GetDocument().MarkHasFindInPageRequest();
EXPECT_EQ(recorder->entries_count(), 1u);
EXPECT_EQ(recorder.entries_count(), 1u);
GetDocument().MarkHasFindInPageRequest();
EXPECT_EQ(recorder->entries_count(), 1u);
EXPECT_EQ(recorder.entries_count(), 1u);

auto entries = recorder->GetEntriesByName("Blink.FindInPage");
auto entries = recorder.GetEntriesByName("Blink.FindInPage");
EXPECT_EQ(entries.size(), 1u);
EXPECT_TRUE(ukm::TestUkmRecorder::EntryHasMetric(entries[0], "DidSearch"));
EXPECT_EQ(*ukm::TestUkmRecorder::GetEntryMetric(entries[0], "DidSearch"), 1);
EXPECT_FALSE(ukm::TestUkmRecorder::EntryHasMetric(
entries[0], "DidHaveRenderSubtreeMatch"));

GetDocument().MarkHasFindInPageContentVisibilityActiveMatch();
EXPECT_EQ(recorder->entries_count(), 2u);
EXPECT_EQ(recorder.entries_count(), 2u);
GetDocument().MarkHasFindInPageContentVisibilityActiveMatch();
EXPECT_EQ(recorder->entries_count(), 2u);
entries = recorder->GetEntriesByName("Blink.FindInPage");
EXPECT_EQ(recorder.entries_count(), 2u);
entries = recorder.GetEntriesByName("Blink.FindInPage");
EXPECT_EQ(entries.size(), 2u);

EXPECT_TRUE(ukm::TestUkmRecorder::EntryHasMetric(entries[0], "DidSearch"));
Expand Down Expand Up @@ -1071,30 +1067,25 @@ TEST_F(DocumentTest, FindInPageUkmInFrame) {
ASSERT_TRUE(document);
ASSERT_FALSE(document->IsInMainFrame());

// Save the old recorder and replace it with a test one.
auto old_recorder = std::move(document->ukm_recorder_);
document->ukm_recorder_ = std::make_unique<ukm::TestUkmRecorder>();

auto* recorder = static_cast<ukm::TestUkmRecorder*>(document->UkmRecorder());

EXPECT_EQ(recorder->entries_count(), 0u);
ukm::TestAutoSetUkmRecorder recorder;
EXPECT_EQ(recorder.entries_count(), 0u);
document->MarkHasFindInPageRequest();
EXPECT_EQ(recorder->entries_count(), 1u);
EXPECT_EQ(recorder.entries_count(), 1u);
document->MarkHasFindInPageRequest();
EXPECT_EQ(recorder->entries_count(), 1u);
EXPECT_EQ(recorder.entries_count(), 1u);

auto entries = recorder->GetEntriesByName("Blink.FindInPage");
auto entries = recorder.GetEntriesByName("Blink.FindInPage");
EXPECT_EQ(entries.size(), 1u);
EXPECT_TRUE(ukm::TestUkmRecorder::EntryHasMetric(entries[0], "DidSearch"));
EXPECT_EQ(*ukm::TestUkmRecorder::GetEntryMetric(entries[0], "DidSearch"), 1);
EXPECT_FALSE(ukm::TestUkmRecorder::EntryHasMetric(
entries[0], "DidHaveRenderSubtreeMatch"));

document->MarkHasFindInPageContentVisibilityActiveMatch();
EXPECT_EQ(recorder->entries_count(), 2u);
EXPECT_EQ(recorder.entries_count(), 2u);
document->MarkHasFindInPageContentVisibilityActiveMatch();
EXPECT_EQ(recorder->entries_count(), 2u);
entries = recorder->GetEntriesByName("Blink.FindInPage");
EXPECT_EQ(recorder.entries_count(), 2u);
entries = recorder.GetEntriesByName("Blink.FindInPage");
EXPECT_EQ(entries.size(), 2u);

EXPECT_TRUE(ukm::TestUkmRecorder::EntryHasMetric(entries[0], "DidSearch"));
Expand All @@ -1108,9 +1099,6 @@ TEST_F(DocumentTest, FindInPageUkmInFrame) {
"DidHaveRenderSubtreeMatch"),
1);
EXPECT_FALSE(ukm::TestUkmRecorder::EntryHasMetric(entries[1], "DidSearch"));

// Restore the old recorder, since some ukm metrics are recorded at shutdown.
document->ukm_recorder_ = std::move(old_recorder);
}

TEST_F(DocumentTest, AtPageMarginWithDeviceScaleFactor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -761,21 +761,19 @@ TEST_F(TextFinderSimTest, BeforeMatchExpandedHiddenMatchableUkm) {
<!DOCTYPE html>
<div id=hiddenid hidden=until-found>hidden</div>
)HTML");
GetDocument().ukm_recorder_ = std::make_unique<ukm::TestUkmRecorder>();
auto* recorder =
static_cast<ukm::TestUkmRecorder*>(GetDocument().UkmRecorder());
ukm::TestAutoSetUkmRecorder recorder;
GetDocument().View()->ResetUkmAggregatorForTesting();

Compositor().BeginFrame();
EXPECT_EQ(recorder->entries_count(), 0u);
EXPECT_EQ(recorder.entries_count(), 0u);

GetTextFinder().Find(/*identifier=*/0, WebString(String("hidden")),
*mojom::blink::FindOptions::New(),
/*wrap_within_frame=*/false);

Compositor().BeginFrame();

auto entries = recorder->GetEntriesByName("Blink.FindInPage");
auto entries = recorder.GetEntriesByName("Blink.FindInPage");
// There are two entries because
// DisplayLockUtilities::ActivateFindInPageMatchRangeIfNeeded followed by
// DisplayLockContext::CommitForActivationWithSignal sets a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class CanvasRenderingAPIUkmMetricsTest : public PageTestBase {

void SetUp() override {
PageTestBase::SetUp();
InstallTestUkmRecorder();
GetDocument().documentElement()->setInnerHTML(
"<body><canvas id='c'></canvas></body>");
canvas_element_ = To<HTMLCanvasElement>(GetDocument().getElementById("c"));
Expand All @@ -36,7 +35,7 @@ class CanvasRenderingAPIUkmMetricsTest : public PageTestBase {
CanvasContextCreationAttributesCore attributes;
canvas_element_->GetCanvasRenderingContext(context_type, attributes);

auto entries = test_ukm_recorder_->GetEntriesByName(
auto entries = recorder_.GetEntriesByName(
ukm::builders::ClientRenderingAPI::kEntryName);
EXPECT_EQ(1ul, entries.size());
auto* entry = entries[0];
Expand All @@ -46,14 +45,7 @@ class CanvasRenderingAPIUkmMetricsTest : public PageTestBase {
}

private:
void InstallTestUkmRecorder() {
DCHECK(!test_ukm_recorder_); // Should be initialized only once.
auto temp_recorder = std::make_unique<ukm::TestUkmRecorder>();
test_ukm_recorder_ = temp_recorder.get();
GetDocument().ukm_recorder_ = std::move(temp_recorder);
}

ukm::TestUkmRecorder* test_ukm_recorder_ = nullptr;
ukm::TestAutoSetUkmRecorder recorder_;
Persistent<HTMLCanvasElement> canvas_element_;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,7 @@ class MobileFriendlinessCheckerTest : public testing::Test {

load(*helper);

std::unique_ptr<ukm::UkmRecorder> old_ukm_recorder =
std::move(helper->GetWebView()
->MainFrameImpl()
->GetFrame()
->GetDocument()
->ukm_recorder_);
helper->GetWebView()
->MainFrameImpl()
->GetFrame()
->GetDocument()
->ukm_recorder_ = std::make_unique<ukm::TestUkmRecorder>();
ukm::TestAutoSetUkmRecorder result;

DCHECK(helper->GetWebView()->MainFrameImpl()->GetFrame()->IsLocalRoot());
helper->GetWebView()
Expand All @@ -74,23 +64,10 @@ class MobileFriendlinessCheckerTest : public testing::Test {
->GetMobileFriendlinessChecker()
->ComputeNowForTesting();

std::unique_ptr<ukm::UkmRecorder> result_ukm =
std::move(helper->GetWebView()
->MainFrameImpl()
->GetFrame()
->GetDocument()
->ukm_recorder_);
ukm::TestUkmRecorder* result =
reinterpret_cast<ukm::TestUkmRecorder*>(result_ukm.get());
auto entries = result->GetEntriesByName("MobileFriendliness");
auto entries = result.GetEntriesByName("MobileFriendliness");
EXPECT_EQ(entries.size(), 1u);
EXPECT_EQ(entries[0]->event_hash,
ukm::builders::MobileFriendliness::kEntryNameHash);
helper->GetWebView()
->MainFrameImpl()
->GetFrame()
->GetDocument()
->ukm_recorder_ = std::move(old_ukm_recorder);
return *entries[0];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,11 @@ static constexpr float kMaximumZoom = 5;
class TapFriendlinessCheckerTest : public testing::Test {
protected:
void TearDown() override {
helper_->GetWebView()
->MainFrameImpl()
->GetFrame()
->GetDocument()
->ukm_recorder_ = std::move(old_ukm_recorder_);
ThreadState::Current()->CollectAllGarbageForTesting();
url_test_helpers::UnregisterAllURLsAndClearMemoryCache();
recorder_ = nullptr;
}

void TapAt(int x, int y) {
gfx::PointF pos(x, y);
WebGestureEvent tap_event(WebInputEvent::Type::kGestureTap,
Expand All @@ -48,14 +45,7 @@ class TapFriendlinessCheckerTest : public testing::Test {
settings->SetViewportEnabled(true);
settings->SetViewportMetaEnabled(true);
}
ukm::TestUkmRecorder* GetUkmRecorder() {
DCHECK(helper_);
return static_cast<ukm::TestUkmRecorder*>(helper_->GetWebView()
->MainFrameImpl()
->GetFrame()
->GetDocument()
->UkmRecorder());
}
ukm::TestUkmRecorder* GetUkmRecorder() { return recorder_.get(); }

void LoadHTML(const std::string& html, float device_scale = 1.0) {
helper_ = std::make_unique<frame_test_helpers::WebViewHelper>();
Expand All @@ -77,23 +67,12 @@ class TapFriendlinessCheckerTest : public testing::Test {
frame_test_helpers::LoadHTMLString(helper_->GetWebView()->MainFrameImpl(),
html,
url_test_helpers::ToKURL(kBaseUrl));
// TODO(https://crbug.com/1380257): Find a better way than swapping the UKM
// recorder.
old_ukm_recorder_ = std::move(helper_->GetWebView()
->MainFrameImpl()
->GetFrame()
->GetDocument()
->ukm_recorder_);
helper_->GetWebView()
->MainFrameImpl()
->GetFrame()
->GetDocument()
->ukm_recorder_ = std::make_unique<ukm::TestUkmRecorder>();
recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
}

protected:
std::unique_ptr<frame_test_helpers::WebViewHelper> helper_;
std::unique_ptr<ukm::UkmRecorder> old_ukm_recorder_;
std::unique_ptr<ukm::TestAutoSetUkmRecorder> recorder_;
};

TEST_F(TapFriendlinessCheckerTest, NoTapTarget) {
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/testing/internals.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
#include "third_party/blink/renderer/core/testing/hit_test_layer_rect_list.h"
#include "third_party/blink/renderer/core/testing/internal_runtime_flags.h"
#include "third_party/blink/renderer/core/testing/internal_settings.h"
#include "third_party/blink/renderer/core/testing/internals_ukm_recorder.h"
#include "third_party/blink/renderer/core/testing/mock_hyphenation.h"
#include "third_party/blink/renderer/core/testing/origin_trials_test.h"
#include "third_party/blink/renderer/core/testing/record_test.h"
Expand Down Expand Up @@ -2866,6 +2867,10 @@ UnionTypesTest* Internals::unionTypesTest() const {
return MakeGarbageCollected<UnionTypesTest>();
}

InternalsUkmRecorder* Internals::initializeUKMRecorder() {
return MakeGarbageCollected<InternalsUkmRecorder>(document_);
}

OriginTrialsTest* Internals::originTrialsTest() const {
return MakeGarbageCollected<OriginTrialsTest>();
}
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/testing/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
#include "third_party/blink/renderer/core/css/css_computed_style_declaration.h"
#include "third_party/blink/renderer/core/testing/color_scheme_helper.h"
#include "third_party/blink/renderer/core/testing/internals_ukm_recorder.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
Expand Down Expand Up @@ -65,6 +66,7 @@ class HitTestLocation;
class HitTestResult;
class InternalRuntimeFlags;
class InternalSettings;
class InternalsUkmRecorder;
class LocalDOMWindow;
class LocalFrame;
class Location;
Expand Down Expand Up @@ -630,6 +632,8 @@ class Internals final : public ScriptWrappable {
void setAllowPerChunkTransferring(ReadableStream* stream);
void setBackForwardCacheRestorationBufferSize(unsigned int maxSize);

InternalsUkmRecorder* initializeUKMRecorder();

private:
Document* ContextDocument() const;
Vector<String> IconURLs(Document*, int icon_types_mask) const;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/testing/internals.idl
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@
RecordTest recordTest();
SequenceTest sequenceTest();
UnionTypesTest unionTypesTest();
InternalsUkmRecorder initializeUKMRecorder();
CallbackFunctionTest callbackFunctionTest();
[RaisesException] void setScrollChain(ScrollState scrollState, sequence<Element> elements);

Expand Down Expand Up @@ -446,5 +447,4 @@
void setAllowPerChunkTransferring(ReadableStream stream);

void setBackForwardCacheRestorationBufferSize(long maxSize);

};
Loading

0 comments on commit 962432a

Please sign in to comment.