Commit 3cfa0dec authored by Charles Zhao's avatar Charles Zhao Committed by Commit Bot

In TabActivityWatcher, a label_id_ of a tab is updated by:

label_id_ = label_id_ ? label_id_ + 1 : NewInt64ForLabelIdOrQueryId();

If a foregrounded event is logged, then label_id_ is set to 0 and a new
one is assigned the next time by
    (++internal_id_for_logging) << kIdShiftBits;

If no foregrounded event is logged between two discarding events,
the label_id_ will increase by 1.

So if it increases 8192 times, it will collide with another label_id_
generated with (++internal_id_for_logging) << kIdShiftBits;

We have seen this occasionally in the logging, and the simplest way to
fix this is increase kIdShiftBits a bit (from 13 to 16).

Bug: 1049896

Change-Id: Ibbc56ee24ba502433283f53c38946b98a25ac37c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041837Reviewed-by: default avatarTony Yeoman <tby@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Reviewed-by: default avatarCharles . <charleszhao@chromium.org>
Commit-Queue: Charles . <charleszhao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740096}
parent 05ae8871
...@@ -53,13 +53,14 @@ static int32_t reactivation_index = 0; ...@@ -53,13 +53,14 @@ static int32_t reactivation_index = 0;
int64_t internal_id_for_logging = 0; int64_t internal_id_for_logging = 0;
// Returns an int64_t number as label_id or query_id. // Returns an int64_t number as label_id or query_id.
int64_t NewInt64ForLabelIdOrQueryId() { int64_t NewInt64ForLabelIdOrQueryId() {
// The id is shifted 13 bits so that the lower bits are reserved for counting // The id is shifted 16 bits so that the lower bits are reserved for counting
// multiple queries. // multiple queries.
// We choose 13 so that the lower bits for counting multiple queries and // We choose 16 so that the lower bits for counting multiple queries and
// higher bits for labeling queries are both unlikely to overflow. (lower bits // higher bits for labeling queries are both unlikely to overflow. (lower bits
// only overflows when we have more than 8192 queries without labeling events; // only overflows when we have more than 65536 queries without labeling
// higher bits only overflow when we have more than 100 billion discards. // events; higher bits only overflow when we have more than 100 billion
constexpr int kIdShiftBits = 13; // discards.
constexpr int kIdShiftBits = 16;
return (++internal_id_for_logging) << kIdShiftBits; return (++internal_id_for_logging) << kIdShiftBits;
} }
......
...@@ -56,7 +56,7 @@ const UkmMetricMap kBasicMetricValues({ ...@@ -56,7 +56,7 @@ const UkmMetricMap kBasicMetricValues({
// These parameters don't affect logging. // These parameters don't affect logging.
const bool kCheckNavigationSuccess = true; const bool kCheckNavigationSuccess = true;
const int64_t kIdShift = 1 << 13; const int64_t kIdShift = 1 << 16;
} // namespace } // namespace
......
...@@ -48,7 +48,7 @@ namespace { ...@@ -48,7 +48,7 @@ namespace {
const char* kTabMetricsEntryName = TabManager_TabMetrics::kEntryName; const char* kTabMetricsEntryName = TabManager_TabMetrics::kEntryName;
const int64_t kIdShift = 1 << 13; const int64_t kIdShift = 1 << 16;
// Test URLs need to be from different origins to test site engagement score. // Test URLs need to be from different origins to test site engagement score.
const std::vector<GURL>& TestUrls() { const std::vector<GURL>& TestUrls() {
......
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