Skip to content

Commit

Permalink
longpress diacritics: Add metric for char code for diacritics.
Browse files Browse the repository at this point in the history
Using sparse histogram so arbitrary integer values are okay even if not
enumerated in the enum.xml. I enumerated all 68 diacritical character
char codes in the xml for easy metric analysis.

Bug: b/241729161
Change-Id: I8ff81d1fd37e749aab418449d789354ebabc0559
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3825680
Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: Keith Lee <keithlee@chromium.org>
Auto-Submit: jhtin <jhtin@chromium.org>
Commit-Queue: jhtin <jhtin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034972}
  • Loading branch information
jhtin authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent e659fc8 commit d3a5afc
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "base/containers/flat_map.h"
#include "base/logging.h"
#include "base/metrics/histogram_functions.h"
#include "base/no_destructor.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
Expand Down Expand Up @@ -43,6 +44,21 @@ AssistiveWindowButton CreateButtonFor(size_t index,
return button;
}

void RecordAcceptanceCharCodeMetric(const std::u16string diacritic) {
// Recording -1 as default value just in case there are issues with
// encoding in utf-16 that means some character isn't
// properly captured in one utf-16 char (for example if emojis are added in
// the future).
int char_code = -1;
if (diacritic.length() == 1) {
char_code = int(diacritic[0]);
}

base::UmaHistogramSparse(
"InputMethod.PhysicalKeyboard.LongpressDiacritics.AcceptedChar",
char_code);
}

} // namespace

LongpressDiacriticsSuggester::LongpressDiacriticsSuggester(
Expand Down Expand Up @@ -200,7 +216,7 @@ bool LongpressDiacriticsSuggester::AcceptSuggestion(size_t index) {
LOG(ERROR) << "Failed to accept suggestion. " << error;
return false;
}

RecordAcceptanceCharCodeMetric(current_suggestions[index]);
Reset();
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/test/metrics/histogram_tester.h"
#include "chrome/browser/ash/input_method/fake_suggestion_handler.h"
#include "chrome/browser/ash/input_method/suggestion_enums.h"
#include "chrome/test/base/testing_profile.h"
Expand Down Expand Up @@ -35,6 +36,19 @@ using AssistiveWindowButton = ui::ime::AssistiveWindowButton;

const int kContextId = 24601;

const auto kDigitToDomCode = base::MakeFixedFlatMap<int, ui::DomCode>({
{0, ui::DomCode::DIGIT0},
{1, ui::DomCode::DIGIT1},
{2, ui::DomCode::DIGIT2},
{3, ui::DomCode::DIGIT3},
{4, ui::DomCode::DIGIT4},
{5, ui::DomCode::DIGIT5},
{6, ui::DomCode::DIGIT6},
{7, ui::DomCode::DIGIT7},
{8, ui::DomCode::DIGIT8},
{9, ui::DomCode::DIGIT9},
});

ui::KeyEvent CreateKeyEventFromCode(const ui::DomCode& code) {
return ui::KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_UNKNOWN, code, ui::EF_NONE,
ui::DomKey::NONE, ui::EventTimeForNow());
Expand Down Expand Up @@ -623,6 +637,32 @@ TEST_P(LongpressDiacriticsSuggesterTest, ReturnsDiacriticsProposeActionType) {
AssistiveType::kLongpressDiacritics);
}

TEST_P(LongpressDiacriticsSuggesterTest, RecordsAcceptanceCharCodeMetric) {
base::HistogramTester histogram_tester;

FakeSuggestionHandler suggestion_handler;
LongpressDiacriticsSuggester suggester =
LongpressDiacriticsSuggester(&suggestion_handler);
suggester.OnFocus(kContextId);

int histogram_accept_count = 0;
for (int i = 0; i < 9 && i < GetParam().candidates.size(); i++) {
// Insert using dom code for index + 1 (i.e. DIGIT1 inserts 0th index
// candidate)
ui::DomCode dom_code = kDigitToDomCode.find(i + 1)->second;
suggester.TrySuggestOnLongpress(GetParam().longpress_char);
suggester.HandleKeyEvent(CreateKeyEventFromCode(dom_code));

histogram_tester.ExpectTotalCount(
"InputMethod.PhysicalKeyboard.LongpressDiacritics.AcceptedChar",
++histogram_accept_count);
int char_code = int(GetParam().candidates[i][0]);
histogram_tester.ExpectBucketCount(
"InputMethod.PhysicalKeyboard.LongpressDiacritics.AcceptedChar",
char_code, 1);
}
}

INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
LongpressDiacriticsSuggesterTest,
Expand Down
77 changes: 77 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -50054,6 +50054,83 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
<int value="2" label="UNDO"/>
</enum>

<enum name="IMEPKLongpressDiacriticUnicodeCharacterCode">
<!--
Do not add the actual diacritical or non basic ascii characters to the text of this file.
Unsupported non-ascii characters will cause issues with xml presubmits.
-->

