Commit eb8227aa authored by Becky Zhou's avatar Becky Zhou Committed by Commit Bot

[Feed] Update enterprise policy UI code to match infra behavior

Once FeedProcessScope has been torn down because of enterprise policy
within one session, we will not provide Feed on the NTP until the next
restart when enterprise policy is removed within this session.

Bug: 887743
Change-Id: If08e94bb23224e8c0cd2c3e361edef6ed4e0a287
Reviewed-on: https://chromium-review.googlesource.com/1239364
Commit-Queue: Becky Zhou <huayinz@chromium.org>
Reviewed-by: default avatarSky Malice <skym@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593768}
parent 81ac7266
...@@ -35,8 +35,6 @@ import org.chromium.chrome.browser.signin.SigninPromoUtil; ...@@ -35,8 +35,6 @@ import org.chromium.chrome.browser.signin.SigninPromoUtil;
*/ */
class FeedNewTabPageMediator class FeedNewTabPageMediator
implements NewTabPageLayout.ScrollDelegate, ContextMenuManager.TouchEnabledDelegate { implements NewTabPageLayout.ScrollDelegate, ContextMenuManager.TouchEnabledDelegate {
private static boolean sOverrideFeedEnabledForTesting;
private final FeedNewTabPage mCoordinator; private final FeedNewTabPage mCoordinator;
private final SnapScrollHelper mSnapScrollHelper; private final SnapScrollHelper mSnapScrollHelper;
private final PrefChangeRegistrar mPrefChangeRegistrar; private final PrefChangeRegistrar mPrefChangeRegistrar;
...@@ -87,13 +85,7 @@ class FeedNewTabPageMediator ...@@ -87,13 +85,7 @@ class FeedNewTabPageMediator
/** Update the content based on supervised user or enterprise policy. */ /** Update the content based on supervised user or enterprise policy. */
private void updateContent() { private void updateContent() {
// TODO(huayinz): Replace sOverrideFeedEnabledForTesting with the real preference in test mFeedEnabled = FeedProcessScopeFactory.isFeedProcessEnabled();
// once we have a decision for whether to re-enable the Feed on supervised/enterprise
// account removed within a session.
if (!sOverrideFeedEnabledForTesting) {
mFeedEnabled =
PrefServiceBridge.getInstance().getBoolean(Pref.NTP_ARTICLES_SECTION_ENABLED);
}
if ((mFeedEnabled && mCoordinator.getStream() != null) if ((mFeedEnabled && mCoordinator.getStream() != null)
|| (!mFeedEnabled && mCoordinator.getScrollViewForPolicy() != null)) || (!mFeedEnabled && mCoordinator.getScrollViewForPolicy() != null))
return; return;
...@@ -328,17 +320,6 @@ class FeedNewTabPageMediator ...@@ -328,17 +320,6 @@ class FeedNewTabPageMediator
} }
} }
@VisibleForTesting
void overrideFeedEnabledForTesting(boolean override) {
sOverrideFeedEnabledForTesting = override;
}
@VisibleForTesting
void updateContentForTesting(boolean feedEnabled) {
mFeedEnabled = feedEnabled;
updateContent();
}
// TODO(huayinz): Return the Model for testing in Coordinator instead once a Model is created. // TODO(huayinz): Return the Model for testing in Coordinator instead once a Model is created.
@VisibleForTesting @VisibleForTesting
SectionHeader getSectionHeaderForTesting() { SectionHeader getSectionHeaderForTesting() {
......
...@@ -26,6 +26,8 @@ import java.util.concurrent.Executors; ...@@ -26,6 +26,8 @@ import java.util.concurrent.Executors;
/** Holds singleton {@link FeedProcessScope} and some of the scope's host implementations. */ /** Holds singleton {@link FeedProcessScope} and some of the scope's host implementations. */
public class FeedProcessScopeFactory { public class FeedProcessScopeFactory {
private static boolean sIsDisableForPolicy =
!PrefServiceBridge.getInstance().getBoolean(Pref.NTP_ARTICLES_SECTION_ENABLED);
private static PrefChangeRegistrar sPrefChangeRegistrar; private static PrefChangeRegistrar sPrefChangeRegistrar;
private static FeedProcessScope sFeedProcessScope; private static FeedProcessScope sFeedProcessScope;
private static FeedScheduler sFeedScheduler; private static FeedScheduler sFeedScheduler;
...@@ -58,11 +60,19 @@ public class FeedProcessScopeFactory { ...@@ -58,11 +60,19 @@ public class FeedProcessScopeFactory {
return sFeedOfflineIndicator; return sFeedOfflineIndicator;
} }
/**
* @return Whether the dependencies provided by this class are allowed to be created. The feed
* process is disabled if supervised user or enterprise policy has once been added
* within the current session.
*/
public static boolean isFeedProcessEnabled() {
return !sIsDisableForPolicy
&& PrefServiceBridge.getInstance().getBoolean(Pref.NTP_ARTICLES_SECTION_ENABLED);
}
private static void initialize() { private static void initialize() {
assert sFeedProcessScope == null && sFeedScheduler == null && sFeedOfflineIndicator == null; assert sFeedProcessScope == null && sFeedScheduler == null && sFeedOfflineIndicator == null;
if (!PrefServiceBridge.getInstance().getBoolean(Pref.NTP_ARTICLES_SECTION_ENABLED)) { if (!isFeedProcessEnabled()) return;
return;
}
sPrefChangeRegistrar = new PrefChangeRegistrar(); sPrefChangeRegistrar = new PrefChangeRegistrar();
sPrefChangeRegistrar.addObserver(Pref.NTP_ARTICLES_SECTION_ENABLED, sPrefChangeRegistrar.addObserver(Pref.NTP_ARTICLES_SECTION_ENABLED,
...@@ -151,6 +161,7 @@ public class FeedProcessScopeFactory { ...@@ -151,6 +161,7 @@ public class FeedProcessScopeFactory {
// Should only be subscribed while it was enabled. A change should mean articles are now // Should only be subscribed while it was enabled. A change should mean articles are now
// disabled. // disabled.
assert !PrefServiceBridge.getInstance().getBoolean(Pref.NTP_ARTICLES_SECTION_ENABLED); assert !PrefServiceBridge.getInstance().getBoolean(Pref.NTP_ARTICLES_SECTION_ENABLED);
sIsDisableForPolicy = true;
destroy(); destroy();
} }
......
...@@ -77,7 +77,7 @@ public class FeedNewTabPageTest { ...@@ -77,7 +77,7 @@ public class FeedNewTabPageTest {
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
mActivityTestRule.startMainActivityWithURL("about:blank"); mActivityTestRule.startMainActivityWithURL("about:blank");
FeedNewTabPage.setInTestMode(true); ThreadUtils.runOnUiThreadBlocking(() -> FeedNewTabPage.setInTestMode(true));
mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext()); mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
mSiteSuggestions = NewTabPageTestUtils.createFakeSiteSuggestions(mTestServer); mSiteSuggestions = NewTabPageTestUtils.createFakeSiteSuggestions(mTestServer);
...@@ -198,11 +198,9 @@ public class FeedNewTabPageTest { ...@@ -198,11 +198,9 @@ public class FeedNewTabPageTest {
@MediumTest @MediumTest
@Feature({"FeedNewTabPage"}) @Feature({"FeedNewTabPage"})
public void testFeedDisabledByPolicy() throws Exception { public void testFeedDisabledByPolicy() throws Exception {
// TODO(huayinz): Re-enable the part commented out on this test and replace final boolean pref = ThreadUtils.runOnUiThreadBlocking(
// sOverrideFeedEnabledForTesting with the real preference once we have a decision for () -> PrefServiceBridge.getInstance().getBoolean(
// whether to re-enable the Feed on supervised/enterprise account removed within a session. Pref.NTP_ARTICLES_SECTION_ENABLED));
FeedNewTabPageMediator mediator = mNtp.getMediatorForTesting();
mediator.overrideFeedEnabledForTesting(true);
// Policy is disabled. Verify the NTP root view contains only the Stream view as child. // Policy is disabled. Verify the NTP root view contains only the Stream view as child.
ViewGroup rootView = (ViewGroup) mNtp.getView(); ViewGroup rootView = (ViewGroup) mNtp.getView();
...@@ -214,7 +212,8 @@ public class FeedNewTabPageTest { ...@@ -214,7 +212,8 @@ public class FeedNewTabPageTest {
// Simulate that policy is enabled. Verify the NTP root view contains only the view for // Simulate that policy is enabled. Verify the NTP root view contains only the view for
// policy as child. // policy as child.
ThreadUtils.runOnUiThreadBlocking(() -> mediator.updateContentForTesting(false)); ThreadUtils.runOnUiThreadBlocking(() -> PrefServiceBridge.getInstance().setBoolean(
Pref.NTP_ARTICLES_SECTION_ENABLED, false));
ViewUtils.waitForStableView(rootView); ViewUtils.waitForStableView(rootView);
Assert.assertNotNull(mNtp.getScrollViewForPolicy()); Assert.assertNotNull(mNtp.getScrollViewForPolicy());
Assert.assertNull(mNtp.getStream()); Assert.assertNull(mNtp.getStream());
...@@ -233,28 +232,27 @@ public class FeedNewTabPageTest { ...@@ -233,28 +232,27 @@ public class FeedNewTabPageTest {
Assert.assertEquals(1, rootView2.getChildCount()); Assert.assertEquals(1, rootView2.getChildCount());
Assert.assertEquals(ntp2.getScrollViewForPolicy(), rootView2.getChildAt(0)); Assert.assertEquals(ntp2.getScrollViewForPolicy(), rootView2.getChildAt(0));
/* // Simulate that policy is disabled. Verify the NTP root view is the view for policy. We
// Simulate that policy is disabled. Verify the NTP root view contains only the Stream view // don't re-enable the Feed until the next restart.
// as child.
ThreadUtils.runOnUiThreadBlocking(() -> PrefServiceBridge.getInstance().setBoolean( ThreadUtils.runOnUiThreadBlocking(() -> PrefServiceBridge.getInstance().setBoolean(
Pref.NTP_ARTICLES_SECTION_ENABLED, true)); Pref.NTP_ARTICLES_SECTION_ENABLED, true));
ViewUtils.waitForStableView(rootView2); ViewUtils.waitForStableView(rootView2);
Assert.assertNotNull(ntp2.getStream()); Assert.assertNotNull(ntp2.getScrollViewForPolicy());
Assert.assertNull(ntp2.getScrollViewForPolicy()); Assert.assertNull(ntp2.getStream());
Assert.assertEquals(1, rootView2.getChildCount()); Assert.assertEquals(1, rootView2.getChildCount());
Assert.assertEquals(ntp2.getStream().getView(), rootView2.getChildAt(0)); Assert.assertEquals(ntp2.getScrollViewForPolicy(), rootView2.getChildAt(0));
// Switch to the old tab. Verify the NTP root view contains only the Stream view as child. // Switch to the old tab. Verify the NTP root view is the view for policy.
ChromeTabUtils.switchTabInCurrentTabModel(mActivityTestRule.getActivity(), mTab.getId()); ChromeTabUtils.switchTabInCurrentTabModel(mActivityTestRule.getActivity(), mTab.getId());
ViewUtils.waitForStableView(rootView); ViewUtils.waitForStableView(rootView);
Assert.assertNotNull(mNtp.getStream()); Assert.assertNotNull(mNtp.getScrollViewForPolicy());
Assert.assertNull(mNtp.getScrollViewForPolicy()); Assert.assertNull(mNtp.getStream());
Assert.assertEquals(1, rootView.getChildCount()); Assert.assertEquals(1, rootView.getChildCount());
Assert.assertEquals(mNtp.getStream().getView(), rootView.getChildAt(0)); Assert.assertEquals(mNtp.getScrollViewForPolicy(), rootView.getChildAt(0));
*/
// Reset state. // Reset state.
mediator.overrideFeedEnabledForTesting(false); ThreadUtils.runOnUiThreadBlocking(() -> PrefServiceBridge.getInstance().setBoolean(
Pref.NTP_ARTICLES_SECTION_ENABLED, pref));
} }
private boolean getPreferenceForArticleSectionHeader() throws Exception { private boolean getPreferenceForArticleSectionHeader() throws Exception {
......
...@@ -136,7 +136,6 @@ public class NewTabPageTest { ...@@ -136,7 +136,6 @@ public class NewTabPageTest {
mInterestFeedEnabled = interestFeedEnabled; mInterestFeedEnabled = interestFeedEnabled;
if (mInterestFeedEnabled) { if (mInterestFeedEnabled) {
Features.getInstance().enable(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS); Features.getInstance().enable(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS);
FeedNewTabPage.setInTestMode(true);
} else { } else {
Features.getInstance().disable(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS); Features.getInstance().disable(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS);
} }
...@@ -144,6 +143,11 @@ public class NewTabPageTest { ...@@ -144,6 +143,11 @@ public class NewTabPageTest {
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
mActivityTestRule.startMainActivityWithURL("about:blank");
if (mInterestFeedEnabled) {
ThreadUtils.runOnUiThreadBlocking(() -> FeedNewTabPage.setInTestMode(true));
}
mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext()); mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
mSiteSuggestions = NewTabPageTestUtils.createFakeSiteSuggestions(mTestServer); mSiteSuggestions = NewTabPageTestUtils.createFakeSiteSuggestions(mTestServer);
...@@ -151,7 +155,7 @@ public class NewTabPageTest { ...@@ -151,7 +155,7 @@ public class NewTabPageTest {
mMostVisitedSites.setTileSuggestions(mSiteSuggestions); mMostVisitedSites.setTileSuggestions(mSiteSuggestions);
mSuggestionsDeps.getFactory().mostVisitedSites = mMostVisitedSites; mSuggestionsDeps.getFactory().mostVisitedSites = mMostVisitedSites;
mActivityTestRule.startMainActivityWithURL(UrlConstants.NTP_URL); mActivityTestRule.loadUrl(UrlConstants.NTP_URL);
mTab = mActivityTestRule.getActivity().getActivityTab(); mTab = mActivityTestRule.getActivity().getActivityTab();
NewTabPageTestUtils.waitForNtpLoaded(mTab); NewTabPageTestUtils.waitForNtpLoaded(mTab);
...@@ -165,7 +169,9 @@ public class NewTabPageTest { ...@@ -165,7 +169,9 @@ public class NewTabPageTest {
@After @After
public void tearDown() throws Exception { public void tearDown() throws Exception {
mTestServer.stopAndDestroyServer(); mTestServer.stopAndDestroyServer();
if (mInterestFeedEnabled) FeedNewTabPage.setInTestMode(false); if (mInterestFeedEnabled) {
ThreadUtils.runOnUiThreadBlocking(() -> FeedNewTabPage.setInTestMode(false));
}
} }
@Test @Test
......
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