Skip to content

Commit

Permalink
[ntp-modules] V2 metrics
Browse files Browse the repository at this point in the history 
Adds logic to output existing Modules metrics.
Defines and outputs a new NewTabPage.Modules.InstanceCount metric.

Bug: 1444758
Change-Id: Ie9bf211b6cecc01810a0aac5087334585267a9d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4779588
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Roman Arora <romanarora@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184328}
  • Loading branch information
Roman Arora authored and Chromium LUCI CQ committed Aug 16, 2023
1 parent 244f435 commit c3c783e
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 40 deletions.
37 changes: 34 additions & 3 deletions chrome/browser/resources/new_tab_page/modules/v2/modules.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {PolymerElement, TemplateInstanceBase, templatize} from 'chrome://resourc
import {loadTimeData} from '../../i18n_setup.js';
import {NewTabPageProxy} from '../../new_tab_page_proxy.js';
import {WindowProxy} from '../../window_proxy.js';
import {Module} from '../module_descriptor.js';
import {ModuleRegistry} from '../module_registry.js';
import {ModuleInstance, ModuleWrapperElement} from '../module_wrapper.js';

Expand Down Expand Up @@ -44,6 +45,8 @@ const CONTAINER_GAP_WIDTH = 8;

const MARGIN_WIDTH = 48;

const METRIC_NAME_MODULE_DISABLED = 'NewTabPage.Modules.Disabled';

export type UndoActionEvent =
CustomEvent<{message: string, restoreCallback?: () => void}>;
export type DismissModuleInstanceEvent = UndoActionEvent;
Expand Down Expand Up @@ -230,11 +233,34 @@ export class ModulesV2Element extends PolymerElement {

chrome.metricsPrivate.recordSmallCount(
'NewTabPage.Modules.LoadedModulesCount', modules.length);
// TODO(crbug.com/1444758): Add module instances count metric.
modulesIdNames.forEach(({id}) => {
chrome.metricsPrivate.recordBoolean(
`NewTabPage.Modules.EnabledOnNTPLoad.${id}`,
!this.disabledModules_.all &&
!this.disabledModules_.ids.includes(id));
});
chrome.metricsPrivate.recordSmallCount(
'NewTabPage.Modules.InstanceCount', this.templateInstances_.length);
chrome.metricsPrivate.recordBoolean(
'NewTabPage.Modules.VisibleOnNTPLoad', !this.disabledModules_.all);
this.recordModuleLoadedWithModules_(modules);
this.dispatchEvent(new Event('modules-loaded'));
}
}

private recordModuleLoadedWithModules_(modules: Module[]) {
const moduleDescriptorIds = modules.map(m => m.descriptor.id);

for (const moduleDescriptorId of moduleDescriptorIds) {
moduleDescriptorIds.forEach(id => {
if (id !== moduleDescriptorId) {
chrome.metricsPrivate.recordSparseValueWithPersistentHash(
`NewTabPage.Modules.LoadedWith.${moduleDescriptorId}`, id);
}
});
}
}

private forwardHostProp_(property: string, value: any) {
this.templateInstances_.forEach(instance => {
instance.forwardHostProp(property, value);
Expand Down Expand Up @@ -311,9 +337,9 @@ export class ModulesV2Element extends PolymerElement {
NewTabPageProxy.getInstance().handler.setModuleDisabled(id, true);
this.$.undoToast.show();
chrome.metricsPrivate.recordSparseValueWithPersistentHash(
'NewTabPage.Modules.Disabled', id);
METRIC_NAME_MODULE_DISABLED, id);
chrome.metricsPrivate.recordSparseValueWithPersistentHash(
'NewTabPage.Modules.Disabled.ModuleRequest', id);
`${METRIC_NAME_MODULE_DISABLED}.ModuleRequest`, id);
}

private onDisabledModulesChange_() {
Expand All @@ -337,12 +363,17 @@ export class ModulesV2Element extends PolymerElement {
this.$.container.insertBefore(
wrapper, this.$.container.childNodes[index]);
restoreCallback();
chrome.metricsPrivate.recordSparseValueWithPersistentHash(
'NewTabPage.Modules.Restored', wrapper.module.descriptor.id);
} :
undefined,
};

// Notify the user.
this.$.undoToast.show();

chrome.metricsPrivate.recordSparseValueWithPersistentHash(
'NewTabPage.Modules.Dismissed', wrapper.module.descriptor.id);
}

