Commit eb51b019 authored by Ted Choc's avatar Ted Choc Committed by Commit Bot

Show the home button immediately on startup to avoid flicker.

Longer term, I wonder if HomepageManager should provide the
ObservableSupplier itself and handle these updates internally without
ToolbarManager (or something else querying).

BUG=

Change-Id: I3ccedd220332fb2ea5bdca936c619934cbfe0420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490491
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: default avatarPatrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820841}
parent fff469c8
...@@ -56,8 +56,11 @@ public class HomeButton extends ChromeImageButton ...@@ -56,8 +56,11 @@ public class HomeButton extends ChromeImageButton
* @param isHomepageManagedByPolicy Supplier that tells if homepage is managed by policy. * @param isHomepageManagedByPolicy Supplier that tells if homepage is managed by policy.
*/ */
public void init(ObservableSupplier<Boolean> homepageVisibility, public void init(ObservableSupplier<Boolean> homepageVisibility,
Callback<Context> onMenuClickCallback, Supplier<Boolean> isHomepageManagedByPolicy) { Callback<Context> onMenuClickCallback,
homepageVisibility.addObserver((visible) -> updateContextMenuListener()); ObservableSupplier<Boolean> isHomepageManagedByPolicy) {
Callback<Boolean> contextUpdateCallback = (visible) -> updateContextMenuListener();
homepageVisibility.addObserver(contextUpdateCallback);
isHomepageManagedByPolicy.addObserver(contextUpdateCallback);
mOnMenuClickCallback = onMenuClickCallback; mOnMenuClickCallback = onMenuClickCallback;
mIsManagedByPolicySupplier = isHomepageManagedByPolicy; mIsManagedByPolicySupplier = isHomepageManagedByPolicy;
updateContextMenuListener(); updateContextMenuListener();
......
...@@ -56,8 +56,8 @@ import org.chromium.chrome.browser.ntp.FakeboxDelegate; ...@@ -56,8 +56,8 @@ import org.chromium.chrome.browser.ntp.FakeboxDelegate;
import org.chromium.chrome.browser.ntp.IncognitoNewTabPage; import org.chromium.chrome.browser.ntp.IncognitoNewTabPage;
import org.chromium.chrome.browser.ntp.NewTabPage; import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.omnibox.LocationBar; import org.chromium.chrome.browser.omnibox.LocationBar;
import org.chromium.chrome.browser.omnibox.OmniboxFocusReason;
import org.chromium.chrome.browser.omnibox.LocationBarCoordinator; import org.chromium.chrome.browser.omnibox.LocationBarCoordinator;
import org.chromium.chrome.browser.omnibox.OmniboxFocusReason;
import org.chromium.chrome.browser.omnibox.UrlFocusChangeListener; import org.chromium.chrome.browser.omnibox.UrlFocusChangeListener;
import org.chromium.chrome.browser.previews.Previews; import org.chromium.chrome.browser.previews.Previews;
import org.chromium.chrome.browser.previews.PreviewsAndroidBridge; import org.chromium.chrome.browser.previews.PreviewsAndroidBridge;
...@@ -132,6 +132,8 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve ...@@ -132,6 +132,8 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
private final FullscreenManager.Observer mFullscreenObserver; private final FullscreenManager.Observer mFullscreenObserver;
private final ObservableSupplierImpl<Boolean> mHomeButtonVisibilitySupplier = private final ObservableSupplierImpl<Boolean> mHomeButtonVisibilitySupplier =
new ObservableSupplierImpl<>(); new ObservableSupplierImpl<>();
private final ObservableSupplierImpl<Boolean> mHomepageManagedByPolicySupplier =
new ObservableSupplierImpl<>();
private final ObservableSupplierImpl<Boolean> mIdentityDiscStateSupplier = private final ObservableSupplierImpl<Boolean> mIdentityDiscStateSupplier =
new ObservableSupplierImpl<>(); new ObservableSupplierImpl<>();
private final ObservableSupplier<Boolean> mOmniboxFocusStateSupplier; private final ObservableSupplier<Boolean> mOmniboxFocusStateSupplier;
...@@ -675,9 +677,13 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve ...@@ -675,9 +677,13 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
() ()
-> identityDiscController.getForStartSurface(mStartSurfaceState), -> identityDiscController.getForStartSurface(mStartSurfaceState),
startSurfaceSupplier); startSurfaceSupplier);
mHomepageStateListener = mHomepageStateListener = () -> {
() -> mHomeButtonVisibilitySupplier.set(HomepageManager.isHomepageEnabled()); mHomeButtonVisibilitySupplier.set(HomepageManager.isHomepageEnabled());
mHomepageManagedByPolicySupplier.set(HomepagePolicyManager.isHomepageManagedByPolicy());
};
HomepageManager.getInstance().addListener(mHomepageStateListener); HomepageManager.getInstance().addListener(mHomepageStateListener);
mHomepageStateListener.onHomepageStateUpdated();
if (toolbarLayout instanceof ToolbarPhone if (toolbarLayout instanceof ToolbarPhone
&& StartSurfaceConfiguration.isStartSurfaceEnabled()) { && StartSurfaceConfiguration.isStartSurfaceEnabled()) {
identityDiscController.addObserver( identityDiscController.addObserver(
...@@ -686,8 +692,7 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve ...@@ -686,8 +692,7 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
HomeButton homeButton = mActivity.findViewById(R.id.home_button); HomeButton homeButton = mActivity.findViewById(R.id.home_button);
if (homeButton != null) { if (homeButton != null) {
homeButton.init(mHomeButtonVisibilitySupplier, homeButton.init(mHomeButtonVisibilitySupplier,
HomepageManager.getInstance()::onMenuClick, HomepageManager.getInstance()::onMenuClick, mHomepageManagedByPolicySupplier);
HomepagePolicyManager::isHomepageManagedByPolicy);
} }
return toolbar; return toolbar;
} }
......
...@@ -51,7 +51,6 @@ import org.chromium.chrome.R; ...@@ -51,7 +51,6 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior; import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
import org.chromium.chrome.browser.device.DeviceClassManager; import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory; import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.homepage.HomepageManager;
import org.chromium.chrome.browser.ntp.NewTabPage; import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.omnibox.LocationBar; import org.chromium.chrome.browser.omnibox.LocationBar;
import org.chromium.chrome.browser.omnibox.LocationBarCoordinator; import org.chromium.chrome.browser.omnibox.LocationBarCoordinator;
...@@ -477,8 +476,6 @@ public class ToolbarPhone extends ToolbarLayout ...@@ -477,8 +476,6 @@ public class ToolbarPhone extends ToolbarLayout
} }
}); });
onHomeButtonUpdate(HomepageManager.isHomepageEnabled());
updateVisualsForLocationBarState(); updateVisualsForLocationBarState();
} }
......
...@@ -29,7 +29,6 @@ import org.chromium.base.metrics.RecordUserAction; ...@@ -29,7 +29,6 @@ import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.NavigationPopup; import org.chromium.chrome.browser.NavigationPopup;
import org.chromium.chrome.browser.download.DownloadUtils; import org.chromium.chrome.browser.download.DownloadUtils;
import org.chromium.chrome.browser.homepage.HomepageManager;
import org.chromium.chrome.browser.ntp.NewTabPage; import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.omnibox.LocationBar; import org.chromium.chrome.browser.omnibox.LocationBar;
import org.chromium.chrome.browser.omnibox.LocationBarCoordinator; import org.chromium.chrome.browser.omnibox.LocationBarCoordinator;
...@@ -270,9 +269,6 @@ public class ToolbarTablet extends ToolbarLayout ...@@ -270,9 +269,6 @@ public class ToolbarTablet extends ToolbarLayout
return getMenuButtonCoordinator().onEnterKeyPress(); return getMenuButtonCoordinator().onEnterKeyPress();
} }
}); });
if (HomepageManager.isHomepageEnabled()) {
mHomeButton.setVisibility(VISIBLE);
}
mSaveOfflineButton.setOnClickListener(this); mSaveOfflineButton.setOnClickListener(this);
mSaveOfflineButton.setOnLongClickListener(this); mSaveOfflineButton.setOnLongClickListener(this);
......
...@@ -92,7 +92,7 @@ public class HomepagePolicyIntegrationTest { ...@@ -92,7 +92,7 @@ public class HomepagePolicyIntegrationTest {
@After @After
public void tearDown() { public void tearDown() {
mTestServer.stopAndDestroyServer(); if (mTestServer != null) mTestServer.stopAndDestroyServer();
} }
@Test @Test
...@@ -138,22 +138,24 @@ public class HomepagePolicyIntegrationTest { ...@@ -138,22 +138,24 @@ public class HomepagePolicyIntegrationTest {
ChromeTabUtils.getUrlStringOnUiThread( ChromeTabUtils.getUrlStringOnUiThread(
mActivityTestRule.getActivity().getActivityTab())); mActivityTestRule.getActivity().getActivityTab()));
CriteriaHelper.pollUiThread(() -> {
ToolbarManager toolbarManager = mActivityTestRule.getActivity().getToolbarManager();
Criteria.checkThat(toolbarManager, Matchers.notNullValue());
HomeButton homeButton = toolbarManager.getHomeButtonForTesting();
Criteria.checkThat(homeButton, Matchers.notNullValue());
Criteria.checkThat("Home Button should be visible", homeButton.getVisibility(),
Matchers.is(View.VISIBLE));
Criteria.checkThat("Long press for home button should be disabled",
homeButton.isLongClickable(), Matchers.is(false));
});
ChromeTabUtils.waitForTabPageLoaded( ChromeTabUtils.waitForTabPageLoaded(
mActivityTestRule.getActivity().getActivityTab(), TEST_URL, () -> { mActivityTestRule.getActivity().getActivityTab(), TEST_URL, () -> {
ToolbarManager toolbarManager = ToolbarManager toolbarManager =
mActivityTestRule.getActivity().getToolbarManager(); mActivityTestRule.getActivity().getToolbarManager();
if (toolbarManager != null) { HomeButton homeButton = toolbarManager.getHomeButtonForTesting();
HomeButton homeButton = toolbarManager.getHomeButtonForTesting(); TouchCommon.singleClickView(homeButton);
if (homeButton != null) {
Assert.assertEquals("Home Button should be visible", View.VISIBLE,
homeButton.getVisibility());
// Context menu is disabled by checking long clickable
Assert.assertFalse("Long press for home button should be disabled",
homeButton.isLongClickable());
TouchCommon.singleClickView(homeButton);
}
}
}); });
Assert.assertEquals("After clicking HomeButton, URL should be back to Homepage", TEST_URL, Assert.assertEquals("After clicking HomeButton, URL should be back to Homepage", TEST_URL,
......
...@@ -30,7 +30,6 @@ import org.chromium.base.supplier.ObservableSupplierImpl; ...@@ -30,7 +30,6 @@ import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.homepage.HomepageManager; import org.chromium.chrome.browser.homepage.HomepageManager;
import org.chromium.chrome.browser.homepage.HomepagePolicyManager;
import org.chromium.chrome.browser.homepage.HomepageTestRule; import org.chromium.chrome.browser.homepage.HomepageTestRule;
import org.chromium.chrome.browser.homepage.settings.HomepageSettings; import org.chromium.chrome.browser.homepage.settings.HomepageSettings;
import org.chromium.chrome.browser.settings.SettingsLauncher; import org.chromium.chrome.browser.settings.SettingsLauncher;
...@@ -85,9 +84,10 @@ public class HomeButtonTest extends DummyUiActivityTestCase { ...@@ -85,9 +84,10 @@ public class HomeButtonTest extends DummyUiActivityTestCase {
mIdHomeButton = View.generateViewId(); mIdHomeButton = View.generateViewId();
mHomeButton = new HomeButton(getActivity(), null); mHomeButton = new HomeButton(getActivity(), null);
ObservableSupplierImpl<Boolean> homepagePolicySupplier = new ObservableSupplierImpl<>();
homepagePolicySupplier.set(false);
mHomeButton.init(new ObservableSupplierImpl<Boolean>(), mHomeButton.init(new ObservableSupplierImpl<Boolean>(),
HomepageManager.getInstance()::onMenuClick, HomepageManager.getInstance()::onMenuClick, homepagePolicySupplier);
HomepagePolicyManager::isHomepageManagedByPolicy);
mHomeButton.setId(mIdHomeButton); mHomeButton.setId(mIdHomeButton);
HomepageManager.getInstance().setSettingsLauncherForTesting(mSettingsLauncher); HomepageManager.getInstance().setSettingsLauncherForTesting(mSettingsLauncher);
HomeButton.setSaveContextMenuForTests(true); HomeButton.setSaveContextMenuForTests(true);
......
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