Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2711)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

Issue 1727823002: [Herb] Add metrics to track button usage (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebasing Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
index f92ddbefe0b550d9fd6467a9a49943cfec1d4ab2..64db99734f7aaed92fb58e47c275bb137bdd9c1b 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
@@ -14,6 +14,7 @@ import android.os.Build;
import android.os.Bundle;
import android.os.SystemClock;
import android.provider.Browser;
+import android.support.annotation.IntDef;
import android.support.annotation.Nullable;
import android.text.TextUtils;
import android.view.KeyEvent;
@@ -60,6 +61,7 @@ import org.chromium.chrome.browser.firstrun.FirstRunActivity;
import org.chromium.chrome.browser.firstrun.FirstRunFlowSequencer;
import org.chromium.chrome.browser.firstrun.FirstRunSignInProcessor;
import org.chromium.chrome.browser.firstrun.FirstRunStatus;
+import org.chromium.chrome.browser.metrics.ActivityStopMetrics;
import org.chromium.chrome.browser.metrics.LaunchMetrics;
import org.chromium.chrome.browser.metrics.StartupMetrics;
import org.chromium.chrome.browser.metrics.UmaUtils;
@@ -99,6 +101,9 @@ import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.base.PageTransition;
import org.chromium.ui.widget.Toast;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
/**
* This is the main activity for ChromeMobile when not running in document mode. All the tabs
* are accessible via a chrome specific tab switching UI.
@@ -108,6 +113,21 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
private static final int FIRST_RUN_EXPERIENCE_RESULT = 101;
private static final int CCT_RESULT = 102;
+ @Retention(RetentionPolicy.SOURCE)
+ @IntDef({
+ BACK_PRESSED_NOTHING_HAPPENED,
+ BACK_PRESSED_HELP_URL_CLOSED,
+ BACK_PRESSED_MINIMIZED_NO_TAB_CLOSED,
+ BACK_PRESSED_MINIMIZED_TAB_CLOSED,
+ BACK_PRESSED_TAB_CLOSED
+ })
+ private @interface BackPressedResult {}
+ private static final int BACK_PRESSED_NOTHING_HAPPENED = 0;
+ private static final int BACK_PRESSED_HELP_URL_CLOSED = 1;
+ private static final int BACK_PRESSED_MINIMIZED_NO_TAB_CLOSED = 2;
+ private static final int BACK_PRESSED_MINIMIZED_TAB_CLOSED = 3;
+ private static final int BACK_PRESSED_TAB_CLOSED = 4;
+
private static final String TAG = "ChromeTabbedActivity";
private static final String HELP_URL_PREFIX = "https://support.google.com/chrome/";
@@ -139,6 +159,9 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
private static final String ACTION_CLOSE_TABS =
"com.google.android.apps.chrome.ACTION_CLOSE_TABS";
+ private final ActivityStopMetrics mActivityStopMetrics =
+ new ActivityStopMetrics("Android.Activity.ChromeTabbedActivity.StopReason");
+
private FindToolbarManager mFindToolbarManager;
private UndoBarPopupController mUndoBarPopupController;
@@ -322,6 +345,7 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
// This may happen when onStop get called very early in UI test.
}
StartupMetrics.getInstance().recordHistogram(true);
+ mActivityStopMetrics.onStopWithNative(this);
}
@Override
@@ -475,7 +499,11 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
getToolbarManager().getToolbar().setReturnButtonListener(new OnClickListener() {
@Override
public void onClick(View v) {
- if (getActivityTab() != null) handleBackPressedWithoutBackStack(true);
+ if (getActivityTab() == null) return;
+ mActivityStopMetrics.setStopReason(
+ ActivityStopMetrics.STOP_REASON_RETURN_BUTTON);
+ RecordUserAction.record("Herb.ReturnButtonClicked");
+ handleBackPressedWithoutBackStack(true);
}
});
@@ -613,7 +641,10 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
data.getDataString()), TabLaunchType.FROM_CHROME_UI, null);
} else if ((resultCode == CustomTabActivity.RESULT_BACK_PRESSED
|| resultCode == CustomTabActivity.RESULT_CLOSED) && data != null) {
+ // Herb UMA should have already been recorded by the CustomTabActivity.
Log.d(TAG, "Herb: Sending app to the background");
+ mActivityStopMetrics.setStopReason(
+ ActivityStopMetrics.STOP_REASON_CUSTOM_TAB_STOPPED);
sendToBackground(null);
}
return true;
@@ -770,6 +801,8 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
&& ChromeLauncherActivity.canBeHijackedByHerb(intent)
&& TextUtils.equals(ChromeSwitches.HERB_FLAVOR_DILL, herbFlavor)) {
Log.d(TAG, "Sending to CustomTabActivity");
+ mActivityStopMetrics.setStopReason(
+ ActivityStopMetrics.STOP_REASON_CUSTOM_TAB_STARTED);
Intent newIntent = ChromeLauncherActivity.createCustomTabActivityIntent(
ChromeTabbedActivity.this, intent, false);
@@ -1043,13 +1076,8 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
final Tab currentTab = getActivityTab();
if (currentTab == null) {
- if (getToolbarManager().back()) {
- RecordUserAction.record("SystemBackForNavigation");
Ian Wen 2016/02/24 23:25:09 Q: Is this change intentional? Do we decide not to
gone 2016/02/25 01:30:21 This actually shouldn't ever happen -- if the tab
- RecordUserAction.record("MobileTabClobbered");
- } else {
- moveTaskToBack(true);
- }
- RecordUserAction.record("SystemBack");
+ moveTaskToBack(true);
+ RecordUserAction.record("SystemBack.TabIsNull");
Log.i(TAG, "handleBackPressed() - currentTab is null");
return true;
}
@@ -1067,17 +1095,35 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
return true;
}
- if (!getToolbarManager().back()) {
- Log.i(TAG, "handleBackPressed() - no back stack");
- if (handleBackPressedWithoutBackStack(false)) return true;
- } else {
+ if (getToolbarManager().back()) {
Log.i(TAG, "handleBackPressed() - moving back in navigation");
RecordUserAction.record("SystemBackForNavigation");
RecordUserAction.record("MobileTabClobbered");
+ return true;
}
- RecordUserAction.record("SystemBack");
Ian Wen 2016/02/24 23:25:09 After this change SystemBack will be only recorded
gone 2016/02/25 01:30:21 Redoing this bit.
- return true;
+ Log.i(TAG, "handleBackPressed() - no back stack");
+ int result = handleBackPressedWithoutBackStack(false);
+ if (result == BACK_PRESSED_HELP_URL_CLOSED) {
+ Log.i(TAG, "handleBackPressed() - help url");
+ return true;
+ } else if (result == BACK_PRESSED_MINIMIZED_TAB_CLOSED) {
+ Log.i(TAG, "handleBackPressed() - minimized, tab closed");
+ RecordUserAction.record("SystemBack.MinimizedAppTabClosed");
+ mActivityStopMetrics.setStopReason(ActivityStopMetrics.STOP_REASON_BACK_BUTTON);
+ return true;
+ } else if (result == BACK_PRESSED_MINIMIZED_NO_TAB_CLOSED) {
+ Log.i(TAG, "handleBackPressed() - minimized, tab kept open");
+ RecordUserAction.record("SystemBack.MinimizedAppNoTabClosed");
+ mActivityStopMetrics.setStopReason(ActivityStopMetrics.STOP_REASON_BACK_BUTTON);
+ return true;
+ } else if (result == BACK_PRESSED_TAB_CLOSED) {
+ Log.i(TAG, "handleBackPressed() - tab closed, app still in foreground");
+ RecordUserAction.record("SystemBack.TabClosed");
+ return true;
+ }
+
+ return false;
}
/**
@@ -1087,9 +1133,11 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
*
* @param alwaysAllowTabClosure Setting this to true always allows a tab opened by an external
* app to be closed.
- * @return Whether the tab closed was opened for a help page.
+ * @return See {@link BackPressedResult}.
*/
- private boolean handleBackPressedWithoutBackStack(boolean alwaysAllowTabClosure) {
+ @BackPressedResult
+ private int handleBackPressedWithoutBackStack(
+ boolean alwaysAllowTabClosure) {
Ian Wen 2016/02/24 23:25:09 Can we move #1140 to the previous line?
gone 2016/02/25 01:30:21 This artificial split isn't necessary anymore
final Tab currentTab = getActivityTab();
final TabLaunchType type = currentTab.getLaunchType();
final int parentId = currentTab.getParentId();
@@ -1100,8 +1148,7 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
// actual redirected URL is a different system language based help url.
if (type == TabLaunchType.FROM_CHROME_UI && helpUrl) {
getCurrentTabModel().closeTab(currentTab);
- Log.i(TAG, "handleBackPressedWithoutBackStack() - help url");
- return true;
+ return BACK_PRESSED_HELP_URL_CLOSED;
}
// Herb: Current spec says that tabs are closed only when there is NO "X" visible, i.e. when
@@ -1134,11 +1181,19 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
final boolean minimizeApp = !shouldCloseTab || currentTab.isCreatedForExternalApp();
if (minimizeApp) {
- sendToBackground(shouldCloseTab ? currentTab : null);
+ if (shouldCloseTab) {
+ sendToBackground(currentTab);
+ return BACK_PRESSED_MINIMIZED_TAB_CLOSED;
+ } else {
+ sendToBackground(null);
+ return BACK_PRESSED_MINIMIZED_NO_TAB_CLOSED;
+ }
} else if (shouldCloseTab) {
getCurrentTabModel().closeTab(currentTab, true, false, false);
+ return BACK_PRESSED_TAB_CLOSED;
}
- return false;
+
+ return BACK_PRESSED_NOTHING_HAPPENED;
}
/**

Powered by Google App Engine
This is Rietveld 408576698