private onUndoButtonClick_() {
Expand Down
116 changes: 79 additions & 37 deletions chrome/test/data/webui/new_tab_page/modules/v2/modules_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,19 @@ suite('NewTabPageModulesModulesV2Test', () => {
});

async function createModulesElement(
modules: Module[], width: number): Promise<ModulesV2Element> {
const modulesPromise = Promise.resolve(modules);
modules: Module[], enabled: boolean, width: number) {
if (!enabled) {
assertTrue(
modules.length === 0,
'modules array must be empty if modules disabled');
}
const modulesPromise = new Promise<Module[]>((resolve, _) => {
callbackRouterRemote.setDisabledModules(!enabled, []);
callbackRouterRemote.$.flushForTesting().then(() => {
resolve(modules);
});
});

moduleRegistry.setResultFor('initializeModulesHavingIds', modulesPromise);
const element = new ModulesV2Element();
document.body.style.width = `${width}px`;
Expand Down Expand Up @@ -149,9 +160,7 @@ suite('NewTabPageModulesModulesV2Test', () => {
elements: Array(scenario.count).fill(0).map(_ => createElement()),
},
],
scenario.width);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();
true, scenario.width);
await waitAfterNextRender(modulesElement);

const wrappers = modulesElement.shadowRoot!.querySelectorAll(
Expand All @@ -173,7 +182,7 @@ suite('NewTabPageModulesModulesV2Test', () => {
});
});

test('single module multiple element instances', async () => {
test('No modules rendered when all disabled', async () => {
const fooDescriptor = new ModuleDescriptor('foo', initNullModule);
const barDescriptor = new ModuleDescriptor('bar', initNullModule);
handler.setResultFor('getModulesIdNames', {
Expand All @@ -182,27 +191,62 @@ suite('NewTabPageModulesModulesV2Test', () => {
{id: barDescriptor.id, name: barDescriptor.id},
],
});

const modulesElement = await createModulesElement(
[
{
descriptor: fooDescriptor,
elements: Array(3).fill(0).map(_ => createElement()),
},
{
descriptor: barDescriptor,
elements: [createElement()],
},
],
SAMPLE_SCREEN_WIDTH);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();
const modulesElement =
await createModulesElement([], false, SAMPLE_SCREEN_WIDTH);
await waitAfterNextRender(modulesElement);

const moduleWrappers =
modulesElement.shadowRoot!.querySelectorAll('ntp-module-wrapper');
assertEquals(4, moduleWrappers.length);
assertEquals(0, moduleWrappers.length);
assertEquals(1, metrics.count('NewTabPage.Modules.LoadedModulesCount', 0));
assertEquals(1, metrics.count('NewTabPage.Modules.InstanceCount', 0));
assertEquals(
1, metrics.count('NewTabPage.Modules.VisibleOnNTPLoad', false));
});

test(
'module(s) with multiple element instances render correcly', async () => {
const fooDescriptor = new ModuleDescriptor('foo', initNullModule);
const barDescriptor = new ModuleDescriptor('bar', initNullModule);
handler.setResultFor('getModulesIdNames', {
data: [
{id: fooDescriptor.id, name: fooDescriptor.id},
{id: barDescriptor.id, name: barDescriptor.id},
],
});

const modulesElement = await createModulesElement(
[
{
descriptor: fooDescriptor,
elements: Array(3).fill(0).map(_ => createElement()),
},
{
descriptor: barDescriptor,
elements: [createElement()],
},
],
true, SAMPLE_SCREEN_WIDTH);

const moduleWrappers =
modulesElement.shadowRoot!.querySelectorAll('ntp-module-wrapper');
assertEquals(4, moduleWrappers.length);
assertEquals(1, metrics.count('NewTabPage.Modules.LoadedModulesCount'));
assertEquals(1, metrics.count('NewTabPage.Modules.InstanceCount', 4));
assertEquals(
1, metrics.count('NewTabPage.Modules.VisibleOnNTPLoad', true));

// Assert metrics for module loaded with other modules.
assertEquals(
1, metrics.count('NewTabPage.Modules.LoadedWith.foo', 'bar'));
assertEquals(
0, metrics.count('NewTabPage.Modules.LoadedWith.foo', 'foo'));
assertEquals(
1, metrics.count('NewTabPage.Modules.LoadedWith.bar', 'foo'));
assertEquals(
0, metrics.count('NewTabPage.Modules.LoadedWith.bar', 'bar'));
});

enum UndoStrategy {
BUTTON_ACTIVATION = 'button activation',
SHORTCUT_KEY = 'shortcut key',
Expand All @@ -225,9 +269,7 @@ suite('NewTabPageModulesModulesV2Test', () => {
descriptor: fooDescriptor,
elements: [createElement()],
}],
SAMPLE_SCREEN_WIDTH);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();
true, SAMPLE_SCREEN_WIDTH);

// Assert.
const moduleWrappers =
Expand Down Expand Up @@ -317,9 +359,7 @@ suite('NewTabPageModulesModulesV2Test', () => {
descriptor: fooDescriptor,
elements: [createElement()],
}],
SAMPLE_SCREEN_WIDTH);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();
true, SAMPLE_SCREEN_WIDTH);