<int value="-1" label="UNKOWN CHARACTER"/>
<int value="192" label="Latin Capital Letter A with grave"/>
<int value="193" label="Latin Capital letter A with acute"/>
<int value="194" label="Latin Capital letter A with circumflex"/>
<int value="195" label="Latin Capital letter A with tilde"/>
<int value="196" label="Latin Capital letter A with diaeresis"/>
<int value="197" label="Latin Capital letter A with ring above"/>
<int value="198" label="Latin Capital letter AE"/>
<int value="199" label="Latin Capital letter C with cedilla"/>
<int value="200" label="Latin Capital letter E with grave"/>
<int value="201" label="Latin Capital letter E with acute"/>
<int value="202" label="Latin Capital letter E with circumflex"/>
<int value="203" label="Latin Capital letter E with diaeresis"/>
<int value="204" label="Latin Capital letter I with grave"/>
<int value="205" label="Latin Capital letter I with acute"/>
<int value="206" label="Latin Capital letter I with circumflex"/>
<int value="207" label="Latin Capital letter I with diaeresis"/>
<int value="209" label="Latin Capital letter N with tilde"/>
<int value="210" label="Latin Capital letter O with grave"/>
<int value="211" label="Latin Capital letter O with acute"/>
<int value="212" label="Latin Capital letter O with circumflex"/>
<int value="213" label="Latin Capital letter O with tilde"/>
<int value="214" label="Latin Capital letter O with diaeresis"/>
<int value="216" label="Latin Capital letter O with stroke"/>
<int value="217" label="Latin Capital letter U with grave"/>
<int value="218" label="Latin Capital letter U with acute"/>
<int value="219" label="Latin Capital Letter U with circumflex"/>
<int value="220" label="Latin Capital Letter U with diaeresis"/>
<int value="223" label="Latin Small Letter sharp S"/>
<int value="224" label="Latin Small Letter A with grave"/>
<int value="225" label="Latin Small Letter A with acute"/>
<int value="226" label="Latin Small Letter A with circumflex"/>
<int value="227" label="Latin Small Letter A with tilde"/>
<int value="228" label="Latin Small Letter A with diaeresis"/>
<int value="229" label="Latin Small Letter A with ring above"/>
<int value="230" label="Latin Small Letter AE"/>
<int value="231" label="Latin Small Letter C with cedilla"/>
<int value="232" label="Latin Small Letter E with grave"/>
<int value="233" label="Latin Small Letter E with acute"/>
<int value="234" label="Latin Small Letter E with circumflex"/>
<int value="235" label="Latin Small Letter E with diaeresis"/>
<int value="236" label="Latin Small Letter I with grave"/>
<int value="237" label="Latin Small Letter I with acute"/>
<int value="238" label="Latin Small Letter I with circumflex"/>
<int value="239" label="Latin Small Letter I with diaeresis"/>
<int value="241" label="Latin Small Letter N with tilde"/>
<int value="242" label="Latin Small Letter O with grave"/>
<int value="243" label="Latin Small Letter O with acute"/>
<int value="244" label="Latin Small Letter O with circumflex"/>
<int value="245" label="Latin Small Letter O with tilde"/>
<int value="246" label="Latin Small Letter O with diaeresis"/>
<int value="248" label="Latin Small Letter O with stroke"/>
<int value="249" label="Latin Small Letter U with grave"/>
<int value="250" label="Latin Small Letter U with acute"/>
<int value="251" label="Latin Small Letter U with circumflex"/>
<int value="252" label="Latin Small Letter U with diaeresis"/>
<int value="256" label="Latin Capital Letter A with macron"/>
<int value="257" label="Latin Small Letter A with macron"/>
<int value="274" label="Latin Capital Letter E with macron"/>
<int value="275" label="Latin Small Letter E with macron"/>
<int value="298" label="Latin Capital Letter I with macron"/>
<int value="299" label="Latin Small Letter I with macron"/>
<int value="332" label="Latin Capital Letter O with macron"/>
<int value="333" label="Latin Small Letter O with macron"/>
<int value="338" label="Latin Capital Ligature OE"/>
<int value="339" label="Latin Small Ligature OE"/>
<int value="362" label="Latin Capital Letter U with macron"/>
<int value="363" label="Latin Small Letter U with macron"/>
<int value="7838" label="Capital sharp s, German"/>
</enum>

<enum name="IMERegisterProxyView">
<obsolete>
Removed as of Jan 2020.
Expand Down
11 changes: 11 additions & 0 deletions tools/metrics/histograms/metadata/input/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,17 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram name="InputMethod.PhysicalKeyboard.LongpressDiacritics.AcceptedChar"
enum="IMEPKLongpressDiacriticUnicodeCharacterCode"
expires_after="2023-08-08">
<owner>jhtin@chromium.org</owner>
<owner>essential-inputs-team@google.com</owner>
<summary>
The unicode code point for the diacritic or character variant accepted by a
user using longpress diacritics on physical keyboard.
</summary>
</histogram>

<histogram name="InputMethod.PkCommit.Index" units="units"
expires_after="2021-10-10">
<owner>shend@chromium.org</owner>
Expand Down

0 comments on commit d3a5afc

Please sign in to comment.