Skip to content

Commit

Permalink
Reland "[PageZoom] Add UKM for new zoom value on slider dismissal"
Browse files Browse the repository at this point in the history
This is a reland of commit 3d3d5d0

This reland has been updated to include bucketing in intervals of 5
for the zoom value and a unit test.

See go/clank-page-zoom-design for the feature design document and
UKM request, as well as approval from chrome-privacy-core.

Original change's description:
> [PageZoom] Add UKM for new zoom value on slider dismissal
>
> This CL adds logging for UKM at the same point when existing UMA
> is logged: at dismissal of the Page Zoom slider if the user
> changed the slider value before dismissal.
>
> See UKM Collection in the Privacy Considerations section at
> go/clank-page-zoom-design for doc and UKM request.
>
> Bug: 1232536
> Fixed: 1402526
> Change-Id: I431d0a05efc1f841650340503f6a3c274eb75307
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4094081
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Mark Schillaci <mschillaci@google.com>
> Commit-Queue: Amanda Lin Dietz <aldietz@google.com>
> Cr-Commit-Position: refs/heads/main@{#1085286}

(cherry picked from commit 60d1bf2)

Bug: 1232536, 1402526
Change-Id: I55054d0ebbfd1085de7ed939fa528d892a352543
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4118930
Reviewed-by: Yue Ru Sun <yrsun@chromium.org>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Auto-Submit: Amanda Lin Dietz <aldietz@google.com>
Commit-Queue: Amanda Lin Dietz <aldietz@google.com>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1091057}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4163259
Reviewed-by: Mark Schillaci <mschillaci@google.com>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5481@{#248}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
aldietz authored and Chromium LUCI CQ committed Jan 12, 2023
1 parent 4cca7c5 commit 8c6ce10
Show file tree
Hide file tree
Showing 10 changed files with 213 additions and 1 deletion.
1 change: 1 addition & 0 deletions components/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ test("components_unittests") {
deps += [
"//base:base_java_unittest_support",
"//components/android_autofill/browser:unit_tests",
"//components/browser_ui/accessibility/android:unit_tests",
"//components/browser_ui/sms/android:unit_tests",
"//components/cdm/browser:unit_tests",
"//components/component_updater/android:embedded_component_loader_java",
Expand Down
2 changes: 2 additions & 0 deletions components/browser_ui/accessibility/DEPS
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
include_rules = [
"+components/prefs",
"+components/ukm",
"+components/user_prefs",
"+content/public/android/java",
"+content/public/browser",
"+services/metrics/public/cpp",
"+ui/android",
]
19 changes: 18 additions & 1 deletion components/browser_ui/accessibility/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,26 @@
import("//build/config/android/rules.gni")

generate_jni("accessibility_jni_headers") {
sources = [ "java/src/org/chromium/components/browser_ui/accessibility/FontSizePrefs.java" ]
sources = [
"java/src/org/chromium/components/browser_ui/accessibility/FontSizePrefs.java",
"java/src/org/chromium/components/browser_ui/accessibility/PageZoomMetrics.java",
]
}

source_set("android") {
sources = [
"font_size_prefs_android.cc",
"font_size_prefs_android.h",
"page_zoom_metrics.cc",
"page_zoom_metrics.h",
]
deps = [
":accessibility_jni_headers",
"//base",
"//components/prefs",
"//components/user_prefs",
"//content/public/browser",
"//services/metrics/public/cpp:ukm_builders",
]
}

Expand All @@ -34,6 +40,7 @@ android_library("java") {
"java/src/org/chromium/components/browser_ui/accessibility/FontSizePrefs.java",
"java/src/org/chromium/components/browser_ui/accessibility/PageZoomCoordinator.java",
"java/src/org/chromium/components/browser_ui/accessibility/PageZoomMediator.java",
"java/src/org/chromium/components/browser_ui/accessibility/PageZoomMetrics.java",
"java/src/org/chromium/components/browser_ui/accessibility/PageZoomPreference.java",
"java/src/org/chromium/components/browser_ui/accessibility/PageZoomProperties.java",
"java/src/org/chromium/components/browser_ui/accessibility/PageZoomUma.java",
Expand Down Expand Up @@ -97,3 +104,13 @@ robolectric_library("junit") {
"//ui/android:ui_no_recycler_view_java",
]
}

source_set("unit_tests") {
testonly = true
sources = [ "page_zoom_metrics_unittest.cc" ]
deps = [
"//components/browser_ui/accessibility/android",
"//components/ukm:test_support",
"//testing/gtest",
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public void hide() {
// Ensure that the user has set a zoom value during this session.
double zoomValue = mMediator.latestZoomValue();
if (zoomValue != 0.0) {
mMediator.logZoomLevelUKM(zoomValue);
PageZoomUma.logAppMenuSliderZoomLevelChangedHistogram();
PageZoomUma.logAppMenuSliderZoomLevelValueHistogram(zoomValue);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ protected double latestZoomValue() {
return mLatestZoomValue;
}

/**
* Logs UKM for the user changing the zoom level on the page from the slider.
*/
protected void logZoomLevelUKM(double value) {
PageZoomMetrics.logZoomLevelUKM(mWebContents, value);
}

@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
void handleDecreaseClicked(Void unused) {
// When decreasing zoom, "snap" to the greatest preset value that is less than the current.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.components.browser_ui.accessibility;

import androidx.annotation.NonNull;

import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.content_public.browser.WebContents;

/**
* Centralizes metrics data collection for Page Zoom.
*/
@JNINamespace("browser_ui")
public class PageZoomMetrics {
/**
* Logs new zoom level UKM for the given web contents.
* Recorded on slider dismissal if the user chose a new value.
* @param webContents WebContents for which to log value
* @param newZoomLevel double - new zoom level
*/
public static void logZoomLevelUKM(@NonNull WebContents webContents, double newZoomLevel) {
PageZoomMetricsJni.get().logZoomLevelUKM(webContents, newZoomLevel);
}

@NativeMethods
public interface Natives {
void logZoomLevelUKM(WebContents webContents, double value);
}
}
49 changes: 49 additions & 0 deletions components/browser_ui/accessibility/android/page_zoom_metrics.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/browser_ui/accessibility/android/page_zoom_metrics.h"

#include "base/android/scoped_java_ref.h"
#include "components/browser_ui/accessibility/android/accessibility_jni_headers/PageZoomMetrics_jni.h"
#include "content/public/browser/web_contents.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"

using base::android::JavaParamRef;
using base::android::JavaRef;

namespace {
int ValueToStep(double value) {
return (static_cast<int>(round(100 * value)) / 5) * 5;
}
} // namespace

namespace browser_ui {

void JNI_PageZoomMetrics_LogZoomLevelUKM(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& j_web_contents,
jdouble new_zoom_level) {
content::WebContents* web_contents =
content::WebContents::FromJavaWebContents(j_web_contents);
DCHECK(web_contents);

ukm::SourceId ukm_source_id =
web_contents->GetPrimaryMainFrame()->GetPageUkmSourceId();

PageZoomMetrics::LogZoomLevelUKMHelper(ukm_source_id, new_zoom_level,
ukm::UkmRecorder::Get());
}

// static
void PageZoomMetrics::LogZoomLevelUKMHelper(ukm::SourceId ukm_source_id,
double new_zoom_level,
ukm::UkmRecorder* ukm_recorder) {
ukm::builders::Accessibility_PageZoom(ukm_source_id)
.SetSliderZoomValue(ValueToStep(new_zoom_level))
.Record(ukm_recorder);
}

} // namespace browser_ui
37 changes: 37 additions & 0 deletions components/browser_ui/accessibility/android/page_zoom_metrics.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_BROWSER_UI_ACCESSIBILITY_ANDROID_PAGE_ZOOM_METRICS_H_
#define COMPONENTS_BROWSER_UI_ACCESSIBILITY_ANDROID_PAGE_ZOOM_METRICS_H_

#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_source_id.h"

namespace content {
class WebContents;
}

namespace browser_ui {

/*
* Native component to log metrics for Page Zoom.
*/
class PageZoomMetrics {
public:
PageZoomMetrics();
~PageZoomMetrics();

// Logs UKM with the current zoom level for the specified WebContents.
void LogZoomLevelUKM(content::WebContents* web_contents,
double new_zoom_level);

// Helper function for UKM logging
static void LogZoomLevelUKMHelper(ukm::SourceId ukm_source_id,
double new_zoom_level,
ukm::UkmRecorder* ukm_recorder);
};

} // namespace browser_ui

#endif // COMPONENTS_BROWSER_UI_ACCESSIBILITY_ANDROID_PAGE_ZOOM_METRICS_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/browser_ui/accessibility/android/page_zoom_metrics.h"

#include "components/ukm/test_ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace browser_ui {

TEST(PageZoomMetricsTest, PageZoomUkmExactValue) {
ukm::TestUkmRecorder test_recorder;
ukm::SourceId mock_source_id = test_recorder.GetNewSourceID();

PageZoomMetrics::LogZoomLevelUKMHelper(mock_source_id, 0.75, &test_recorder);

base::StringPiece expectedMetricName = "SliderZoomValue";

auto entries = test_recorder.GetEntriesByName(
ukm::builders::Accessibility_PageZoom::kEntryName);
ASSERT_EQ(entries.size(), 1u);
test_recorder.ExpectEntryMetric(entries.front(), expectedMetricName, 75);
}

TEST(PageZoomMetricsTest, PageZoomUkmBucket) {
ukm::TestUkmRecorder test_recorder;
ukm::SourceId mock_source_id = test_recorder.GetNewSourceID();

PageZoomMetrics::LogZoomLevelUKMHelper(mock_source_id, 0.78, &test_recorder);

base::StringPiece expectedMetricName = "SliderZoomValue";

auto entries = test_recorder.GetEntriesByName(
ukm::builders::Accessibility_PageZoom::kEntryName);
ASSERT_EQ(entries.size(), 1u);
test_recorder.ExpectEntryMetric(entries.front(), expectedMetricName, 75);
}

} // namespace browser_ui
25 changes: 25 additions & 0 deletions tools/metrics/ukm/ukm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,31 @@ be describing additional metrics about the same event.
</metric>
</event>

<event name="Accessibility.PageZoom">
<owner>aldietz@google.com</owner>
<owner>mschillaci@google.com</owner>
<owner>chrome-a11y-core@google.com</owner>
<summary>
Tracks usage of Page Zoom on Chrome for Android. Recorded each time the
slider is dismissed if the user changed the zoom level on this page before
dismissal.
</summary>
<metric name="SliderZoomValue">
<summary>
For users who changed the individual zoom level on this page, tracks what
zoom level has been set on the page at slider dismissal. There may be
multiple instances where the slider is opened on the same page, so there
may be multiple values logged per page. The value will be an integer
between 25 and 500, inclusive, bucketed in intervals of 5.
</summary>
<aggregation>
<history>
<statistics/>
</history>
</aggregation>
</metric>
</event>

<event name="Accessibility.Renderer">
<owner>aleventhal@chromium.org</owner>
<owner>nektar@chromium.org</owner>
Expand Down

0 comments on commit 8c6ce10

Please sign in to comment.