Commit d6979edd authored by alex linker's avatar alex linker Committed by Commit Bot

Move external protocol dialog to intent handling metrics.

The external protocol dialog class recorded metrics that fall under
the purview of the intent handling metrics class, so it has been
moved to be under that class structure.

BUG=1044710
TEST=ran IntentHandlingMetrics.* unit tests.

Change-Id: I156617bb999ca6072c744e117306b48287144c28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2024388Reviewed-by: default avatarDavid Jacobo <djacobo@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Alex Linker <ajlinker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736173}
parent bb3be552
...@@ -4,8 +4,6 @@ ...@@ -4,8 +4,6 @@
#include "chrome/browser/chromeos/apps/metrics/intent_handling_metrics.h" #include "chrome/browser/chromeos/apps/metrics/intent_handling_metrics.h"
#include <string>
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "chrome/browser/apps/intent_helper/apps_navigation_types.h" #include "chrome/browser/apps/intent_helper/apps_navigation_types.h"
#include "components/arc/metrics/arc_metrics_constants.h" #include "components/arc/metrics/arc_metrics_constants.h"
...@@ -52,4 +50,20 @@ void IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics( ...@@ -52,4 +50,20 @@ void IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics(
RecordIntentPickerMetrics(source, should_persist, action, platform); RecordIntentPickerMetrics(source, should_persist, action, platform);
} }
void IntentHandlingMetrics::RecordExternalProtocolMetrics(
arc::Scheme scheme,
PickerEntryType entry_type,
bool accepted,
bool persisted) {
arc::ProtocolAction action =
arc::GetProtocolAction(scheme, entry_type, accepted, persisted);
if (accepted) {
UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.ExternalProtocolDialog.Accepted",
action);
} else {
UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.ExternalProtocolDialog.Rejected",
action);
}
}
} // namespace apps } // namespace apps
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "chrome/browser/apps/intent_helper/apps_navigation_throttle.h" #include "chrome/browser/apps/intent_helper/apps_navigation_throttle.h"
#include "chrome/browser/apps/intent_helper/apps_navigation_types.h" #include "chrome/browser/apps/intent_helper/apps_navigation_types.h"
#include "chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.h"
#include "components/arc/metrics/arc_metrics_constants.h" #include "components/arc/metrics/arc_metrics_constants.h"
namespace apps { namespace apps {
...@@ -29,6 +30,11 @@ class IntentHandlingMetrics { ...@@ -29,6 +30,11 @@ class IntentHandlingMetrics {
IntentPickerCloseReason close_reason, IntentPickerCloseReason close_reason,
Source source, Source source,
bool should_persist); bool should_persist);
static void RecordExternalProtocolMetrics(arc::Scheme scheme,
apps::PickerEntryType entry_type,
bool accepted,
bool persisted);
}; };
} // namespace apps } // namespace apps
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/test/gtest_util.h" #include "base/test/gtest_util.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "chrome/browser/chromeos/apps/intent_helper/chromeos_apps_navigation_throttle.h" #include "chrome/browser/chromeos/apps/intent_helper/chromeos_apps_navigation_throttle.h"
#include "chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace apps { namespace apps {
...@@ -58,4 +59,16 @@ TEST_F(IntentHandlingMetricsTest, ...@@ -58,4 +59,16 @@ TEST_F(IntentHandlingMetricsTest,
AppsNavigationThrottle::PickerAction::ARC_APP_PREFERRED_PRESSED, 1); AppsNavigationThrottle::PickerAction::ARC_APP_PREFERRED_PRESSED, 1);
} }
TEST_F(IntentHandlingMetricsTest, TestRecordExternalProtocolMetrics) {
base::HistogramTester histogram_tester;
IntentHandlingMetrics test = IntentHandlingMetrics();
test.RecordExternalProtocolMetrics(arc::Scheme::IRC, PickerEntryType::kArc,
/*accepted=*/true, /*persisted=*/true);
histogram_tester.ExpectBucketCount(
"ChromeOS.Apps.ExternalProtocolDialog.Accepted",
arc::ProtocolAction::IRC_ACCEPTED_PERSISTED, 1);
}
} // namespace apps } // namespace apps
...@@ -493,8 +493,8 @@ void OnIntentPickerClosed( ...@@ -493,8 +493,8 @@ void OnIntentPickerClosed(
DCHECK_EQ(apps::IntentPickerCloseReason::OPEN_APP, reason); DCHECK_EQ(apps::IntentPickerCloseReason::OPEN_APP, reason);
DCHECK(!should_persist); DCHECK(!should_persist);
HandleDeviceSelection(web_contents, devices, selected_app_package, url); HandleDeviceSelection(web_contents, devices, selected_app_package, url);
RecordUmaDialogAction(Scheme::TEL, entry_type, /*accepted=*/true, apps::IntentHandlingMetrics::RecordExternalProtocolMetrics(
should_persist); Scheme::TEL, entry_type, /*accepted=*/true, should_persist);
apps::IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics( apps::IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics(
selected_app_package, entry_type, reason, selected_app_package, entry_type, reason,
apps::Source::kExternalProtocol, should_persist); apps::Source::kExternalProtocol, should_persist);
...@@ -588,8 +588,8 @@ void OnIntentPickerClosed( ...@@ -588,8 +588,8 @@ void OnIntentPickerClosed(
auto scheme_it = string_to_scheme.find(scheme); auto scheme_it = string_to_scheme.find(scheme);
if (scheme_it != string_to_scheme.end()) if (scheme_it != string_to_scheme.end())
url_scheme = scheme_it->second; url_scheme = scheme_it->second;
RecordUmaDialogAction(url_scheme, entry_type, protocol_accepted, apps::IntentHandlingMetrics::RecordExternalProtocolMetrics(
should_persist); url_scheme, entry_type, protocol_accepted, should_persist);
apps::IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics( apps::IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics(
selected_app_package, entry_type, reason, apps::Source::kExternalProtocol, selected_app_package, entry_type, reason, apps::Source::kExternalProtocol,
...@@ -824,22 +824,6 @@ void OnIntentPickerClosedForTesting( ...@@ -824,22 +824,6 @@ void OnIntentPickerClosedForTesting(
reason, should_persist); reason, should_persist);
} }
// TODO(crbug.com/1044710): convert this to new scheme.
void RecordUmaDialogAction(Scheme scheme,
apps::PickerEntryType entry_type,
bool accepted,
bool persisted) {
ProtocolAction action =
GetProtocolAction(scheme, entry_type, accepted, persisted);
if (accepted) {
base::UmaHistogramEnumeration(
"ChromeOS.Apps.ExternalProtocolDialog.Accepted", action);
} else {
base::UmaHistogramEnumeration(
"ChromeOS.Apps.ExternalProtocolDialog.Rejected", action);
}
}
ProtocolAction GetProtocolAction(Scheme scheme, ProtocolAction GetProtocolAction(Scheme scheme,
apps::PickerEntryType entry_type, apps::PickerEntryType entry_type,
bool accepted, bool accepted,
......
...@@ -149,11 +149,6 @@ bool GetAndResetSafeToRedirectToArcWithoutUserConfirmationFlagForTesting( ...@@ -149,11 +149,6 @@ bool GetAndResetSafeToRedirectToArcWithoutUserConfirmationFlagForTesting(
bool IsChromeAnAppCandidateForTesting( bool IsChromeAnAppCandidateForTesting(
const std::vector<mojom::IntentHandlerInfoPtr>& handlers); const std::vector<mojom::IntentHandlerInfoPtr>& handlers);
void RecordUmaDialogAction(Scheme scheme,
apps::PickerEntryType entry_type,
bool accepted,
bool persisted);
ProtocolAction GetProtocolAction(Scheme scheme, ProtocolAction GetProtocolAction(Scheme scheme,
apps::PickerEntryType entry_type, apps::PickerEntryType entry_type,
bool accepted, bool accepted,
......
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