Commit 7f3150d6 authored by Jian Li's avatar Jian Li Committed by Commit Bot

[Feed v2] Java side of clear all feed data

Bug: none
Change-Id: I03cd5f3d360907d4bf5f8a733e95950e6a08e08d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324616Reviewed-by: default avatarDan H <harringtond@chromium.org>
Commit-Queue: Jian Li <jianli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792939}
parent e51a6fcf
...@@ -34,6 +34,11 @@ public final class FeedServiceBridge { ...@@ -34,6 +34,11 @@ public final class FeedServiceBridge {
return result; return result;
} }
@CalledByNative
public static void clearAll() {
FeedStreamSurface.clearAll();
}
/** Called at startup to trigger creation of |FeedService|. */ /** Called at startup to trigger creation of |FeedService|. */
public static void startup() { public static void startup() {
FeedServiceBridgeJni.get().startup(); FeedServiceBridgeJni.get().startup();
......
...@@ -121,31 +121,59 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand ...@@ -121,31 +121,59 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
// the correct sign-in state is used if attaching the surface triggers a fetch. // the correct sign-in state is used if attaching the surface triggers a fetch.
private static boolean sStartupCalled; private static boolean sStartupCalled;
private static HashSet<FeedStreamSurface> sWaitingSurfaces; // Tracks all the surfaces that are waiting to be attached or already attached. When
// |sStartupCalled| is false, |startup()| has not been called and thus all the surfaces
// in this set are waiting to be attached. Otherwise, all the surfaces in this set are
// already attached.
private static HashSet<FeedStreamSurface> sSurfaces;
public static void startup() { public static void startup() {
if (sStartupCalled) return; if (sStartupCalled) return;
sStartupCalled = true; sStartupCalled = true;
FeedServiceBridge.startup(); FeedServiceBridge.startup();
xSurfaceProcessScope(); xSurfaceProcessScope();
if (sWaitingSurfaces != null) { if (sSurfaces != null) {
for (FeedStreamSurface surface : sWaitingSurfaces) { for (FeedStreamSurface surface : sSurfaces) {
surface.surfaceOpened(); surface.surfaceOpened();
} }
sWaitingSurfaces = null;
} }
} }
private static void openSurfaceAtStartup(FeedStreamSurface surface) { // Only called for cleanup during testing.
if (sWaitingSurfaces == null) { @VisibleForTesting
sWaitingSurfaces = new HashSet<FeedStreamSurface>(); static void shutdownForTesting() {
sStartupCalled = false;
sSurfaces = null;
sXSurfaceProcessScope = null;
}
private static void trackSurface(FeedStreamSurface surface) {
if (sSurfaces == null) {
sSurfaces = new HashSet<FeedStreamSurface>();
} }
sWaitingSurfaces.add(surface); sSurfaces.add(surface);
} }
private static void cancelOpenSurfaceAtStartup(FeedStreamSurface surface) { private static void untrackSurface(FeedStreamSurface surface) {
if (sWaitingSurfaces != null) { if (sSurfaces != null) {
sWaitingSurfaces.remove(surface); sSurfaces.remove(surface);
}
}
/**
* Clear all the data related to all surfaces.
*/
public static void clearAll() {
if (sSurfaces != null) {
for (FeedStreamSurface surface : sSurfaces) {
surface.surfaceClosed();
}
sSurfaces = null;
}
ProcessScope processScope = xSurfaceProcessScope();
if (processScope != null) {
processScope.resetAccount();
} }
} }
...@@ -645,9 +673,8 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand ...@@ -645,9 +673,8 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
* the content is available, onStreamUpdated will be called. * the content is available, onStreamUpdated will be called.
*/ */
public void surfaceOpened() { public void surfaceOpened() {
if (!sStartupCalled) { trackSurface(this);
openSurfaceAtStartup(this); if (sStartupCalled) {
} else {
FeedStreamSurfaceJni.get().surfaceOpened( FeedStreamSurfaceJni.get().surfaceOpened(
mNativeFeedStreamSurface, FeedStreamSurface.this); mNativeFeedStreamSurface, FeedStreamSurface.this);
} }
...@@ -662,9 +689,8 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand ...@@ -662,9 +689,8 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
mContentManager.removeContents(mHeaderCount, feedCount); mContentManager.removeContents(mHeaderCount, feedCount);
} }
if (!sStartupCalled) { untrackSurface(this);
cancelOpenSurfaceAtStartup(this); if (sStartupCalled) {
} else {
FeedStreamSurfaceJni.get().surfaceClosed( FeedStreamSurfaceJni.get().surfaceClosed(
mNativeFeedStreamSurface, FeedStreamSurface.this); mNativeFeedStreamSurface, FeedStreamSurface.this);
} }
......
...@@ -29,6 +29,7 @@ import androidx.recyclerview.widget.RecyclerView; ...@@ -29,6 +29,7 @@ import androidx.recyclerview.widget.RecyclerView;
import com.google.protobuf.ByteString; import com.google.protobuf.ByteString;
import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
...@@ -45,12 +46,15 @@ import org.robolectric.annotation.Config; ...@@ -45,12 +46,15 @@ import org.robolectric.annotation.Config;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.JniMocker; import org.chromium.base.test.util.JniMocker;
import org.chromium.chrome.browser.AppHooks;
import org.chromium.chrome.browser.AppHooksImpl;
import org.chromium.chrome.browser.help.HelpAndFeedback; import org.chromium.chrome.browser.help.HelpAndFeedback;
import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate; import org.chromium.chrome.browser.native_page.NativePageNavigationDelegate;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.MockTab; import org.chromium.chrome.browser.tab.MockTab;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager; import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.xsurface.FeedActionsHandler; import org.chromium.chrome.browser.xsurface.FeedActionsHandler;
import org.chromium.chrome.browser.xsurface.ProcessScope;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController; import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.feed.proto.FeedUiProto.Slice; import org.chromium.components.feed.proto.FeedUiProto.Slice;
...@@ -63,7 +67,7 @@ import java.util.Arrays; ...@@ -63,7 +67,7 @@ import java.util.Arrays;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
/** Unit tests for {@link FeedStreamSurface}. */ /** Unit tests for {@link FeedStreamSeSurface}. */
@RunWith(BaseRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE) @Config(manifest = Config.NONE)
public class FeedStreamSurfaceTest { public class FeedStreamSurfaceTest {
...@@ -105,6 +109,11 @@ public class FeedStreamSurfaceTest { ...@@ -105,6 +109,11 @@ public class FeedStreamSurfaceTest {
@Mock @Mock
private FeedStreamSurface.Natives mFeedStreamSurfaceJniMock; private FeedStreamSurface.Natives mFeedStreamSurfaceJniMock;
@Mock
private AppHooksImpl mAppHooks;
@Mock
private ProcessScope mProcessScope;
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
...@@ -116,6 +125,10 @@ public class FeedStreamSurfaceTest { ...@@ -116,6 +125,10 @@ public class FeedStreamSurfaceTest {
when(mFeedServiceBridgeJniMock.getLoadMoreTriggerLookahead()) when(mFeedServiceBridgeJniMock.getLoadMoreTriggerLookahead())
.thenReturn(LOAD_MORE_TRIGGER_LOOKAHEAD); .thenReturn(LOAD_MORE_TRIGGER_LOOKAHEAD);
when(mAppHooks.getExternalSurfaceProcessScope(any())).thenReturn(mProcessScope);
AppHooks.setInstanceForTesting(mAppHooks);
Profile.setLastUsedProfileForTesting(mProfileMock); Profile.setLastUsedProfileForTesting(mProfileMock);
mFeedStreamSurface = new FeedStreamSurface(mActivity, false, mSnackbarManager, mFeedStreamSurface = new FeedStreamSurface(mActivity, false, mSnackbarManager,
mPageNavigationDelegate, mBottomSheetController, mHelpAndFeedback); mPageNavigationDelegate, mBottomSheetController, mHelpAndFeedback);
...@@ -126,6 +139,12 @@ public class FeedStreamSurfaceTest { ...@@ -126,6 +139,12 @@ public class FeedStreamSurfaceTest {
mRecyclerView.setLayoutManager(mLayoutManager); mRecyclerView.setLayoutManager(mLayoutManager);
} }
@After
public void tearDown() {
FeedStreamSurface.shutdownForTesting();
AppHooks.setInstanceForTesting(null);
}
@Test @Test
@SmallTest @SmallTest
public void testAddSlicesOnStreamUpdated() { public void testAddSlicesOnStreamUpdated() {
...@@ -453,10 +472,7 @@ public class FeedStreamSurfaceTest { ...@@ -453,10 +472,7 @@ public class FeedStreamSurfaceTest {
@Test @Test
@SmallTest @SmallTest
public void testSurfaceClosed() { public void testRemoveContentsOnSurfaceClosed() {
FeedListContentManager mContentManager =
mFeedStreamSurface.getFeedListContentManagerForTesting();
// Set 2 header views first. // Set 2 header views first.
View v0 = new View(mActivity); View v0 = new View(mActivity);
View v1 = new View(mActivity); View v1 = new View(mActivity);
...@@ -542,6 +558,42 @@ public class FeedStreamSurfaceTest { ...@@ -542,6 +558,42 @@ public class FeedStreamSurfaceTest {
.loadMore(anyLong(), any(FeedStreamSurface.class), any(Callback.class)); .loadMore(anyLong(), any(FeedStreamSurface.class), any(Callback.class));
} }
@Test
@SmallTest
public void testSurfaceOpenedAndClosed() {
// Calling surfaceOpened() before startup() should not trigger native open call.
mFeedStreamSurface.surfaceOpened();
verify(mFeedStreamSurfaceJniMock, never())
.surfaceOpened(anyLong(), any(FeedStreamSurface.class));
// Calling surfaceClosed() before startup() should not trigger native open call.
mFeedStreamSurface.surfaceClosed();
verify(mFeedStreamSurfaceJniMock, never())
.surfaceClosed(anyLong(), any(FeedStreamSurface.class));
FeedStreamSurface.startup();
// Calling surfaceOpened() after startup() should trigger native open call.
mFeedStreamSurface.surfaceOpened();
verify(mFeedStreamSurfaceJniMock).surfaceOpened(anyLong(), any(FeedStreamSurface.class));
// Calling surfaceClosed() after startup() should trigger native open call.
mFeedStreamSurface.surfaceClosed();
verify(mFeedStreamSurfaceJniMock).surfaceClosed(anyLong(), any(FeedStreamSurface.class));
}
@Test
@SmallTest
public void testClearAll() {
FeedStreamSurface.startup();
mFeedStreamSurface.surfaceOpened();
verify(mFeedStreamSurfaceJniMock).surfaceOpened(anyLong(), any(FeedStreamSurface.class));
FeedStreamSurface.clearAll();
verify(mFeedStreamSurfaceJniMock).surfaceClosed(anyLong(), any(FeedStreamSurface.class));
verify(mProcessScope).resetAccount();
}
private SliceUpdate createSliceUpdateForExistingSlice(String sliceId) { private SliceUpdate createSliceUpdateForExistingSlice(String sliceId) {
return SliceUpdate.newBuilder().setSliceId(sliceId).build(); return SliceUpdate.newBuilder().setSliceId(sliceId).build();
} }
......
...@@ -56,6 +56,11 @@ DisplayMetrics FeedServiceBridge::GetDisplayMetrics() { ...@@ -56,6 +56,11 @@ DisplayMetrics FeedServiceBridge::GetDisplayMetrics() {
return result; return result;
} }
void FeedServiceBridge::ClearAll() {
JNIEnv* env = base::android::AttachCurrentThread();
Java_FeedServiceBridge_clearAll(env);
}
bool FeedServiceBridge::IsEnabled() { bool FeedServiceBridge::IsEnabled() {
Profile* profile = ProfileManager::GetLastUsedProfile(); Profile* profile = ProfileManager::GetLastUsedProfile();
return FeedService::IsEnabled(*profile->GetPrefs()); return FeedService::IsEnabled(*profile->GetPrefs());
......
...@@ -16,6 +16,7 @@ class FeedServiceBridge { ...@@ -16,6 +16,7 @@ class FeedServiceBridge {
public: public:
static std::string GetLanguageTag(); static std::string GetLanguageTag();
static DisplayMetrics GetDisplayMetrics(); static DisplayMetrics GetDisplayMetrics();
static void ClearAll();
static bool IsEnabled(); static bool IsEnabled();
}; };
......
...@@ -44,6 +44,7 @@ class FeedServiceDelegateImpl : public FeedService::Delegate { ...@@ -44,6 +44,7 @@ class FeedServiceDelegateImpl : public FeedService::Delegate {
DisplayMetrics GetDisplayMetrics() override { DisplayMetrics GetDisplayMetrics() override {
return FeedServiceBridge::GetDisplayMetrics(); return FeedServiceBridge::GetDisplayMetrics();
} }
void ClearAll() override { FeedServiceBridge::ClearAll(); }
}; };
} // namespace } // namespace
......
...@@ -536,6 +536,8 @@ void FeedStream::BackgroundRefreshComplete(LoadStreamTask::Result result) { ...@@ -536,6 +536,8 @@ void FeedStream::BackgroundRefreshComplete(LoadStreamTask::Result result) {
} }
void FeedStream::ClearAll() { void FeedStream::ClearAll() {
delegate_->ClearAll();
metrics_reporter_->OnClearAll(clock_->Now() - GetLastFetchTime()); metrics_reporter_->OnClearAll(clock_->Now() - GetLastFetchTime());
task_queue_.AddTask(std::make_unique<ClearAllTask>(this)); task_queue_.AddTask(std::make_unique<ClearAllTask>(this));
......
...@@ -65,6 +65,7 @@ class FeedStream : public FeedStreamApi, ...@@ -65,6 +65,7 @@ class FeedStream : public FeedStreamApi,
virtual bool IsOffline() = 0; virtual bool IsOffline() = 0;
virtual DisplayMetrics GetDisplayMetrics() = 0; virtual DisplayMetrics GetDisplayMetrics() = 0;
virtual std::string GetLanguageTag() = 0; virtual std::string GetLanguageTag() = 0;
virtual void ClearAll() = 0;
}; };
// Forwards to |feed::TranslateWireResponse()| by default. Can be overridden // Forwards to |feed::TranslateWireResponse()| by default. Can be overridden
......
...@@ -539,6 +539,7 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate { ...@@ -539,6 +539,7 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate {
return result; return result;
} }
std::string GetLanguageTag() override { return "en-US"; } std::string GetLanguageTag() override { return "en-US"; }
void ClearAll() override {}
// For tests. // For tests.
......
...@@ -116,6 +116,7 @@ class FeedService::StreamDelegateImpl : public FeedStream::Delegate { ...@@ -116,6 +116,7 @@ class FeedService::StreamDelegateImpl : public FeedStream::Delegate {
std::string GetLanguageTag() override { std::string GetLanguageTag() override {
return service_delegate_->GetLanguageTag(); return service_delegate_->GetLanguageTag();
} }
void ClearAll() override { service_delegate_->ClearAll(); }
private: private:
FeedService::Delegate* service_delegate_; FeedService::Delegate* service_delegate_;
......
...@@ -63,6 +63,8 @@ class FeedService : public KeyedService { ...@@ -63,6 +63,8 @@ class FeedService : public KeyedService {
virtual std::string GetLanguageTag() = 0; virtual std::string GetLanguageTag() = 0;
// Returns display metrics for the device. // Returns display metrics for the device.
virtual DisplayMetrics GetDisplayMetrics() = 0; virtual DisplayMetrics GetDisplayMetrics() = 0;
// Clear all stored data.
virtual void ClearAll() = 0;
}; };
// Construct a FeedService given an already constructed FeedStream. // Construct a FeedService given an already constructed FeedStream.
......
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