PrivacyIndicators: Add metric to detect flickering indicator view

This metric records repeated shows of privacy indicator view per 100ms,
detecting a bad UX causing by the privacy indicator view.

Fixed: b:255825208
Change-Id: I9ddcf6dd4bc8eb62eeca253e0ffa320aecfe219c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4108688
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Andre Le <leandre@chromium.org>
Reviewed-by: Gavin Williams <gavinwill@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084605}

ash/system/privacy/privacy_indicators_tray_item_view.cc

@@ -20,6 +20,7 @@
 #include "base/containers/flat_set.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/color/color_id.h"
 #include "ui/color/color_provider.h"
@@ -50,6 +51,8 @@ const int kPrivacyIndicatorsViewExpandedLongerSideSize = 50;
 const int kPrivacyIndicatorsViewExpandedWithScreenShareSize = 68;
 const int kPrivacyIndicatorsViewSize = 8;
 
+constexpr auto kRepeatedShowTimerInterval = base::Milliseconds(100);
+
 constexpr auto kDwellInExpandDuration = base::Milliseconds(1000);
 constexpr auto kShorterSizeShrinkAnimationDelay =
     kDwellInExpandDuration + base::Milliseconds(133);
@@ -119,6 +122,17 @@ void FadeInView(views::View* view,
       .SetOpacity(view, 1.0f);
 }
 
+// Returns true if the widget is in the primary display.
+bool IsInPrimaryDisplay(views::Widget* widget) {
+  if (!widget) {
+    return false;
+  }
+
+  auto* screen = display::Screen::GetScreen();
+  return screen->GetDisplayNearestWindow(widget->GetNativeWindow()) ==
+         screen->GetPrimaryDisplay();
+}
+
 }  // namespace
 
 PrivacyIndicatorsTrayItemView::PrivacyIndicatorsTrayItemView(Shelf* shelf)
