Uses Hash values for sparse histograms for InputMethod.ID statistics.

Uses Hash values for sparse histograms for InputMethod.ID statistics.

So that the ID mapping can be stable when adding more and more input
methods.

BUG=352459
TEST=None

Committed: https://crrev.com/254e75d880369bf49bdf0e8ccc400ab71f6f2fee
Cr-Commit-Position: refs/heads/master@{#339225}

[Shu Chen, 2015-07-16 08:17:00]

shuchen@chromium.org changed reviewers:
+ isherman@chromium.org, nona@chromium.org

[Shu Chen, 2015-07-16 08:17:01]

Guys, can you please review this? Thanks

[Seigo Nonaka, 2015-07-16 08:24:47]

Please address one nit style error.

https://codereview.chromium.org/1237493003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/input_method/input_method_manager_impl.cc (right):

https://codereview.chromium.org/1237493003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:870:
stat_id_map_[id] = (int)base::SuperFastHash(id.c_str(), id.length());
nit: please use static_cast<int>

[Seigo Nonaka, 2015-07-16 08:25:03]

lgtm, please wait Ilya's review.

[Shu Chen, 2015-07-16 08:57:27]

Thanks for the review!

https://codereview.chromium.org/1237493003/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/input_method/input_method_manager_impl.cc (right):

https://codereview.chromium.org/1237493003/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:870:
stat_id_map_[id] = (int)base::SuperFastHash(id.c_str(), id.length());
On 2015/07/16 08:24:47, Seigo Nonaka wrote:
> nit: please use static_cast<int>

Done.

[Ilya Sherman, 2015-07-17 01:43:58]

Thanks!

https://codereview.chromium.org/1237493003/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/input_method/input_method_manager_impl.cc (right):

https://codereview.chromium.org/1237493003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:871:
static_cast<int>(base::SuperFastHash(id.c_str(), id.length()));
nit: Any reason not to call this as "base::Hash(id)"?  If the lack of
BASE_EXPORT is the problem, I bet it's fine to add it -- I see no reason why the
convenience wrappers would not be exported.

https://codereview.chromium.org/1237493003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:871:
static_cast<int>(base::SuperFastHash(id.c_str(), id.length()));
nit: Rather than static_cast<int>, please use base::strict_cast, and cast to
int32_t or base::HistogramBase::Sample.

https://codereview.chromium.org/1237493003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:871:
static_cast<int>(base::SuperFastHash(id.c_str(), id.length()));
Do you actually need this map anymore?  I think you could just directly hash at
the histogram logging site.

https://codereview.chromium.org/1237493003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1237493003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13729: +    please see InputMethod.ID2.
nit: Please mark this histogram as <obsolete>, and move the deprecation message
into the <obsolete> tag.

https://codereview.chromium.org/1237493003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:59355: +  <int value="1763643144"
label="xkb:lt::lit">keyboard_lithuanian</int>
nit: Could you please change these labels to be more like "Lithuanian", rather
than "keyboard_lithuanian"?  The latter is considerably less readable.  I
realize it's a little tedious, but I bet a simple Python script could automate
90% of the lines, leaving just a few for tedious manual work.

[Shu Chen, 2015-07-17 03:55:50]

https://codereview.chromium.org/1237493003/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/input_method/input_method_manager_impl.cc (right):

https://codereview.chromium.org/1237493003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:871:
static_cast<int>(base::SuperFastHash(id.c_str(), id.length()));
On 2015/07/17 01:43:58, Ilya Sherman (Away 7.18-8.03) wrote:
> Do you actually need this map anymore?  I think you could just directly hash
at
> the histogram logging site.

Done.

https://codereview.chromium.org/1237493003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:871:
static_cast<int>(base::SuperFastHash(id.c_str(), id.length()));
On 2015/07/17 01:43:58, Ilya Sherman (Away 7.18-8.03) wrote:
> nit: Any reason not to call this as "base::Hash(id)"?  If the lack of
> BASE_EXPORT is the problem, I bet it's fine to add it -- I see no reason why
the
> convenience wrappers would not be exported.

The reason was simply because I'd like it be super fast. :)
I've now changed to use base::Hash.

https://codereview.chromium.org/1237493003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:871:
static_cast<int>(base::SuperFastHash(id.c_str(), id.length()));
On 2015/07/17 01:43:58, Ilya Sherman (Away 7.18-8.03) wrote:
> nit: Rather than static_cast<int>, please use base::strict_cast, and cast to
> int32_t or base::HistogramBase::Sample.

There will be compiling errors as below. I want to cast uint32 to int32 here.

../../base/numerics/safe_conversions.h:70:3: error: static_assert failed "The
numeric conversion is out of range for this type. You should probably use one of
the following conversion mechanisms on the value you want to pass:\n-
base::checked_cast\n- base::saturated_cast\n- base::CheckedNumeric"
  static_assert((internal::StaticDstRangeRelationToSrcRange<Dst, Src>::value ==
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../chrome/browser/chromeos/input_method/input_method_manager_impl.cc:886:56:
note: in instantiation of function template specialization
'base::strict_cast<int, unsigned int>' requested here
  UMA_HISTOGRAM_SPARSE_SLOWLY("InputMethod.ID2", base::strict_cast<int32_t>(
...

https://codereview.chromium.org/1237493003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1237493003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13729: +    please see InputMethod.ID2.
On 2015/07/17 01:43:58, Ilya Sherman (Away 7.18-8.03) wrote:
> nit: Please mark this histogram as <obsolete>, and move the deprecation
message
> into the <obsolete> tag.

Done.

https://codereview.chromium.org/1237493003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:59355: +  <int value="1763643144"
label="xkb:lt::lit">keyboard_lithuanian</int>
On 2015/07/17 01:43:58, Ilya Sherman (Away 7.18-8.03) wrote:
> nit: Could you please change these labels to be more like "Lithuanian", rather
> than "keyboard_lithuanian"?  The latter is considerably less readable.  I
> realize it's a little tedious, but I bet a simple Python script could automate
> 90% of the lines, leaving just a few for tedious manual work.

Done.

[Ilya Sherman, 2015-07-17 04:40:12]

LGTM, thanks.

https://codereview.chromium.org/1237493003/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1237493003/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13731: +    This is deprecated since
M46, InputMethod.ID2 is used since them.
nit: "them" -> "then"

[Shu Chen, 2015-07-17 05:39:30]

https://codereview.chromium.org/1237493003/diff/80001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1237493003/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13731: +    This is deprecated since
M46, InputMethod.ID2 is used since them.
On 2015/07/17 04:40:12, Ilya Sherman (Away 7.18-8.03) wrote:
> nit: "them" -> "then"

Done.

[Shu Chen, 2015-07-17 05:39:40]

The CQ bit was checked by shuchen@chromium.org

[Shu Chen, 2015-07-17 05:39:40]

The patchset sent to the CQ was uploaded after l-g-t-m from nona@chromium.org
Link to the patchset: https://codereview.chromium.org/1237493003/#ps80002
(title: "nit.")

[commit-bot: I haz the power, 2015-07-17 05:39:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1237493003/80002

[commit-bot: I haz the power, 2015-07-17 06:45:39]

Committed patchset #6 (id:80002)

[commit-bot: I haz the power, 2015-07-17 06:46:49]

Patchset 6 (id:??) landed as
https://crrev.com/254e75d880369bf49bdf0e8ccc400ab71f6f2fee
Cr-Commit-Position: refs/heads/master@{#339225}