[go: up one dir, main page]

[iOS] Add UMA metrics for feed issues

Logs a UMA histogram each time the view hierarchy has to be repaired,
indicating what went wrong. Also adds a user action for when users
engage with the feed.

Bug: 1268164
Change-Id: I462497c02a015c836b5e1dac275689b70a46c56d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3282450
Auto-Submit: Adam Trudeau-Arcaro <adamta@google.com>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/branch-heads/4664@{#1069}
Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}
diff --git a/ios/chrome/browser/ui/content_suggestions/discover_feed_metrics_recorder.h b/ios/chrome/browser/ui/content_suggestions/discover_feed_metrics_recorder.h
index c57ab57..6b8f623 100644
--- a/ios/chrome/browser/ui/content_suggestions/discover_feed_metrics_recorder.h
+++ b/ios/chrome/browser/ui/content_suggestions/discover_feed_metrics_recorder.h
@@ -7,6 +7,20 @@
 
 #import <UIKit/UIKit.h>
 
+// DO NOT CHANGE. Values are from enums.xml representing what could be broken in
+// the NTP view hierarchy. These values are persisted to logs. Entries should
+// not be renumbered and numeric values should never be reused.
+enum class BrokenNTPHierarchyRelationship {
+  kContentSuggestionsParent = 0,
+  kELMCollectionParent = 1,
+  kDiscoverFeedParent = 2,
+  kDiscoverFeedWrapperParent = 3,
+  kContentSuggestionsReset = 4,
+
+  // Change this to match max value.
+  kMaxValue = 4,
+};
+
 // Records different metrics for the NTP's Discover feed.
 // TODO(crbug.com/1200303): Move this file to */ui/ntp.
 @interface DiscoverFeedMetricsRecorder : NSObject
@@ -124,6 +138,11 @@
 // Records the native pull-down menu visibility change.
 - (void)recordNativePulldownMenuVisibilityChanged:(BOOL)shown;
 
+// Records the broken view hierarchy before repairing it.
+// TODO(crbug.com/1262536): Remove this when issue is fixed.
+- (void)recordBrokenNTPHierarchy:
+    (BrokenNTPHierarchyRelationship)brokenRelationship;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_DISCOVER_FEED_METRICS_RECORDER_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/discover_feed_metrics_recorder.mm b/ios/chrome/browser/ui/content_suggestions/discover_feed_metrics_recorder.mm
index 0c2d985..388b351 100644
--- a/ios/chrome/browser/ui/content_suggestions/discover_feed_metrics_recorder.mm
+++ b/ios/chrome/browser/ui/content_suggestions/discover_feed_metrics_recorder.mm
@@ -120,6 +120,9 @@
 const char kDiscoverFeedUserActionInfiniteFeedTriggered[] =
     "ContentSuggestions.Feed.InfiniteFeedTriggered";
 
+// User action name for engaging with feed.
+const char kDiscoverFeedUserActionEngaged[] = "ContentSuggestions.Feed.Engaged";
+
 // Histogram name for the feed engagement types.
 const char kDiscoverFeedEngagementTypeHistogram[] =
     "ContentSuggestions.Feed.EngagementType";
@@ -171,6 +174,11 @@
 const char kDiscoverFeedActivityLoggingEnabled[] =
     "ContentSuggestions.Feed.ActivityLoggingEnabled";
 
+// Histogram name for broken NTP view hierarchy logs.
+// TODO(crbug.com/1262536): Remove this when issue is fixed.
+const char kDiscoverFeedBrokenNTPHierarchy[] =
+    "ContentSuggestions.Feed.BrokenNTPHierarchy";
+
 // Minimum scrolling amount to record a FeedEngagementType::kFeedEngaged due to
 // scrolling.
 const int kMinScrollThreshold = 160;
@@ -443,6 +451,10 @@
                             loggingEnabled);
 }
 
+- (void)recordBrokenNTPHierarchy:(BrokenNTPHierarchyRelationship)relationship {
+  base::UmaHistogramEnumeration(kDiscoverFeedBrokenNTPHierarchy, relationship);
+}
+
 #pragma mark - Private
 
 // Records histogram metrics for Discover feed user actions.
@@ -478,6 +490,7 @@
   if (!self.engagedReported &&
       (scrollDistance > kMinScrollThreshold || interacted)) {
     [self recordEngagementTypeHistogram:FeedEngagementType::kFeedEngaged];
+    base::RecordAction(base::UserMetricsAction(kDiscoverFeedUserActionEngaged));
     self.engagedReported = YES;
   }
 }
diff --git a/ios/chrome/browser/ui/ntp/new_tab_page_view_controller.mm b/ios/chrome/browser/ui/ntp/new_tab_page_view_controller.mm
index eddf0832..996d956 100644
--- a/ios/chrome/browser/ui/ntp/new_tab_page_view_controller.mm
+++ b/ios/chrome/browser/ui/ntp/new_tab_page_view_controller.mm
@@ -129,6 +129,9 @@
     [self.contentSuggestionsViewController willMoveToParentViewController:nil];
     [self.contentSuggestionsViewController.view removeFromSuperview];
     [self.contentSuggestionsViewController removeFromParentViewController];