@@ -134,7 +148,12 @@ PrivacyIndicatorsTrayItemView::PrivacyIndicatorsTrayItemView(Shelf* shelf)
       shorter_side_shrink_animation_(std::make_unique<gfx::LinearAnimation>(
           kSizeChangeAnimationDuration,
           gfx::LinearAnimation::kDefaultFrameRate,
-          this)) {
+          this)),
+      repeated_shows_timer_(
+          FROM_HERE,
+          kRepeatedShowTimerInterval,
+          this,
+          &PrivacyIndicatorsTrayItemView::RecordRepeatedShows) {
   SetVisible(false);
 
   auto container_view = std::make_unique<views::View>();
@@ -413,13 +432,8 @@ void PrivacyIndicatorsTrayItemView::OnSessionStateChanged(
   if (count_visible_per_session_ == 0)
     return;
 
-  // `GetWidget()` might be null in unit tests.
-  if (!GetWidget())
-    return;
-  auto* screen = display::Screen::GetScreen();
   // Only record this metric on primary screen.
-  if (screen->GetDisplayNearestWindow(GetWidget()->GetNativeWindow()) !=
-      screen->GetPrimaryDisplay()) {
+  if (!IsInPrimaryDisplay(GetWidget())) {
     return;
   }
 
@@ -508,10 +522,24 @@ void PrivacyIndicatorsTrayItemView::UpdateAccessStatus(
 void PrivacyIndicatorsTrayItemView::UpdateVisibility() {
   // We only hide the view when all the sets are empty.
   bool visible = IsCameraUsed() || IsMicrophoneUsed() || is_screen_sharing_;
+
+  if (GetVisible() == visible) {
+    return;
+  }
+
   SetVisible(visible);
 
-  if (visible)
-    count_visible_per_session_++;
+  if (!visible) {
+    return;
+  }
+
+  ++count_visible_per_session_;
+
+  // Keeps increment the count to track the number of times the view flickers.
+  // When the delay of `kRepeatedShowTimerInterval` has reached, record that
+  // count.
+  ++count_repeated_shows_;
+  repeated_shows_timer_.Reset();
 }
 
 void PrivacyIndicatorsTrayItemView::EndAllAnimations() {
@@ -551,4 +579,17 @@ void PrivacyIndicatorsTrayItemView::RecordPrivacyIndicatorsType() {
   }
 }
 
+void PrivacyIndicatorsTrayItemView::RecordRepeatedShows() {
+  // We are only interested in more than 1 repeated shows per 100ms. Also only
+  // records in primary display.
+  if (count_repeated_shows_ <= 1 || !IsInPrimaryDisplay(GetWidget())) {
+    count_repeated_shows_ = 0;
+    return;
+  }
+
+  base::UmaHistogramCounts100("Ash.PrivacyIndicators.NumberOfRepeatedShows",
+                              count_repeated_shows_);
+  count_repeated_shows_ = 0;
+}
+
 }  // namespace ash

ash/system/privacy/privacy_indicators_tray_item_view.h

@@ -9,6 +9,7 @@
 #include "ash/public/cpp/session/session_observer.h"
 #include "ash/system/tray/tray_item_view.h"
 #include "base/containers/flat_set.h"
+#include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
 #include "ui/compositor/throughput_tracker.h"
 
@@ -139,6 +140,9 @@ class ASH_EXPORT PrivacyIndicatorsTrayItemView : public TrayItemView,
   // Record the type of privacy indicators that are showing.
   void RecordPrivacyIndicatorsType();
 
+  // Record repeated shows metric when the timer is stop.
+  void RecordRepeatedShows();
+
   views::BoxLayout* layout_manager_ = nullptr;
 
   // Owned by the views hierarchy.
@@ -169,6 +173,10 @@ class ASH_EXPORT PrivacyIndicatorsTrayItemView : public TrayItemView,
   // Used to record metrics of the number of shows per session.
   int count_visible_per_session_ = 0;
 
+  // Used to record metrics of repeated shows per 100 ms.
+  int count_repeated_shows_ = 0;
+  base::DelayTimer repeated_shows_timer_;
+
   // Measure animation smoothness metrics for all the animations.
   absl::optional<ui::ThroughputTracker> throughput_tracker_;
 };

ash/system/privacy/privacy_indicators_tray_item_view_unittest.cc

@@ -16,6 +16,8 @@
 #include "ash/test/ash_test_base.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/task_environment.h"
+#include "base/time/time.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/animation/linear_animation.h"
 #include "ui/views/controls/image_view.h"
@@ -35,6 +37,8 @@ constexpr char kCountAppsAccessCameraHistogramName[] =
     "Ash.PrivacyIndicators.NumberOfAppsAccessingCamera";
 constexpr char kCountAppsAccessMicrophoneHistogramName[] =
     "Ash.PrivacyIndicators.NumberOfAppsAccessingMicrophone";
+constexpr char kRepeatedShowsHistogramName[] =
+    "Ash.PrivacyIndicators.NumberOfRepeatedShows";
 
 // Get the expected size in expand animation, given the animation value.
 int GetExpectedSizeInExpandAnimation(double progress) {
@@ -74,7 +78,8 @@ namespace ash {
 
 class PrivacyIndicatorsTrayItemViewTest : public AshTestBase {
  public:
-  PrivacyIndicatorsTrayItemViewTest() = default;
+  PrivacyIndicatorsTrayItemViewTest()
+      : AshTestBase(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
   PrivacyIndicatorsTrayItemViewTest(const PrivacyIndicatorsTrayItemViewTest&) =
       delete;
   PrivacyIndicatorsTrayItemViewTest& operator=(
@@ -635,4 +640,68 @@ TEST_F(PrivacyIndicatorsTrayItemViewTest, RecordAppAccessSimultaneously) {
   histograms.ExpectBucketCount(kCountAppsAccessMicrophoneHistogramName, 2, 1);
 }
 
+TEST_F(PrivacyIndicatorsTrayItemViewTest, RecordRepeatedShows) {
+  // Set up 2 displays. Note that only one instance should be recorded for the
+  // primary display when session changes.
+  UpdateDisplay("100x200,300x400");
+
+  base::HistogramTester histograms;
+
+  int expected_sample = 6;
+
+  // Makes the view flicker (show then hide) for `expected_sample` of times.
+  // Metric should be recorded for this repeated shows.
+  for (auto i = 0; i < expected_sample; i++) {
+    for (auto* controller : Shell::Get()->GetAllRootWindowControllers()) {
+      auto* indicator_view = controller->GetStatusAreaWidget()
+                                 ->unified_system_tray()
+                                 ->privacy_indicators_view();
+      indicator_view->Update(/*app_id=*/"app_id", /*is_camera_used=*/true,
+                             /*is_microphone_used=*/true);
+      indicator_view->Update(/*app_id=*/"app_id", /*is_camera_used=*/false,
+                             /*is_microphone_used=*/false);
+    }
+    task_environment()->FastForwardBy(base::Milliseconds(80));
+  }
+
+  task_environment()->FastForwardBy(base::Milliseconds(100));
+  histograms.ExpectBucketCount(kRepeatedShowsHistogramName, expected_sample, 1);
+
+  // Makes one more flickering after 100ms. This flicker should not count
+  // towards the previous ones.
+  for (auto* controller : Shell::Get()->GetAllRootWindowControllers()) {
+    auto* indicator_view = controller->GetStatusAreaWidget()
+                               ->unified_system_tray()
+                               ->privacy_indicators_view();
+    indicator_view->Update(/*app_id=*/"app_id", /*is_camera_used=*/true,
+                           /*is_microphone_used=*/true);
+    indicator_view->Update(/*app_id=*/"app_id", /*is_camera_used=*/false,
+                           /*is_microphone_used=*/false);
+  }
+  task_environment()->FastForwardBy(base::Milliseconds(100));
+  histograms.ExpectBucketCount(kRepeatedShowsHistogramName, expected_sample + 1,
+                               0);
+
+  // Make sure it works again.
+  expected_sample = 8;
+
+  // Makes the view flicker (show then hide) for `expected_sample` of times.
+  // Metric should be recorded for this repeated shows.
+  for (auto i = 0; i < expected_sample; i++) {
+    for (auto* controller : Shell::Get()->GetAllRootWindowControllers()) {
+      auto* indicator_view = controller->GetStatusAreaWidget()
+                                 ->unified_system_tray()
+                                 ->privacy_indicators_view();
+      indicator_view->Update(/*app_id=*/"app_id", /*is_camera_used=*/true,
+                             /*is_microphone_used=*/true);
+      indicator_view->Update(/*app_id=*/"app_id", /*is_camera_used=*/false,
+                             /*is_microphone_used=*/false);
+    }
+    task_environment()->FastForwardBy(base::Milliseconds(80));
+  }
+
+  task_environment()->FastForwardBy(base::Milliseconds(100));
+  histograms.ExpectBucketCount(kRepeatedShowsHistogramName, expected_sample, 1);
+}
+
 }  // namespace ash

tools/metrics/histograms/metadata/ash/histograms.xml

@@ -3909,6 +3909,17 @@ chromium-metrics-reviews@google.com.
   </summary>
 </histogram>
 
+<histogram name="Ash.PrivacyIndicators.NumberOfRepeatedShows" units="shows"
+    expires_after="2023-09-19">
+  <owner>leandre@chromium.org</owner>
+  <owner>cros-status-area-eng@google.com</owner>
+  <summary>
+    Record the number of times that the privacy indicator repeatedly shows per
+    100ms. Emitted when the sequence of consecutive shows in the privacy
+    indicator has finished.
+  </summary>
+</histogram>
+
 <histogram name="Ash.PrivacyIndicators.NumberOfShowsPerSession" units="shows"
     expires_after="2023-09-19">
   <owner>leandre@chromium.org</owner>