Commit 7db9977c authored by François Doray's avatar François Doray Committed by Chromium LUCI CQ

Revert "Record LSS search status metric in trusted frame per search."

This reverts commit 21296da9.

Reason for revert: crbug.com/1166333

Original change's description:
> Record LSS search status metric in trusted frame per search.
>
> Bug: b/170162419
> Change-Id: I862f02fa7949d3c1e06b83c2389ddc63273f2188
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617406
> Commit-Queue: Callistus Tan <callistus@google.com>
> Reviewed-by: dstockwell <dstockwell@google.com>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Brian White <bcwhite@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#842864}

TBR=xiyuan@chromium.org,dstockwell@google.com,bcwhite@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,callistus@google.com

Change-Id: I402f5b44b91661032ef2d2c4eb163ed8d1eca84e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/170162419, 1166333
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626000Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843188}
parent 32e81772
...@@ -31,14 +31,12 @@ ...@@ -31,14 +31,12 @@
#include "chrome/test/base/interactive_test_utils.h" #include "chrome/test/base/interactive_test_utils.h"
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
#include "chromeos/components/help_app_ui/url_constants.h" #include "chromeos/components/help_app_ui/url_constants.h"
#include "chromeos/components/local_search_service/shared_structs.h"
#include "chromeos/components/web_applications/test/sandboxed_web_ui_test_base.h" #include "chromeos/components/web_applications/test/sandboxed_web_ui_test_base.h"
#include "chromeos/constants/chromeos_features.h" #include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h" #include "chromeos/constants/chromeos_switches.h"
#include "components/user_manager/user_names.h" #include "components/user_manager/user_names.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "content/public/test/test_navigation_observer.h" #include "content/public/test/test_navigation_observer.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/display/screen.h" #include "ui/display/screen.h"
#include "ui/display/types/display_constants.h" #include "ui/display/types/display_constants.h"
...@@ -157,75 +155,6 @@ IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2InAppMetrics) { ...@@ -157,75 +155,6 @@ IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2InAppMetrics) {
EXPECT_EQ(1, user_action_tester.GetActionCount("Discover.Help.TabClicked")); EXPECT_EQ(1, user_action_tester.GetActionCount("Discover.Help.TabClicked"));
} }
// Test that the Help App logs metrics on search status.
IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2SearchStatusMetrics) {
WaitForTestSystemAppInstall();
content::WebContents* web_contents = LaunchApp(web_app::SystemAppType::HELP);
base::HistogramTester histogram_tester;
// Use ExecuteScriptAndExtractBool instead of EvalJsInAppFrame because the
// script needs to run in the same world as the page's code.
bool are_results_null;
EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
SandboxedWebUiAppTestBase::GetAppFrame(web_contents), R"(
(async () => {
const res = await DELEGATE.findInSearchIndex('crome');
window.domAutomationController.send(res.results === null);
})();
)",
&are_results_null));
// Since the index has not initialized (which takes ~3.6 seconds), there
// should be no results and one histogram count for kEmptyIndex.
EXPECT_TRUE(are_results_null);
EXPECT_EQ(1,
histogram_tester.GetBucketCount(
"Discover.Search.SearchStatus",
chromeos::local_search_service::ResponseStatus::kEmptyIndex));
int no_of_results;
EXPECT_TRUE(content::ExecuteScriptAndExtractInt(
SandboxedWebUiAppTestBase::GetAppFrame(web_contents), R"(
(async () => {
await DELEGATE.addOrUpdateSearchIndex([{
// Title match. No subheadings.
id: 'test-id-1',
title: 'verycomplicatedsearchtoken',
body: 'Body text',
mainCategoryName: 'Help',
locale: 'en-US',
}]);
const res = await DELEGATE.findInSearchIndex(
'verycomplicatedsearchtoken');
window.domAutomationController.send(res.results.length);
})();
)",
&no_of_results));
// Now that the index has been initialized with a test item, there should be
// 1 result for the above search, and one histogram count for kSuccess.
EXPECT_EQ(1, no_of_results);
EXPECT_EQ(1, histogram_tester.GetBucketCount(
"Discover.Search.SearchStatus",
chromeos::local_search_service::ResponseStatus::kSuccess));
EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
SandboxedWebUiAppTestBase::GetAppFrame(web_contents), R"(
(async () => {
const res = await DELEGATE.findInSearchIndex('');
window.domAutomationController.send(res.results === null);
})();
)",
&are_results_null));
// Searches with empty queries will return {results: null}. Results should be
// null and there should be one histogram count for kEmptyQuery.
EXPECT_TRUE(are_results_null);
EXPECT_EQ(1,
histogram_tester.GetBucketCount(
"Discover.Search.SearchStatus",
chromeos::local_search_service::ResponseStatus::kEmptyQuery));
}
IN_PROC_BROWSER_TEST_P(HelpAppAllProfilesIntegrationTest, HelpAppV2ShowHelp) { IN_PROC_BROWSER_TEST_P(HelpAppAllProfilesIntegrationTest, HelpAppV2ShowHelp) {
WaitForTestSystemAppInstall(); WaitForTestSystemAppInstall();
......
...@@ -29,7 +29,6 @@ js_library("browser_proxy") { ...@@ -29,7 +29,6 @@ js_library("browser_proxy") {
# The privileged context can't access the app, but shares struct definitions # The privileged context can't access the app, but shares struct definitions
# passed over postMessage. # passed over postMessage.
"help_app.externs.js", "help_app.externs.js",
"//third_party/closure_compiler/externs/metrics_private.js",
] ]
deps = [ deps = [
":message_types", ":message_types",
......
...@@ -129,12 +129,6 @@ guestMessagePipe.registerHandler( ...@@ -129,12 +129,6 @@ guestMessagePipe.registerHandler(
const response = await indexRemote.find( const response = await indexRemote.find(
toString16((/** @type {{query: string}} */ (message)).query), toString16((/** @type {{query: string}} */ (message)).query),
/*max_results=*/ 100); /*max_results=*/ 100);
// Record the search status in the trusted frame.
chrome.metricsPrivate.recordEnumerationValue(
'Discover.Search.SearchStatus', response.status,
chromeos.localSearchService.mojom.ResponseStatus.MAX_VALUE);
if (response.status !== if (response.status !==
chromeos.localSearchService.mojom.ResponseStatus.kSuccess || chromeos.localSearchService.mojom.ResponseStatus.kSuccess ||
!response.results) { !response.results) {
......
...@@ -77,7 +77,6 @@ template("generate_expired_histograms_array") { ...@@ -77,7 +77,6 @@ template("generate_expired_histograms_array") {
"//tools/metrics/histograms/histograms_xml/google/histograms.xml", "//tools/metrics/histograms/histograms_xml/google/histograms.xml",
"//tools/metrics/histograms/histograms_xml/gpu/histograms.xml", "//tools/metrics/histograms/histograms_xml/gpu/histograms.xml",
"//tools/metrics/histograms/histograms_xml/hang_watcher/histograms.xml", "//tools/metrics/histograms/histograms_xml/hang_watcher/histograms.xml",
"//tools/metrics/histograms/histograms_xml/help_app/histograms.xml",
"//tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml", "//tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml",
"//tools/metrics/histograms/histograms_xml/history/histograms.xml", "//tools/metrics/histograms/histograms_xml/history/histograms.xml",
"//tools/metrics/histograms/histograms_xml/holding_space/histograms.xml", "//tools/metrics/histograms/histograms_xml/holding_space/histograms.xml",
......
...@@ -308,8 +308,7 @@ ...@@ -308,8 +308,7 @@
"chrome://tab-strip/*", "chrome://tab-strip/*",
"chrome://welcome/*", "chrome://welcome/*",
"chrome://profile-picker/*", "chrome://profile-picker/*",
"chrome://file-manager/*", "chrome://file-manager/*"
"chrome://help-app/*"
] ]
}, { }, {
"channel": "stable", "channel": "stable",
......
...@@ -43,7 +43,6 @@ tools/metrics/histograms/histograms_xml/geolocation/histograms.xml ...@@ -43,7 +43,6 @@ tools/metrics/histograms/histograms_xml/geolocation/histograms.xml
tools/metrics/histograms/histograms_xml/google/histograms.xml tools/metrics/histograms/histograms_xml/google/histograms.xml
tools/metrics/histograms/histograms_xml/gpu/histograms.xml tools/metrics/histograms/histograms_xml/gpu/histograms.xml
tools/metrics/histograms/histograms_xml/hang_watcher/histograms.xml tools/metrics/histograms/histograms_xml/hang_watcher/histograms.xml
tools/metrics/histograms/histograms_xml/help_app/histograms.xml
tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml
tools/metrics/histograms/histograms_xml/history/histograms.xml tools/metrics/histograms/histograms_xml/history/histograms.xml
tools/metrics/histograms/histograms_xml/holding_space/histograms.xml tools/metrics/histograms/histograms_xml/holding_space/histograms.xml
...@@ -118,4 +117,4 @@ tools/metrics/histograms/histograms_xml/web_audio/histograms.xml ...@@ -118,4 +117,4 @@ tools/metrics/histograms/histograms_xml/web_audio/histograms.xml
tools/metrics/histograms/histograms_xml/web_core/histograms.xml tools/metrics/histograms/histograms_xml/web_core/histograms.xml
tools/metrics/histograms/histograms_xml/web_rtc/histograms.xml tools/metrics/histograms/histograms_xml/web_rtc/histograms.xml
tools/metrics/histograms/histograms_xml/weblayer/histograms.xml tools/metrics/histograms/histograms_xml/weblayer/histograms.xml
tools/metrics/histograms/histograms_xml/windows/histograms.xml tools/metrics/histograms/histograms_xml/windows/histograms.xml
\ No newline at end of file
<!--
Copyright 2021 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.
-->
<!--
This file is used to generate a comprehensive list of History histograms
along with a detailed description for each histogram.
For best practices on writing histogram descriptions, see
https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histograms/README.md
Please send CLs to chromium-metrics-reviews@google.com rather than to specific
individuals. These CLs will be automatically reassigned to a reviewer within
about 5 minutes. This approach helps the metrics team to load-balance incoming
reviews. Googlers can read more about this at go/gwsq-gerrit.
-->
<histogram-configuration>
<histograms>
<histogram name="Discover.Search.SearchStatus"
enum="LocalSearchServiceResponseStatus" expires_after="2022-01-08">
<owner>callistus@google.com</owner>
<owner>showoff-eng@google.com</owner>
<summary>
Records the search status when searching with the Local Search Service in
the Discover (internally &quot;Showoff&quot;) app. This is logged once per
user search and will count the number of searches while the index is still
empty, successful searches, etc.
</summary>
</histogram>
</histograms>
</histogram-configuration>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment