Skip to content

Commit

Permalink
Fix cached visible url logic for start surface
Browse files Browse the repository at this point in the history
The previous logic failed to notify when the logical url changed as a
result of the start surface state changing, which caused it to use the
most recent tab's url rather than the hardcoded start surface url.

(cherry picked from commit 1f9279e)

Bug: 1396319, 1394962
Change-Id: Ib56f15e741ad8b29a44df1f003a38724b7f4e3fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4081091
Reviewed-by: Xi Han <hanxi@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1079903}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4086080
Reviewed-by: Krishna Govind <govind@chromium.org>
Owners-Override: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/5359@{#1123}
Cr-Branched-From: 27d3765-refs/heads/main@{#1058933}
  • Loading branch information
Patrick Noland authored and Chromium LUCI CQ committed Dec 8, 2022
1 parent 1dbde24 commit d4753ca
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
Expand Up @@ -39,6 +39,7 @@
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.incognito.IncognitoCctProfileManager;
import org.chromium.chrome.browser.incognito.IncognitoUtils;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.omnibox.ChromeAutocompleteSchemeClassifier;
import org.chromium.chrome.browser.omnibox.ChromeAutocompleteSchemeClassifierJni;
import org.chromium.chrome.browser.omnibox.LocationBarDataProvider;
Expand All @@ -48,10 +49,12 @@
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TrustedCdn;
import org.chromium.chrome.browser.toolbar.LocationBarModelUnitTest.ShadowTrustedCdn;
import org.chromium.chrome.features.start_surface.StartSurfaceState;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.components.dom_distiller.core.DomDistillerUrlUtilsJni;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.components.omnibox.OmniboxUrlEmphasizerJni;
import org.chromium.components.url_formatter.UrlFormatter;
import org.chromium.components.url_formatter.UrlFormatterJni;
Expand Down Expand Up @@ -116,6 +119,8 @@ public static String getPublisherUrl(@Nullable Tab tab) {
private OmniboxUrlEmphasizerJni mOmniboxUrlEmphasizerJni;
@Mock
private GURL mMockGurl;
@Mock
private LayoutStateProvider mLayoutStateProvider;

@Before
public void setUp() {
Expand Down Expand Up @@ -334,4 +339,38 @@ public void testSpannableCache() {

regularLocationBarModel.destroy();
}

@EnableFeatures({ChromeFeatureList.ANDROID_SCROLL_OPTIMIZATIONS})
@Test
@MediumTest
public void testUpdateVisibleGurlStartSurfaceShowing() {
doReturn(123L).when(mLocationBarModelJni).init(Mockito.any());
mJniMocker.mock(ChromeAutocompleteSchemeClassifierJni.TEST_HOOKS,
mChromeAutocompleteSchemeClassifierJni);
LocationBarModel regularLocationBarModel =
new TestRegularLocationBarModel(mRegularTabMock, mSearchEngineLogoUtils);
doReturn(true).when(mRegularTabMock).isInitialized();
regularLocationBarModel.initializeWithNative();
regularLocationBarModel.setShouldShowOmniboxInOverviewMode(true);
regularLocationBarModel.setLayoutStateProvider(mLayoutStateProvider);
regularLocationBarModel.addObserver(mLocationBarDataObserver);
doReturn(mMockGurl)
.when(mLocationBarModelJni)
.getUrlOfVisibleNavigationEntry(Mockito.anyLong(), Mockito.any());
Assert.assertNotEquals(regularLocationBarModel.getCurrentGurl(), UrlConstants.ntpGurl());

regularLocationBarModel.setIsShowingTabSwitcher(true);
verify(mLocationBarDataObserver).onUrlChanged();
regularLocationBarModel.setStartSurfaceState(StartSurfaceState.SHOWN_HOMEPAGE);

verify(mLocationBarDataObserver, times(2)).onUrlChanged();
Assert.assertEquals(regularLocationBarModel.getCurrentGurl(), UrlConstants.ntpGurl());

regularLocationBarModel.setStartSurfaceState(StartSurfaceState.NOT_SHOWN);

verify(mLocationBarDataObserver, times(3)).onUrlChanged();
Assert.assertEquals(regularLocationBarModel.getCurrentGurl(), mMockGurl);

regularLocationBarModel.destroy();
}
}
Expand Up @@ -294,14 +294,14 @@ public String getCurrentUrl() {

@Override
public GURL getCurrentGurl() {
if (mOptimizationsEnabled) {
return mVisibleGurl;
}

if (isInOverviewAndShowingOmnibox()) {
return UrlConstants.ntpGurl();
}

if (mOptimizationsEnabled) {
return mVisibleGurl;
}

Tab tab = getTab();
return tab != null && tab.isInitialized() ? tab.getUrl() : GURL.emptyGURL();
}
Expand All @@ -314,6 +314,7 @@ void updateVisibleGurl() {
mFormattedFullUrl = "";
mUrlForDisplay = "";
mVisibleGurl = UrlConstants.ntpGurl();
return;
}

GURL gurl = getUrlOfVisibleNavigationEntry();
Expand Down Expand Up @@ -815,6 +816,7 @@ public void setIsShowingTabSwitcher(boolean isShowingTabSwitcher) {
*/
public void setStartSurfaceState(@StartSurfaceState int startSurfaceState) {
mStartSurfaceState = startSurfaceState;
notifyUrlChanged();
}

@NativeMethods
Expand Down

0 comments on commit d4753ca

Please sign in to comment.