let moduleWrappers = modulesElement.shadowRoot!.querySelectorAll(
'ntp-module-wrapper');
Expand All @@ -346,6 +386,9 @@ suite('NewTabPageModulesModulesV2Test', () => {
.length);
assertTrue(modulesElement.$.undoToast.open);
assertFalse(restoreCalled);
assertEquals(
1, metrics.count('NewTabPage.Modules.Dismissed', 'foo'),
'Dismiss metric value');

await waitAfterNextRender(modulesElement);
if (undoStrategy === UndoStrategy.BUTTON_ACTIVATION) {
Expand All @@ -364,6 +407,9 @@ suite('NewTabPageModulesModulesV2Test', () => {
assertEquals(1, moduleWrappers.length);
assertFalse(modulesElement.$.undoToast.open);
assertTrue(restoreCalled);
assertEquals(
1, metrics.count('NewTabPage.Modules.Restored', 'foo'),
'Restore metric value');
});
});

Expand All @@ -380,9 +426,7 @@ suite('NewTabPageModulesModulesV2Test', () => {
descriptor: fooDescriptor,
elements: [createElement()],
}],
SAMPLE_SCREEN_WIDTH);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();
true, SAMPLE_SCREEN_WIDTH);
await waitAfterNextRender(modulesElement);

// Act.
Expand Down Expand Up @@ -476,9 +520,8 @@ suite('NewTabPageModulesModulesV2Test', () => {
}),
});
const modulesElement =
await createModulesElement(modules, SAMPLE_SCREEN_WIDTH);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();
await createModulesElement(modules, true, SAMPLE_SCREEN_WIDTH);
await waitAfterNextRender(modulesElement);

const moduleWrappers =
Array.from(
Expand Down Expand Up @@ -569,9 +612,8 @@ suite('NewTabPageModulesModulesV2Test', () => {
}),
});
const modulesElement =
await createModulesElement(modules, SAMPLE_SCREEN_WIDTH);
callbackRouterRemote.setDisabledModules(false, []);
await callbackRouterRemote.$.flushForTesting();
await createModulesElement(modules, true, SAMPLE_SCREEN_WIDTH);
await waitAfterNextRender(modulesElement);

const moduleWrappers =
Array.from(
Expand Down
13 changes: 13 additions & 0 deletions tools/metrics/histograms/metadata/new_tab_page/histograms.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1237,6 +1237,19 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="NewTabPage.Modules.InstanceCount" units="count"
expires_after="2024-01-14">
<owner>danpeng@google.com</owner>
<owner>romanarora@chromium.org</owner>
<owner>chrome-desktop-ntp@google.com</owner>
<summary>
The number of rendered module instances on the NTP. Only logged on the 1P
NTP. Note that even if the user has Google as their default search engine,
Incognito and Guest mode NTPs are not considered 1P and don't log this
histogram.
</summary>
</histogram>

<histogram name="NewTabPage.Modules.LoadDuration" units="ms"
expires_after="2024-02-04">
<owner>danpeng@google.com</owner>
Expand Down

0 comments on commit c3c783e

Please sign in to comment.