Commit 21793cfe authored by Idries Hamadi's avatar Idries Hamadi Committed by Commit Bot

[aw] Fix edge case in Visibility metrics logging

When visibility metrics are written to histograms only the currently
tracked visibility bucket is logged. This can lead to some data loss.
This patch changes the behaviour of the record implemeantation to write
any outstanding data for all buckets. Also fixed a couple of spelling
errors in comments.

R=tobiasjs@chromium.org

Bug: 1139372
Change-Id: Idad1e18c9f581dab2579bce315b7b5f0750c736e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480565Reviewed-by: default avatarTobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Idries Hamadi <idries@google.com>
Auto-Submit: Idries Hamadi <idries@google.com>
Cr-Commit-Position: refs/heads/master@{#819398}
parent ec986c82
...@@ -291,19 +291,17 @@ void VisibilityMetricsLogger::RecordOpenWebDisplayMetrics() { ...@@ -291,19 +291,17 @@ void VisibilityMetricsLogger::RecordOpenWebDisplayMetrics() {
} }
void VisibilityMetricsLogger::RecordScreenPortionMetrics() { void VisibilityMetricsLogger::RecordScreenPortionMetrics() {
int tracked_portion = static_cast<int>(current_open_web_screen_portion_); for (int i = 0; i < static_cast<int>(WebViewOpenWebScreenPortion::kMaxValue);
i++) {
// TODO (idries@): record every bucket, not just int32_t elapsed_seconds =
// current_open_web_screen_portion_ open_web_screen_portion_tracked_duration_[i].InSeconds();
int32_t elapsed_seconds = if (elapsed_seconds == 0)
open_web_screen_portion_tracked_duration_[tracked_portion].InSeconds(); continue;
if (elapsed_seconds == 0)
return; open_web_screen_portion_tracked_duration_[i] -=
base::TimeDelta::FromSeconds(elapsed_seconds);
open_web_screen_portion_tracked_duration_[tracked_portion] -= GetOpenWebVisibileScreenPortionHistogram()->AddCount(i, elapsed_seconds);
base::TimeDelta::FromSeconds(elapsed_seconds); }
GetOpenWebVisibileScreenPortionHistogram()->AddCount(tracked_portion,
elapsed_seconds);
} }
} // namespace android_webview } // namespace android_webview
\ No newline at end of file
...@@ -113,7 +113,7 @@ class VisibilityMetricsLogger { ...@@ -113,7 +113,7 @@ class VisibilityMetricsLogger {
base::TimeDelta no_webview_tracked_duration_ = base::TimeDelta no_webview_tracked_duration_ =
base::TimeDelta::FromSeconds(0); base::TimeDelta::FromSeconds(0);
// Total duration that WebViews meet the tracking criteria (i.e. if // Total duration that WebViews meet the tracking criteria (i.e. if
// 2x WebViews meet the criteria for 1 second then increment by 2 sections) // 2x WebViews meet the criteria for 1 second then increment by 2 seconds)
base::TimeDelta per_webview_duration_ = base::TimeDelta::FromSeconds(0); base::TimeDelta per_webview_duration_ = base::TimeDelta::FromSeconds(0);
// Total duration that WebViews exist but do not meet the tracking criteria // Total duration that WebViews exist but do not meet the tracking criteria
base::TimeDelta per_webview_untracked_duration_ = base::TimeDelta per_webview_untracked_duration_ =
......
...@@ -86,7 +86,7 @@ public final class ProductionSupportedFlagList { ...@@ -86,7 +86,7 @@ public final class ProductionSupportedFlagList {
+ "stricter same-origin feature is enabled."), + "stricter same-origin feature is enabled."),
Flag.baseFeature(AwFeatures.WEBVIEW_MEASURE_SCREEN_COVERAGE, Flag.baseFeature(AwFeatures.WEBVIEW_MEASURE_SCREEN_COVERAGE,
"Measure the number of pixels occupied by one or more WebViews as a proportion " "Measure the number of pixels occupied by one or more WebViews as a proportion "
+ "of the total screen size. Depending on the number of WebVieaws and " + "of the total screen size. Depending on the number of WebViews and "
+ "the size of the screen this might be expensive so hidden behind a " + "the size of the screen this might be expensive so hidden behind a "
+ "feature flag until the true runtime cost can be measured."), + "feature flag until the true runtime cost can be measured."),
Flag.baseFeature(BlinkFeatures.WEB_COMPONENTS_V0, Flag.baseFeature(BlinkFeatures.WEB_COMPONENTS_V0,
......
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