[Home] Record some user actions for the Chrome Home BottomSheet

chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetMetrics.java

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.widget.bottomsheet;
+
+import org.chromium.base.metrics.RecordUserAction;
+import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.BottomSheetContent;
+
+/**
+ * Records user action and histograms related to the {@link BottomSheet}.
+ */
+public class BottomSheetMetrics extends EmptyBottomSheetObserver {
+    private boolean mIsSheetOpen;
+    private BottomSheetContent mLastContent;
+
+    /**
+     * @param sheet The {@link BottomSheet} to record metrics for.
+     */
+    public BottomSheetMetrics(BottomSheet sheet) {
+        sheet.addObserver(this);
+    }
+
+    @Override
+    public void onSheetOpened() {
+        if (mIsSheetOpen) return;
+
+        mIsSheetOpen = true;
+        RecordUserAction.record("Android.ChromeHome.Opened");
+    }
+
+    @Override
+    public void onSheetClosed() {
+        if (!mIsSheetOpen) return;

[mdjones, 2017/03/17 18:17:26]

Are the onSheetOpened and onSheetClosed events not 1:1?

[Theresa, 2017/03/17 20:26:30]

onSheetClosed gets called when the activity is first initialized so this check
is needed to avoid logging an event then. I figured it was good to be
symmetrical and check in both observer methods. We could also update BottomSheet
to make sure onSheetClosed() isn't called if the sheet hasn't been opened yet.

[mdjones, 2017/03/17 21:53:32]

I think I'd rather have this check in the bottom sheet. It should be the
observed object's responsibility to trigger reliable events.

[Theresa, 2017/03/17 22:55:25]

Done.

+
+        mIsSheetOpen = false;
+        RecordUserAction.record("Android.ChromeHome.Closed");
+    }
+
+    @Override
+    public void onSheetStateChanged(int newState) {
+        if (newState == BottomSheet.SHEET_STATE_HALF) {
+            RecordUserAction.record("Android.ChromeHome.HalfState");
+        } else if (newState == BottomSheet.SHEET_STATE_FULL) {
+            RecordUserAction.record("Android.ChromeHome.FullState");
+        }
+    }
+
+    @Override
+    public void onSheetContentChanged(BottomSheetContent newContent) {
+        if (mLastContent == null || !mIsSheetOpen) {
+            mLastContent = newContent;
+            return;
+        }
+
+        mLastContent = newContent;
+        RecordUserAction.record("Android.ChromeHome.Show" + mLastContent.getMetricsName());

[Theresa, 2017/03/17 16:07:12]

The upside to this approach is that it's simple. The downside to this approach
is that our presubmit scripts can't check for missing metrics due to the string
concatenation. Another approach could be:

if (newContent instanceof SuggestionsBottomSheetContent) {
   RecordUserAction.record("Android.ChromeHome.ShowSuggestions");
} else if (...) {
...
}


Or using an emun value or type returned by the BottomSheetContent to determine
which string to print.

[mdjones, 2017/03/17 18:17:26]

Though I like the current approach, I'm inclined to make this easier for other
developers. This would also be more difficult for code search. I'm not opposed
to having verbose metrics code in this class as it's already dedicated to
metrics.

[Theresa, 2017/03/17 20:26:30]

Done.

+    }
+
+}