Commit f9f7e45f authored by Gang Wu's avatar Gang Wu Committed by Commit Bot

[Feed] Add spinner metrics to Zine

Adding 2 metrics, which existing in Feed, but not in Zine
ontentSuggestions.Feed.FetchPendingSpinner.Shown
ContentSuggestions.Feed.FetchPendingSpinner.VisibleDurationWithoutCompleting

Bug: 943282
Change-Id: Id72d6fe55a34a4a294f9ea2bb452ce007e376286
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1576018
Commit-Queue: Gang Wu <gangwu@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652755}
parent b22273da
......@@ -31,18 +31,19 @@ import java.util.Locale;
* show a progress indicator over the same space. See {@link State}.
*/
public class ActionItem extends OptionalLeaf {
@IntDef({State.HIDDEN, State.BUTTON, State.LOADING})
@IntDef({State.HIDDEN, State.BUTTON, State.INITIAL_LOADING, State.MORE_BUTTON_LOADING})
@Retention(RetentionPolicy.SOURCE)
public @interface State {
int HIDDEN = 0;
int BUTTON = 1;
int LOADING = 2;
int INITIAL_LOADING = 2;
int MORE_BUTTON_LOADING = 3;
}
private final SuggestionsCategoryInfo mCategoryInfo;
private final SuggestionsSection mParentSection;
private final SuggestionsRanker mSuggestionsRanker;
private final SuggestionsMetrics.DurationTracker mSpinnerDurationTracker =
private final SuggestionsMetrics.SpinnerDurationTracker mSpinnerDurationTracker =
SuggestionsMetrics.getSpinnerVisibilityReporter();
private boolean mImpressionTracked;
......@@ -72,7 +73,8 @@ public class ActionItem extends OptionalLeaf {
switch (mState) {
case State.BUTTON:
return String.format(Locale.US, "ACTION(%d)", mCategoryInfo.getAdditionalAction());
case State.LOADING:
case State.INITIAL_LOADING:
case State.MORE_BUTTON_LOADING:
return "PROGRESS";
case State.HIDDEN:
// If state is HIDDEN, itemCount should be 0 and this method should not be called.
......@@ -103,10 +105,10 @@ public class ActionItem extends OptionalLeaf {
if (mState == newState) return;
mState = newState;
if (mState == State.LOADING) {
mSpinnerDurationTracker.startTracking();
if (mState == State.INITIAL_LOADING || mState == State.MORE_BUTTON_LOADING) {
mSpinnerDurationTracker.startTracking(mState);
} else {
mSpinnerDurationTracker.endTracking();
mSpinnerDurationTracker.endCompleteTracking();
}
boolean newVisibility = (newState != State.HIDDEN);
......@@ -127,6 +129,10 @@ public class ActionItem extends OptionalLeaf {
}
}
public void destroy() {
mSpinnerDurationTracker.endIncompleteTracking();
}
/**
* Perform the Action associated with this ActionItem.
* @param uiDelegate A {@link SuggestionsUiDelegate} to provide context.
......@@ -220,7 +226,7 @@ public class ActionItem extends OptionalLeaf {
if (state == State.BUTTON) {
mButton.setVisibility(View.VISIBLE);
mProgressIndicator.hide(/* keepSpace = */ true);
} else if (state == State.LOADING) {
} else if (state == State.INITIAL_LOADING || state == State.MORE_BUTTON_LOADING) {
mButton.setVisibility(View.INVISIBLE);
mProgressIndicator.show();
} else {
......
......@@ -203,6 +203,7 @@ public class SuggestionsSection extends InnerNode<NewTabPageViewHolder, PartialB
mOfflineModelObserver.onDestroy();
if (mSigninPromo != null) mSigninPromo.destroy();
mSuggestionsList.destroy();
mMoreButton.destroy();
mIsDestroyed = true;
}
......@@ -301,7 +302,8 @@ public class SuggestionsSection extends InnerNode<NewTabPageViewHolder, PartialB
/** Whether the section is waiting for content to be loaded. */
public boolean isLoading() {
return mMoreButton.getState() == ActionItem.State.LOADING;
return mMoreButton.getState() == ActionItem.State.INITIAL_LOADING
|| mMoreButton.getState() == ActionItem.State.MORE_BUTTON_LOADING;
}
/**
......@@ -461,7 +463,7 @@ public class SuggestionsSection extends InnerNode<NewTabPageViewHolder, PartialB
return;
}
mMoreButton.updateState(ActionItem.State.LOADING);
mMoreButton.updateState(ActionItem.State.MORE_BUTTON_LOADING);
mSuggestionsSource.fetchSuggestions(mCategoryInfo.getCategory(),
getDisplayedSuggestionIds(), suggestions -> { /* successCallback */
if (mIsDestroyed) return; // The section has been dismissed.
......@@ -491,7 +493,7 @@ public class SuggestionsSection extends InnerNode<NewTabPageViewHolder, PartialB
boolean isLoading = SnippetsBridge.isCategoryLoading(status);
mMoreButton.updateState(!shouldShowSuggestions()
? ActionItem.State.HIDDEN
: (isLoading ? ActionItem.State.LOADING : ActionItem.State.BUTTON));
: (isLoading ? ActionItem.State.INITIAL_LOADING : ActionItem.State.BUTTON));
if (mSigninPromo != null) {
mSigninPromo.setCanShowPersonalizedSuggestions(shouldShowSuggestions());
......
......@@ -6,10 +6,12 @@ package org.chromium.chrome.browser.suggestions;
import android.support.v7.widget.RecyclerView;
import org.chromium.base.Callback;
import com.google.android.libraries.feed.host.logging.SpinnerType;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.ntp.cards.ActionItem.State;
import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
import org.chromium.chrome.browser.ntp.snippets.FaviconFetchResult;
import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
......@@ -126,14 +128,11 @@ public abstract class SuggestionsMetrics {
}
/**
* @return A {@link DurationTracker} to notify to report how long the spinner is visible
* @return A {@link SpinnerDurationTracker} to notify to report how long the spinner is visible
* for.
*/
public static DurationTracker getSpinnerVisibilityReporter() {
return new DurationTracker((duration) -> {
RecordHistogram.recordTimesHistogram(
"ContentSuggestions.Feed.FetchPendingSpinner.VisibleDuration", duration);
});
public static SpinnerDurationTracker getSpinnerVisibilityReporter() {
return new SpinnerDurationTracker();
}
/**
......@@ -167,31 +166,71 @@ public abstract class SuggestionsMetrics {
}
/**
* Utility class to track the duration of an event. Call {@link #startTracking()} and
* {@link #endTracking()} to notify about the key moments. These methods are no-ops when called
* while tracking is not in the expected state.
* Utility class to track the duration of a spinner. Call {@link #startTracking(state)} when a
* Spinner start, call {@link #endCompleteTracking()} when a loading spinner finishes showing,
* and call {@link #endIncompleteTracking()} when a spinner is destroyed without completing.
* These methods are no-ops when called while tracking is not in the expected state.
*/
public static class DurationTracker {
public static class SpinnerDurationTracker {
private long mTrackingStartTimeMs;
private final Callback<Long> mTrackingCompleteCallback;
private @SpinnerType int mSpinnerType;
private DurationTracker(Callback<Long> trackingCompleteCallback) {
mTrackingCompleteCallback = trackingCompleteCallback;
private SpinnerDurationTracker() {
mTrackingStartTimeMs = 0;
}
public void startTracking() {
/**
* Start tracking of the spinner.
* @param state The state of the {@link ActionItem}.
*/
public void startTracking(@State int state) {
assert state == State.INITIAL_LOADING || state == State.MORE_BUTTON_LOADING;
if (isTracking()) return;
if (state == State.INITIAL_LOADING) {
mSpinnerType = SpinnerType.INITIAL_LOAD;
} else if (state == State.MORE_BUTTON_LOADING) {
mSpinnerType = SpinnerType.MORE_BUTTON;
}
mTrackingStartTimeMs = System.currentTimeMillis();
RecordHistogram.recordEnumeratedHistogram(
"ContentSuggestions.Feed.FetchPendingSpinner.Shown", mSpinnerType,
SpinnerType.NEXT_VALUE);
}
public void endTracking() {
/**
* Stop tracking of the spinner which is destroyed without completing.
*/
public void endCompleteTracking() {
if (!isTracking()) return;
mTrackingCompleteCallback.onResult(System.currentTimeMillis() - mTrackingStartTimeMs);
mTrackingStartTimeMs = 0;
recordSpinnerTimeUMA("ContentSuggestions.Feed.FetchPendingSpinner.VisibleDuration");
}
/**
* Stop tracking of the spinner which finishes showing.
*/
public void endIncompleteTracking() {
if (!isTracking()) return;
recordSpinnerTimeUMA(
"ContentSuggestions.Feed.FetchPendingSpinner.VisibleDurationWithoutCompleting");
}
private boolean isTracking() {
return mTrackingStartTimeMs > 0;
}
private void recordSpinnerTimeUMA(String baseName) {
long duration = System.currentTimeMillis() - mTrackingStartTimeMs;
RecordHistogram.recordTimesHistogram(baseName, duration);
if (mSpinnerType == SpinnerType.INITIAL_LOAD) {
RecordHistogram.recordTimesHistogram(baseName + ".InitialLoad", duration);
} else if (mSpinnerType == SpinnerType.MORE_BUTTON) {
RecordHistogram.recordTimesHistogram(baseName + ".MoreButton", duration);
}
mTrackingStartTimeMs = 0;
}
}
}
......@@ -395,13 +395,13 @@ public class NewTabPageAdapterTest {
ActionItem item = section.getActionItemForTesting();
mSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.INITIALIZING);
assertEquals(ActionItem.State.LOADING, item.getState());
assertEquals(ActionItem.State.INITIAL_LOADING, item.getState());
mSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.AVAILABLE);
assertEquals(ActionItem.State.HIDDEN, item.getState());
mSource.setStatusForCategory(TEST_CATEGORY, CategoryStatus.AVAILABLE_LOADING);
assertEquals(ActionItem.State.LOADING, item.getState());
assertEquals(ActionItem.State.INITIAL_LOADING, item.getState());
// After the section gets disabled, it should gone completely, so checking the progress
// indicator doesn't make sense anymore.
......
......@@ -559,7 +559,8 @@ public class SuggestionsSectionTest {
// Tap the button
verifyAction(section, ContentSuggestionsAdditionalAction.FETCH);
assertEquals(ActionItem.State.LOADING, section.getActionItemForTesting().getState());
assertEquals(
ActionItem.State.MORE_BUTTON_LOADING, section.getActionItemForTesting().getState());
// Simulate receiving suggestions.
section.setStatus(CategoryStatus.AVAILABLE);
......@@ -643,7 +644,8 @@ public class SuggestionsSectionTest {
eq(REMOTE_TEST_CATEGORY), any(), any(), sourceOnFailureRunnable.capture());
// Ensure the progress spinner is shown.
assertEquals(ActionItem.State.LOADING, section.getActionItemForTesting().getState());
assertEquals(
ActionItem.State.MORE_BUTTON_LOADING, section.getActionItemForTesting().getState());
// Simulate a failure.
sourceOnFailureRunnable.getValue().run();
......
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