Commit 75ce6b19 authored by Peter E Conn's avatar Peter E Conn Committed by Commit Bot

🏡 Display Snackbar when user triggered fetch fails.

When the user triggers a fetch through the more button, if the fetch
fails, display a Snackbar. Ideally in the future we can offer the user
information specific to how it failed and actions to remedy it.

Bug: 663376, 765603
Change-Id: I12fb27631454fb015803ae4a24a3d69c0a13357c
Reviewed-on: https://chromium-review.googlesource.com/671269Reviewed-by: default avatarBernhard Bauer <bauerb@chromium.org>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Commit-Queue: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504427}
parent f109b625
...@@ -33,6 +33,7 @@ import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; ...@@ -33,6 +33,7 @@ import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.search_engines.TemplateUrlService; import org.chromium.chrome.browser.search_engines.TemplateUrlService;
import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrlServiceObserver; import org.chromium.chrome.browser.search_engines.TemplateUrlService.TemplateUrlServiceObserver;
import org.chromium.chrome.browser.snackbar.SnackbarManager;
import org.chromium.chrome.browser.suggestions.SuggestionsDependencyFactory; import org.chromium.chrome.browser.suggestions.SuggestionsDependencyFactory;
import org.chromium.chrome.browser.suggestions.SuggestionsEventReporter; import org.chromium.chrome.browser.suggestions.SuggestionsEventReporter;
import org.chromium.chrome.browser.suggestions.SuggestionsMetrics; import org.chromium.chrome.browser.suggestions.SuggestionsMetrics;
...@@ -159,9 +160,10 @@ public class NewTabPage ...@@ -159,9 +160,10 @@ public class NewTabPage
public NewTabPageManagerImpl(SuggestionsSource suggestionsSource, public NewTabPageManagerImpl(SuggestionsSource suggestionsSource,
SuggestionsEventReporter eventReporter, SuggestionsEventReporter eventReporter,
SuggestionsNavigationDelegate navigationDelegate, Profile profile, SuggestionsNavigationDelegate navigationDelegate, Profile profile,
NativePageHost nativePageHost, DiscardableReferencePool referencePool) { NativePageHost nativePageHost, DiscardableReferencePool referencePool,
SnackbarManager snackbarManager) {
super(suggestionsSource, eventReporter, navigationDelegate, profile, nativePageHost, super(suggestionsSource, eventReporter, navigationDelegate, profile, nativePageHost,
referencePool); referencePool, snackbarManager);
} }
@Override @Override
...@@ -265,7 +267,8 @@ public class NewTabPage ...@@ -265,7 +267,8 @@ public class NewTabPage
new SuggestionsNavigationDelegateImpl( new SuggestionsNavigationDelegateImpl(
activity, profile, nativePageHost, tabModelSelector); activity, profile, nativePageHost, tabModelSelector);
mNewTabPageManager = new NewTabPageManagerImpl(suggestionsSource, eventReporter, mNewTabPageManager = new NewTabPageManagerImpl(suggestionsSource, eventReporter,
navigationDelegate, profile, nativePageHost, activity.getReferencePool()); navigationDelegate, profile, nativePageHost, activity.getReferencePool(),
activity.getSnackbarManager());
mTileGroupDelegate = new NewTabPageTileGroupDelegate(activity, profile, navigationDelegate); mTileGroupDelegate = new NewTabPageTileGroupDelegate(activity, profile, navigationDelegate);
mTitle = activity.getResources().getString(R.string.button_new_tab); mTitle = activity.getResources().getString(R.string.button_new_tab);
......
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.ntp.cards; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.ntp.cards;
import android.support.annotation.IntDef; import android.support.annotation.IntDef;
import android.support.annotation.LayoutRes; import android.support.annotation.LayoutRes;
import android.support.annotation.Nullable;
import android.view.View; import android.view.View;
import android.widget.Button; import android.widget.Button;
...@@ -14,6 +15,8 @@ import org.chromium.chrome.R; ...@@ -14,6 +15,8 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.metrics.ImpressionTracker; import org.chromium.chrome.browser.metrics.ImpressionTracker;
import org.chromium.chrome.browser.ntp.ContextMenuManager; import org.chromium.chrome.browser.ntp.ContextMenuManager;
import org.chromium.chrome.browser.ntp.snippets.CategoryInt; import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
import org.chromium.chrome.browser.snackbar.Snackbar;
import org.chromium.chrome.browser.snackbar.SnackbarManager;
import org.chromium.chrome.browser.suggestions.ContentSuggestionsAdditionalAction; import org.chromium.chrome.browser.suggestions.ContentSuggestionsAdditionalAction;
import org.chromium.chrome.browser.suggestions.SuggestionsMetrics; import org.chromium.chrome.browser.suggestions.SuggestionsMetrics;
import org.chromium.chrome.browser.suggestions.SuggestionsRanker; import org.chromium.chrome.browser.suggestions.SuggestionsRanker;
...@@ -114,8 +117,14 @@ public class ActionItem extends OptionalLeaf { ...@@ -114,8 +117,14 @@ public class ActionItem extends OptionalLeaf {
return mState; return mState;
} }
/**
* Perform the Action associated with this ActionItem.
* @param uiDelegate A {@link SuggestionsUiDelegate} to provide context.
* @param onFailure A {@link Runnable} that will be run if the action was to fetch more
* suggestions but that action failed.
*/
@VisibleForTesting @VisibleForTesting
void performAction(SuggestionsUiDelegate uiDelegate) { void performAction(SuggestionsUiDelegate uiDelegate, @Nullable Runnable onFailure) {
assert mState == State.BUTTON; assert mState == State.BUTTON;
uiDelegate.getEventReporter().onMoreButtonClicked(this); uiDelegate.getEventReporter().onMoreButtonClicked(this);
...@@ -127,7 +136,7 @@ public class ActionItem extends OptionalLeaf { ...@@ -127,7 +136,7 @@ public class ActionItem extends OptionalLeaf {
mCategoryInfo.performViewAllAction(uiDelegate.getNavigationDelegate()); mCategoryInfo.performViewAllAction(uiDelegate.getNavigationDelegate());
return; return;
case ContentSuggestionsAdditionalAction.FETCH: case ContentSuggestionsAdditionalAction.FETCH:
mParentSection.fetchSuggestions(); mParentSection.fetchSuggestions(onFailure);
return; return;
case ContentSuggestionsAdditionalAction.NONE: case ContentSuggestionsAdditionalAction.NONE:
default: default:
...@@ -141,15 +150,18 @@ public class ActionItem extends OptionalLeaf { ...@@ -141,15 +150,18 @@ public class ActionItem extends OptionalLeaf {
private ActionItem mActionListItem; private ActionItem mActionListItem;
private final ProgressIndicatorView mProgressIndicator; private final ProgressIndicatorView mProgressIndicator;
private final Button mButton; private final Button mButton;
private final SuggestionsUiDelegate mUiDelegate;
public ViewHolder(SuggestionsRecyclerView recyclerView, public ViewHolder(SuggestionsRecyclerView recyclerView,
ContextMenuManager contextMenuManager, SuggestionsUiDelegate uiDelegate, ContextMenuManager contextMenuManager, final SuggestionsUiDelegate uiDelegate,
UiConfig uiConfig) { UiConfig uiConfig) {
super(getLayout(), recyclerView, uiConfig, contextMenuManager); super(getLayout(), recyclerView, uiConfig, contextMenuManager);
mProgressIndicator = itemView.findViewById(R.id.progress_indicator); mProgressIndicator = itemView.findViewById(R.id.progress_indicator);
mButton = itemView.findViewById(R.id.action_button); mButton = itemView.findViewById(R.id.action_button);
mButton.setOnClickListener(v -> mActionListItem.performAction(uiDelegate)); mUiDelegate = uiDelegate;
mButton.setOnClickListener(v -> mActionListItem.performAction(uiDelegate,
this::showFetchFailureSnackbar));
new ImpressionTracker(itemView, () -> { new ImpressionTracker(itemView, () -> {
if (mActionListItem != null && !mActionListItem.mImpressionTracked) { if (mActionListItem != null && !mActionListItem.mImpressionTracked) {
...@@ -159,6 +171,15 @@ public class ActionItem extends OptionalLeaf { ...@@ -159,6 +171,15 @@ public class ActionItem extends OptionalLeaf {
}); });
} }
private void showFetchFailureSnackbar() {
mUiDelegate.getSnackbarManager().showSnackbar(Snackbar.make(
itemView.getResources().getString(R.string.ntp_suggestions_fetch_failed),
new SnackbarManager.SnackbarController() { },
Snackbar.TYPE_ACTION,
Snackbar.UMA_SNIPPET_FETCH_FAILED)
);
}
public void onBindViewHolder(ActionItem item) { public void onBindViewHolder(ActionItem item) {
super.onBindViewHolder(); super.onBindViewHolder();
mActionListItem = item; mActionListItem = item;
......
...@@ -244,7 +244,10 @@ public class SectionList ...@@ -244,7 +244,10 @@ public class SectionList
} else if (supportingSections.get(0).isLoading()) { } else if (supportingSections.get(0).isLoading()) {
Log.d(TAG, "SectionList.fetchMore - Supporting section is already loading."); Log.d(TAG, "SectionList.fetchMore - Supporting section is already loading.");
} else { } else {
supportingSections.get(0).fetchSuggestions(); // Fetch more is called when the user does not explicitly trigger a fetch (eg, the user
// scrolls down). In this case we don't inform the user of a failure, hence the null
// parameter.
supportingSections.get(0).fetchSuggestions(null);
} }
} }
......
...@@ -534,8 +534,12 @@ public class SuggestionsSection extends InnerNode { ...@@ -534,8 +534,12 @@ public class SuggestionsSection extends InnerNode {
return true; return true;
} }
/** Fetches additional suggestions only for this section. */
public void fetchSuggestions() { /**
* Fetches additional suggestions only for this section.
* @param onFailure A {@link Runnable} that will be run if the fetch fails.
*/
public void fetchSuggestions(@Nullable final Runnable onFailure) {
assert !isLoading(); assert !isLoading();
if (getSuggestionsCount() == 0 && getCategoryInfo().isRemote()) { if (getSuggestionsCount() == 0 && getCategoryInfo().isRemote()) {
...@@ -546,12 +550,19 @@ public class SuggestionsSection extends InnerNode { ...@@ -546,12 +550,19 @@ public class SuggestionsSection extends InnerNode {
} }
mMoreButton.updateState(ActionItem.State.LOADING); mMoreButton.updateState(ActionItem.State.LOADING);
mSuggestionsSource.fetchSuggestions( mSuggestionsSource.fetchSuggestions(mCategoryInfo.getCategory(),
mCategoryInfo.getCategory(), getDisplayedSuggestionIds(), additionalSuggestions -> { getDisplayedSuggestionIds(),
suggestions -> { /* successCallback */
if (!isAttached()) return; // The section has been dismissed.
mMoreButton.updateState(ActionItem.State.BUTTON);
appendSuggestions(suggestions, /* keepSectionSize = */ false);
},
() -> { /* failureRunnable */
if (!isAttached()) return; // The section has been dismissed. if (!isAttached()) return; // The section has been dismissed.
appendSuggestions(additionalSuggestions, /* keepSectionSize = */ false);
mMoreButton.updateState(ActionItem.State.BUTTON); mMoreButton.updateState(ActionItem.State.BUTTON);
if (onFailure != null) onFailure.run();
}); });
} }
......
...@@ -186,9 +186,13 @@ public class SnippetsBridge implements SuggestionsSource { ...@@ -186,9 +186,13 @@ public class SnippetsBridge implements SuggestionsSource {
@Override @Override
public void fetchSuggestions(@CategoryInt int category, String[] displayedSuggestionIds, public void fetchSuggestions(@CategoryInt int category, String[] displayedSuggestionIds,
Callback<List<SnippetArticle>> callback) { Callback<List<SnippetArticle>> successCallback, Runnable failureRunnable) {
assert mNativeSnippetsBridge != 0; assert mNativeSnippetsBridge != 0;
nativeFetch(mNativeSnippetsBridge, category, displayedSuggestionIds, callback); // We have nice JNI support for Callbacks but not for Runnables, so wrap the Runnable
// in a Callback and discard the parameter.
// TODO(peconn): Use a Runnable here if they get nice JNI support.
nativeFetch(mNativeSnippetsBridge, category, displayedSuggestionIds, successCallback,
ignored -> failureRunnable.run());
} }
@CalledByNative @CalledByNative
...@@ -281,7 +285,8 @@ public class SnippetsBridge implements SuggestionsSource { ...@@ -281,7 +285,8 @@ public class SnippetsBridge implements SuggestionsSource {
String idWithinCategory, int minimumSizePx, int desiredSizePx, String idWithinCategory, int minimumSizePx, int desiredSizePx,
Callback<Bitmap> callback); Callback<Bitmap> callback);
private native void nativeFetch(long nativeNTPSnippetsBridge, int category, private native void nativeFetch(long nativeNTPSnippetsBridge, int category,
String[] knownSuggestions, Callback<List<SnippetArticle>> callback); String[] knownSuggestions, Callback<List<SnippetArticle>> successCallback,
Callback<Integer> failureCallback);
private native void nativeFetchContextualSuggestions( private native void nativeFetchContextualSuggestions(
long nativeNTPSnippetsBridge, String url, Callback<List<SnippetArticle>> callback); long nativeNTPSnippetsBridge, String url, Callback<List<SnippetArticle>> callback);
private native void nativeFetchContextualSuggestionImage(long nativeNTPSnippetsBridge, private native void nativeFetchContextualSuggestionImage(long nativeNTPSnippetsBridge,
......
...@@ -100,10 +100,11 @@ public interface SuggestionsSource { ...@@ -100,10 +100,11 @@ public interface SuggestionsSource {
* Fetches new suggestions. * Fetches new suggestions.
* @param category the category to fetch new suggestions for. * @param category the category to fetch new suggestions for.
* @param displayedSuggestionIds ids of suggestions already known and that we want to keep. * @param displayedSuggestionIds ids of suggestions already known and that we want to keep.
* @param callback The callback to run with the received suggestions. * @param successCallback The callback to run with the received suggestions.
* @param failureRunnable The runnable to be run if the fetch fails.
*/ */
void fetchSuggestions(@CategoryInt int category, String[] displayedSuggestionIds, void fetchSuggestions(@CategoryInt int category, String[] displayedSuggestionIds,
Callback<List<SnippetArticle>> callback); Callback<List<SnippetArticle>> successCallback, Runnable failureRunnable);
/** /**
* Fetches suggestions related to the provided URL. * Fetches suggestions related to the provided URL.
......
...@@ -58,6 +58,7 @@ public class Snackbar { ...@@ -58,6 +58,7 @@ public class Snackbar {
public static final int UMA_TRANSLATE_ALWAYS = 18; public static final int UMA_TRANSLATE_ALWAYS = 18;
public static final int UMA_TRANSLATE_NEVER = 19; public static final int UMA_TRANSLATE_NEVER = 19;
public static final int UMA_TRANSLATE_NEVER_SITE = 20; public static final int UMA_TRANSLATE_NEVER_SITE = 20;
public static final int UMA_SNIPPET_FETCH_FAILED = 21;
private SnackbarController mController; private SnackbarController mController;
private CharSequence mText; private CharSequence mText;
......
...@@ -48,13 +48,13 @@ public class SnackbarManager implements OnClickListener, InfoBarContainer.InfoBa ...@@ -48,13 +48,13 @@ public class SnackbarManager implements OnClickListener, InfoBarContainer.InfoBa
* Called when the user clicks the action button on the snackbar. * Called when the user clicks the action button on the snackbar.
* @param actionData Data object passed when showing this specific snackbar. * @param actionData Data object passed when showing this specific snackbar.
*/ */
void onAction(Object actionData); default void onAction(Object actionData) { }
/** /**
* Called when the snackbar is dismissed by tiemout or UI enviroment change. * Called when the snackbar is dismissed by tiemout or UI enviroment change.
* @param actionData Data object associated with the dismissed snackbar entry. * @param actionData Data object associated with the dismissed snackbar entry.
*/ */
void onDismissNoAction(Object actionData); default void onDismissNoAction(Object actionData) { }
} }
private static final int DEFAULT_SNACKBAR_DURATION_MS = 3000; private static final int DEFAULT_SNACKBAR_DURATION_MS = 3000;
......
...@@ -89,7 +89,7 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon ...@@ -89,7 +89,7 @@ public class SuggestionsBottomSheetContent implements BottomSheet.BottomSheetCon
new TileGroupDelegateImpl(activity, profile, navigationDelegate, snackbarManager); new TileGroupDelegateImpl(activity, profile, navigationDelegate, snackbarManager);
mSuggestionsUiDelegate = new SuggestionsUiDelegateImpl( mSuggestionsUiDelegate = new SuggestionsUiDelegateImpl(
depsFactory.createSuggestionSource(profile), depsFactory.createEventReporter(), depsFactory.createSuggestionSource(profile), depsFactory.createEventReporter(),
navigationDelegate, profile, sheet, activity.getReferencePool()); navigationDelegate, profile, sheet, activity.getReferencePool(), snackbarManager);
mView = LayoutInflater.from(activity).inflate( mView = LayoutInflater.from(activity).inflate(
R.layout.suggestions_bottom_sheet_content, null); R.layout.suggestions_bottom_sheet_content, null);
......
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.suggestions; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.suggestions;
import org.chromium.base.DiscardableReferencePool; import org.chromium.base.DiscardableReferencePool;
import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
import org.chromium.chrome.browser.snackbar.SnackbarManager;
/** /**
* Interface between the suggestion surface and the rest of the browser. * Interface between the suggestion surface and the rest of the browser.
...@@ -27,9 +28,12 @@ public interface SuggestionsUiDelegate { ...@@ -27,9 +28,12 @@ public interface SuggestionsUiDelegate {
/** Convenience method to access the {@link SuggestionsNavigationDelegate}. */ /** Convenience method to access the {@link SuggestionsNavigationDelegate}. */
SuggestionsNavigationDelegate getNavigationDelegate(); SuggestionsNavigationDelegate getNavigationDelegate();
/** Convenience method to access the {@link ImageFetcher} */ /** Convenience method to access the {@link ImageFetcher}. */
ImageFetcher getImageFetcher(); ImageFetcher getImageFetcher();
/** Convenience method to access the {@link SnackbarManager}. */
SnackbarManager getSnackbarManager();
/** /**
* @return The reference pool to use for large objects that should be dropped under * @return The reference pool to use for large objects that should be dropped under
* memory pressure. * memory pressure.
......
...@@ -10,6 +10,7 @@ import org.chromium.base.DiscardableReferencePool; ...@@ -10,6 +10,7 @@ import org.chromium.base.DiscardableReferencePool;
import org.chromium.chrome.browser.NativePageHost; import org.chromium.chrome.browser.NativePageHost;
import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.snackbar.SnackbarManager;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
...@@ -25,6 +26,7 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate { ...@@ -25,6 +26,7 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
private final SuggestionsNavigationDelegate mSuggestionsNavigationDelegate; private final SuggestionsNavigationDelegate mSuggestionsNavigationDelegate;
private final NativePageHost mHost; private final NativePageHost mHost;
private final ImageFetcher mImageFetcher; private final ImageFetcher mImageFetcher;
private final SnackbarManager mSnackbarManager;
private final DiscardableReferencePool mReferencePool; private final DiscardableReferencePool mReferencePool;
...@@ -33,12 +35,13 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate { ...@@ -33,12 +35,13 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
public SuggestionsUiDelegateImpl(SuggestionsSource suggestionsSource, public SuggestionsUiDelegateImpl(SuggestionsSource suggestionsSource,
SuggestionsEventReporter eventReporter, SuggestionsEventReporter eventReporter,
SuggestionsNavigationDelegate navigationDelegate, Profile profile, NativePageHost host, SuggestionsNavigationDelegate navigationDelegate, Profile profile, NativePageHost host,
DiscardableReferencePool referencePool) { DiscardableReferencePool referencePool, SnackbarManager snackbarManager) {
mSuggestionsSource = suggestionsSource; mSuggestionsSource = suggestionsSource;
mSuggestionsRanker = new SuggestionsRanker(); mSuggestionsRanker = new SuggestionsRanker();
mSuggestionsEventReporter = eventReporter; mSuggestionsEventReporter = eventReporter;
mSuggestionsNavigationDelegate = navigationDelegate; mSuggestionsNavigationDelegate = navigationDelegate;
mImageFetcher = new ImageFetcher(suggestionsSource, profile, referencePool, host); mImageFetcher = new ImageFetcher(suggestionsSource, profile, referencePool, host);
mSnackbarManager = snackbarManager;
mHost = host; mHost = host;
mReferencePool = referencePool; mReferencePool = referencePool;
...@@ -66,6 +69,11 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate { ...@@ -66,6 +69,11 @@ public class SuggestionsUiDelegateImpl implements SuggestionsUiDelegate {
return mSuggestionsNavigationDelegate; return mSuggestionsNavigationDelegate;
} }
@Override
public SnackbarManager getSnackbarManager() {
return mSnackbarManager;
}
@Override @Override
public ImageFetcher getImageFetcher() { public ImageFetcher getImageFetcher() {
return mImageFetcher; return mImageFetcher;
......
...@@ -2406,6 +2406,9 @@ To obtain new licenses, connect to the internet and play your downloaded content ...@@ -2406,6 +2406,9 @@ To obtain new licenses, connect to the internet and play your downloaded content
<message name="IDS_NTP_ALL_DISMISSED_BODY_TEXT_MODERN" desc="Body text shown on the New Tab Page when all suggested content has been dismissed. The message refers to the &quot;More&quot; button to fetch more suggestions."> <message name="IDS_NTP_ALL_DISMISSED_BODY_TEXT_MODERN" desc="Body text shown on the New Tab Page when all suggested content has been dismissed. The message refers to the &quot;More&quot; button to fetch more suggestions.">
Tap More to get articles Tap More to get articles
</message> </message>
<message name="IDS_NTP_SUGGESTIONS_FETCH_FAILED" desc="Snackbar text shown when the user presses the More button to get more suggestions, but this fails (eg, no internet connectivity).">
Unable to get suggestions
</message>
<message name="IDS_NTP_ALL_DISMISSED_REFRESH" desc="Text label for button to refresh the New Tab Page when all suggested content has been dismissed. [CHAR-LIMIT=20]"> <message name="IDS_NTP_ALL_DISMISSED_REFRESH" desc="Text label for button to refresh the New Tab Page when all suggested content has been dismissed. [CHAR-LIMIT=20]">
Refresh Refresh
</message> </message>
......
...@@ -52,6 +52,7 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceManager; ...@@ -52,6 +52,7 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
import org.chromium.chrome.browser.signin.DisplayableProfileData; import org.chromium.chrome.browser.signin.DisplayableProfileData;
import org.chromium.chrome.browser.signin.SigninAccessPoint; import org.chromium.chrome.browser.signin.SigninAccessPoint;
import org.chromium.chrome.browser.signin.SigninPromoController; import org.chromium.chrome.browser.signin.SigninPromoController;
import org.chromium.chrome.browser.snackbar.SnackbarManager;
import org.chromium.chrome.browser.suggestions.ContentSuggestionsAdditionalAction; import org.chromium.chrome.browser.suggestions.ContentSuggestionsAdditionalAction;
import org.chromium.chrome.browser.suggestions.DestructionObserver; import org.chromium.chrome.browser.suggestions.DestructionObserver;
import org.chromium.chrome.browser.suggestions.ImageFetcher; import org.chromium.chrome.browser.suggestions.ImageFetcher;
...@@ -463,6 +464,11 @@ public class ArticleSnippetsTest { ...@@ -463,6 +464,11 @@ public class ArticleSnippetsTest {
public ImageFetcher getImageFetcher() { public ImageFetcher getImageFetcher() {
return mImageFetcher; return mImageFetcher;
} }
@Override
public SnackbarManager getSnackbarManager() {
return mActivityTestRule.getActivity().getSnackbarManager();
}
} }
private class MockImageFetcher extends ImageFetcher { private class MockImageFetcher extends ImageFetcher {
......
...@@ -339,7 +339,7 @@ public class TileGridLayoutTest { ...@@ -339,7 +339,7 @@ public class TileGridLayoutTest {
SuggestionsUiDelegate uiDelegate = new SuggestionsUiDelegateImpl( SuggestionsUiDelegate uiDelegate = new SuggestionsUiDelegateImpl(
mSuggestionsDeps.getFactory().createSuggestionSource(null), mSuggestionsDeps.getFactory().createSuggestionSource(null),
mSuggestionsDeps.getFactory().createEventReporter(), null, profile, null, mSuggestionsDeps.getFactory().createEventReporter(), null, profile, null,
activity.getChromeApplication().getReferencePool()); activity.getChromeApplication().getReferencePool(), activity.getSnackbarManager());
OfflinePageBridge opb = OfflinePageBridge.getForProfile(profile); OfflinePageBridge opb = OfflinePageBridge.getForProfile(profile);
TileGroup.Delegate delegate = new TileGroupDelegateImpl(activity, profile, null, null); TileGroup.Delegate delegate = new TileGroupDelegateImpl(activity, profile, null, null);
......
...@@ -34,6 +34,7 @@ import org.junit.Before; ...@@ -34,6 +34,7 @@ import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.robolectric.RuntimeEnvironment; import org.robolectric.RuntimeEnvironment;
...@@ -430,6 +431,7 @@ public class SuggestionsSectionTest { ...@@ -430,6 +431,7 @@ public class SuggestionsSectionTest {
@Feature({"Ntp"}) @Feature({"Ntp"})
public void testFetchMoreProgressDisplay() { public void testFetchMoreProgressDisplay() {
final int suggestionCount = 3; final int suggestionCount = 3;
// Spy so that VerifyAction can check methods being called.
SuggestionsCategoryInfo info = SuggestionsCategoryInfo info =
spy(new CategoryInfoBuilder(TEST_CATEGORY_ID) spy(new CategoryInfoBuilder(TEST_CATEGORY_ID)
.withAction(ContentSuggestionsAdditionalAction.FETCH) .withAction(ContentSuggestionsAdditionalAction.FETCH)
...@@ -459,12 +461,10 @@ public class SuggestionsSectionTest { ...@@ -459,12 +461,10 @@ public class SuggestionsSectionTest {
@Feature({"Ntp"}) @Feature({"Ntp"})
public void testFetchMoreAfterDismissAll() { public void testFetchMoreAfterDismissAll() {
final int suggestionCount = 10; final int suggestionCount = 10;
SuggestionsCategoryInfo info = SuggestionsSection section = createSection(new CategoryInfoBuilder(REMOTE_TEST_CATEGORY)
spy(new CategoryInfoBuilder(REMOTE_TEST_CATEGORY)
.withAction(ContentSuggestionsAdditionalAction.FETCH) .withAction(ContentSuggestionsAdditionalAction.FETCH)
.showIfEmpty() .showIfEmpty()
.build()); .build());
SuggestionsSection section = createSection(info);
section.setStatus(CategoryStatus.AVAILABLE); section.setStatus(CategoryStatus.AVAILABLE);
section.appendSuggestions(createDummySuggestions(suggestionCount, REMOTE_TEST_CATEGORY), section.appendSuggestions(createDummySuggestions(suggestionCount, REMOTE_TEST_CATEGORY),
/*keepSectionSize=*/true); /*keepSectionSize=*/true);
...@@ -485,7 +485,7 @@ public class SuggestionsSectionTest { ...@@ -485,7 +485,7 @@ public class SuggestionsSectionTest {
// Tap the button -- we handle this case explicitly here to avoid complexity in // Tap the button -- we handle this case explicitly here to avoid complexity in
// verifyAction(). // verifyAction().
section.getActionItemForTesting().performAction(mUiDelegate); section.getActionItemForTesting().performAction(mUiDelegate, null);
verify(mUiDelegate.getSuggestionsSource(), times(1)).fetchRemoteSuggestions(); verify(mUiDelegate.getSuggestionsSource(), times(1)).fetchRemoteSuggestions();
// Simulate the arrival of new data. // Simulate the arrival of new data.
...@@ -501,6 +501,40 @@ public class SuggestionsSectionTest { ...@@ -501,6 +501,40 @@ public class SuggestionsSectionTest {
assertFalse(section.isDataStale()); assertFalse(section.isDataStale());
} }
@Test
@Feature("Ntp")
public void testFetchMoreFailure() {
SuggestionsSection section = createSection(new CategoryInfoBuilder(REMOTE_TEST_CATEGORY)
.withAction(ContentSuggestionsAdditionalAction.FETCH)
.showIfEmpty()
.build());
section.setStatus(CategoryStatus.AVAILABLE);
section.appendSuggestions(createDummySuggestions(10, REMOTE_TEST_CATEGORY),
/*keepSectionSize=*/true);
assertEquals(ActionItem.State.BUTTON, section.getActionItemForTesting().getState());
assertTrue(section.getCategoryInfo().isRemote());
// Tap the button, providing a Runnable for when it fails.
Runnable sectionOnFailureRunnable = mock(Runnable.class);
section.getActionItemForTesting().performAction(mUiDelegate, sectionOnFailureRunnable);
// Ensure the tap leads to a fetch request on the source with a (potentially different)
// failure Runnable.
ArgumentCaptor<Runnable> sourceOnFailureRunnable = ArgumentCaptor.forClass(Runnable.class);
verify(mUiDelegate.getSuggestionsSource(), times(1)).fetchSuggestions(
eq(REMOTE_TEST_CATEGORY), any(), any(), sourceOnFailureRunnable.capture());
// Ensure the progress spinner is shown.
assertEquals(ActionItem.State.LOADING, section.getActionItemForTesting().getState());
// Simulate a failure.
sourceOnFailureRunnable.getValue().run();
// Ensure we're back to the button and the section's failure runnable has been run.
assertEquals(ActionItem.State.BUTTON, section.getActionItemForTesting().getState());
verify(sectionOnFailureRunnable, times(1)).run();
}
/** /**
* Tests that the UI updates on updated suggestions. * Tests that the UI updates on updated suggestions.
*/ */
...@@ -908,7 +942,7 @@ public class SuggestionsSectionTest { ...@@ -908,7 +942,7 @@ public class SuggestionsSectionTest {
private void verifyAction( private void verifyAction(
SuggestionsSection section, @ContentSuggestionsAdditionalAction int action) { SuggestionsSection section, @ContentSuggestionsAdditionalAction int action) {
if (action != ContentSuggestionsAdditionalAction.NONE) { if (action != ContentSuggestionsAdditionalAction.NONE) {
section.getActionItemForTesting().performAction(mUiDelegate); section.getActionItemForTesting().performAction(mUiDelegate, null);
} }
verify(section.getCategoryInfo(), verify(section.getCategoryInfo(),
...@@ -918,6 +952,7 @@ public class SuggestionsSectionTest { ...@@ -918,6 +952,7 @@ public class SuggestionsSectionTest {
// noinspection unchecked -- See https://crbug.com/740162 for rationale. // noinspection unchecked -- See https://crbug.com/740162 for rationale.
verify(mUiDelegate.getSuggestionsSource(), verify(mUiDelegate.getSuggestionsSource(),
(action == ContentSuggestionsAdditionalAction.FETCH ? times(1) : never())) (action == ContentSuggestionsAdditionalAction.FETCH ? times(1) : never()))
.fetchSuggestions(anyInt(), any(String[].class), any(Callback.class)); .fetchSuggestions(anyInt(), any(String[].class), any(Callback.class),
any(Runnable.class));
} }
} }
...@@ -283,8 +283,10 @@ void NTPSnippetsBridge::Fetch( ...@@ -283,8 +283,10 @@ void NTPSnippetsBridge::Fetch(
const JavaParamRef<jobject>& obj, const JavaParamRef<jobject>& obj,
jint j_category_id, jint j_category_id,
const JavaParamRef<jobjectArray>& j_displayed_suggestions, const JavaParamRef<jobjectArray>& j_displayed_suggestions,
const JavaParamRef<jobject>& j_callback) { const JavaParamRef<jobject>& j_success_callback,
ScopedJavaGlobalRef<jobject> callback(j_callback); const JavaParamRef<jobject>& j_failure_callback) {
ScopedJavaGlobalRef<jobject> success_callback(j_success_callback);
ScopedJavaGlobalRef<jobject> failure_callback(j_failure_callback);
std::vector<std::string> known_suggestion_ids; std::vector<std::string> known_suggestion_ids;
AppendJavaStringArrayToStringVector(env, j_displayed_suggestions, AppendJavaStringArrayToStringVector(env, j_displayed_suggestions,
&known_suggestion_ids); &known_suggestion_ids);
...@@ -295,7 +297,8 @@ void NTPSnippetsBridge::Fetch( ...@@ -295,7 +297,8 @@ void NTPSnippetsBridge::Fetch(
std::set<std::string>(known_suggestion_ids.begin(), std::set<std::string>(known_suggestion_ids.begin(),
known_suggestion_ids.end()), known_suggestion_ids.end()),
base::Bind(&NTPSnippetsBridge::OnSuggestionsFetched, base::Bind(&NTPSnippetsBridge::OnSuggestionsFetched,
weak_ptr_factory_.GetWeakPtr(), callback, category)); weak_ptr_factory_.GetWeakPtr(), success_callback,
failure_callback, category));
} }
void NTPSnippetsBridge::FetchContextualSuggestions( void NTPSnippetsBridge::FetchContextualSuggestions(
...@@ -418,14 +421,22 @@ void NTPSnippetsBridge::OnImageFetched(ScopedJavaGlobalRef<jobject> callback, ...@@ -418,14 +421,22 @@ void NTPSnippetsBridge::OnImageFetched(ScopedJavaGlobalRef<jobject> callback,
} }
void NTPSnippetsBridge::OnSuggestionsFetched( void NTPSnippetsBridge::OnSuggestionsFetched(
const ScopedJavaGlobalRef<jobject>& callback, const ScopedJavaGlobalRef<jobject>& success_callback,
const ScopedJavaGlobalRef<jobject>& failure_callback,
Category category, Category category,
ntp_snippets::Status status, ntp_snippets::Status status,
std::vector<ContentSuggestion> suggestions) { std::vector<ContentSuggestion> suggestions) {
// TODO(fhorschig, dgn): Allow refetch or show notification acc. to status. // TODO(fhorschig, dgn): Allow refetch or show notification acc. to status.
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
RunCallbackAndroid(callback, if (status.IsSuccess()) {
RunCallbackAndroid(success_callback,
ToJavaSuggestionList(env, category, suggestions)); ToJavaSuggestionList(env, category, suggestions));
} else {
// The second parameter here means nothing - it was more convenient to pass
// a Callback (which has 1 parameter) over to the native side than a
// Runnable (which has no parameters). We ignore the parameter Java-side.
RunCallbackAndroid(failure_callback, 0);
}
} }
void NTPSnippetsBridge::OnContextualSuggestionsFetched( void NTPSnippetsBridge::OnContextualSuggestionsFetched(
......
...@@ -77,7 +77,8 @@ class NTPSnippetsBridge ...@@ -77,7 +77,8 @@ class NTPSnippetsBridge
const base::android::JavaParamRef<jobject>& obj, const base::android::JavaParamRef<jobject>& obj,
jint j_category_id, jint j_category_id,
const base::android::JavaParamRef<jobjectArray>& j_displayed_suggestions, const base::android::JavaParamRef<jobjectArray>& j_displayed_suggestions,
const base::android::JavaParamRef<jobject>& j_callback); const base::android::JavaParamRef<jobject>& j_fetch_success_callback,
const base::android::JavaParamRef<jobject>& j_fetch_failure_callback);
void FetchContextualSuggestions( void FetchContextualSuggestions(
JNIEnv* env, JNIEnv* env,
...@@ -128,7 +129,8 @@ class NTPSnippetsBridge ...@@ -128,7 +129,8 @@ class NTPSnippetsBridge
void OnImageFetched(base::android::ScopedJavaGlobalRef<jobject> callback, void OnImageFetched(base::android::ScopedJavaGlobalRef<jobject> callback,
const gfx::Image& image); const gfx::Image& image);
void OnSuggestionsFetched( void OnSuggestionsFetched(
const base::android::ScopedJavaGlobalRef<jobject>& callback, const base::android::ScopedJavaGlobalRef<jobject>& success_callback,
const base::android::ScopedJavaGlobalRef<jobject>& failure_callback,
ntp_snippets::Category category, ntp_snippets::Category category,
ntp_snippets::Status status, ntp_snippets::Status status,
std::vector<ntp_snippets::ContentSuggestion> suggestions); std::vector<ntp_snippets::ContentSuggestion> suggestions);
......
...@@ -243,7 +243,8 @@ public class FakeSuggestionsSource implements SuggestionsSource { ...@@ -243,7 +243,8 @@ public class FakeSuggestionsSource implements SuggestionsSource {
@Override @Override
public void fetchSuggestions(@CategoryInt int category, String[] displayedSuggestionIds, public void fetchSuggestions(@CategoryInt int category, String[] displayedSuggestionIds,
Callback<List<SnippetArticle>> callback) {} Callback<List<SnippetArticle>> successCallback, Runnable failureRunnable) {
}
@Override @Override
public void fetchContextualSuggestions(String url, Callback<List<SnippetArticle>> callback) { public void fetchContextualSuggestions(String url, Callback<List<SnippetArticle>> callback) {
......
...@@ -35729,6 +35729,7 @@ from previous Chrome versions. ...@@ -35729,6 +35729,7 @@ from previous Chrome versions.
<int value="18" label="TRANSLATE_ALWAYS"/> <int value="18" label="TRANSLATE_ALWAYS"/>
<int value="19" label="TRANSLATE_NEVER"/> <int value="19" label="TRANSLATE_NEVER"/>
<int value="20" label="TRANSLATE_NEVER_SITE"/> <int value="20" label="TRANSLATE_NEVER_SITE"/>
<int value="21" label="SNIPPET_FETCH_FAILED"/>
</enum> </enum>
<enum name="SnippetOpenMethod"> <enum name="SnippetOpenMethod">
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