+    [self.discoverFeedMetricsRecorder
+        recordBrokenNTPHierarchy:BrokenNTPHierarchyRelationship::
+                                     kContentSuggestionsReset];
   }
 
   [self.contentSuggestionsViewController
@@ -752,23 +755,36 @@
     [self.contentSuggestionsViewController
         didMoveToParentViewController:self.discoverFeedWrapperViewController
                                           .discoverFeed];
+
+    [self.discoverFeedMetricsRecorder
+        recordBrokenNTPHierarchy:BrokenNTPHierarchyRelationship::
+                                     kContentSuggestionsParent];
   }
   [self ensureView:self.collectionView
-       isSubviewOf:self.discoverFeedWrapperViewController.discoverFeed.view];
+             isSubviewOf:self.discoverFeedWrapperViewController.discoverFeed
+                             .view
+      withRelationshipID:BrokenNTPHierarchyRelationship::kELMCollectionParent];
   [self ensureView:self.discoverFeedWrapperViewController.discoverFeed.view
-       isSubviewOf:self.discoverFeedWrapperViewController.view];
+             isSubviewOf:self.discoverFeedWrapperViewController.view
+      withRelationshipID:BrokenNTPHierarchyRelationship::kDiscoverFeedParent];
   [self ensureView:self.discoverFeedWrapperViewController.view
-       isSubviewOf:self.view];
+             isSubviewOf:self.view
+      withRelationshipID:BrokenNTPHierarchyRelationship::
+                             kDiscoverFeedWrapperParent];
 }
 
 // Ensures that |subView| is a descendent of |parentView|. If not, logs a DCHECK
-// and adds the subview.
+// and adds the subview. Includes |relationshipID| for metrics recorder to log
+// which part of the view hierarchy was broken.
 // TODO(crbug.com/1262536): Remove this once bug is fixed.
-- (void)ensureView:(UIView*)subView isSubviewOf:(UIView*)parentView {
+- (void)ensureView:(UIView*)subView
+           isSubviewOf:(UIView*)parentView
+    withRelationshipID:(BrokenNTPHierarchyRelationship)relationship {
   if (![parentView.subviews containsObject:subView]) {
     DCHECK([parentView.subviews containsObject:subView]);
     [subView removeFromSuperview];
     [parentView addSubview:subView];
+    [self.discoverFeedMetricsRecorder recordBrokenNTPHierarchy:relationship];
   }
 }
 
diff --git a/tools/metrics/actions/actions.xml b/tools/metrics/actions/actions.xml
index fc8abfd..314969f 100644
--- a/tools/metrics/actions/actions.xml
+++ b/tools/metrics/actions/actions.xml
@@ -5457,6 +5457,16 @@
   </description>
 </action>
 
+<action name="ContentSuggestions.Feed.Engaged">
+  <owner>adamta@google.org</owner>
+  <owner>sczs@chromium.org</owner>
+  <owner>feed@chromium.org</owner>
+  <description>
+    The user has engaged with the feed or scrolled at least 1 inch into it
+    within a 5-minute session.
+  </description>
+</action>
+
 <action name="ContentSuggestions.Feed.HeaderAction.ManageActivity">
   <owner>adamta@google.org</owner>
   <owner>sczs@chromium.org</owner>
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 4cd56671..f9734f74 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -61032,6 +61032,15 @@
       label="Background customization unavailable due to search provider"/>
 </enum>
 
+<enum name="NTPBrokenViewHierarchyRelationship">
+  <int value="0" label="ELM collection containing Content Suggestions fixed"/>
+  <int value="1" label="Discover feed containing ELM collection fixed"/>
+  <int value="2" label="Discover feed wrapper containing Discover feed fixed"/>
+  <int value="3" label="NTP containing Discover feed wrapper fixed"/>
+  <int value="4"
+      label="Content suggestions view controller not cleaned up on restart"/>
+</enum>
+
 <enum name="NTPCustomizeAction">
   <int value="0" label="'Chrome backgrounds' menu item clicked."/>
   <int value="1" label="'Upload an image' menu item clicked."/>
diff --git a/tools/metrics/histograms/metadata/content/histograms.xml b/tools/metrics/histograms/metadata/content/histograms.xml
index 48b044e..d839bab4 100644
--- a/tools/metrics/histograms/metadata/content/histograms.xml
+++ b/tools/metrics/histograms/metadata/content/histograms.xml
@@ -712,6 +712,18 @@
   </summary>
 </histogram>
 
+<histogram name="ContentSuggestions.Feed.BrokenNTPHierarchy"
+    enum="NTPBrokenViewHierarchyRelationship" expires_after="M99">
+  <owner>adamta@google.com</owner>
+  <owner>sczs@chromium.org</owner>
+  <owner>feed@chromium.org</owner>
+  <summary>
+    When the NTP view hierarchy is fixed upon NTP creation, this hisotgram logs
+    which part was broken. Temporary log used to gather info for
+    crbug.com/1262536.
+  </summary>
+</histogram>
+
 <histogram name="ContentSuggestions.Feed.CommitMutationCount"
     units="operations" expires_after="2020-10-01">
   <owner>carlosk@chromium.org</owner>