Skip to content

Commit

Permalink
[PartnerBrowserCustomizations] gurl migration
Browse files Browse the repository at this point in the history 
Part of Homepage code gurl migration. Homepage code is dependent on
PartnerBrowserCustomizations.

Bug: 1296595
Change-Id: I5ec7aba951b30c8605426ebc62b6364d188f8963
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3550798
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Peilin Wang <peilinwang@google.com>
Cr-Commit-Position: refs/heads/main@{#1002651}
  • Loading branch information
Peilin Wang authored and Chromium LUCI CQ committed May 12, 2022
1 parent 00b1e1b commit f445cd9
Show file tree
Hide file tree
Showing 8 changed files with 234 additions and 49 deletions.
16 changes: 13 additions & 3 deletions chrome/android/java/src/org/chromium/chrome/browser/homepage/HomepageManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.chromium.components.browser_ui.settings.SettingsLauncher;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.url.GURL;

/**
* Provides information regarding homepage enabled states and URI.
Expand Down Expand Up @@ -147,11 +148,20 @@ public static String getHomepageUri() {
*/
public static String getDefaultHomepageUri() {
if (PartnerBrowserCustomizations.getInstance().isHomepageProviderAvailableAndEnabled()) {
return PartnerBrowserCustomizations.getInstance().getHomePageUrl();
return PartnerBrowserCustomizations.getInstance().getHomePageUrl().getSpec();
}

String homepagePartnerDefaultUri = SharedPreferencesManager.getInstance().readString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_URI, "");
String homepagePartnerDefaultUri;
String homepagePartnerDefaultGurlSerialized =
SharedPreferencesManager.getInstance().readString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_GURL, "");
if (!homepagePartnerDefaultGurlSerialized.equals("")) {
homepagePartnerDefaultUri =
GURL.deserialize(homepagePartnerDefaultGurlSerialized).getSpec();
} else {
homepagePartnerDefaultUri = SharedPreferencesManager.getInstance().readString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_URI, "");
}
if (!homepagePartnerDefaultUri.equals("")) return homepagePartnerDefaultUri;

return UrlConstants.NTP_NON_NATIVE_URL;
Expand Down
8 changes: 4 additions & 4 deletions ...atests/src/org/chromium/chrome/browser/partnercustomizations/PartnerHomepageUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public void testHomepageDisabled() throws InterruptedException {
Assert.assertTrue(mPartnerBrowserCustomizations.isInitialized());
Assert.assertTrue(mPartnerBrowserCustomizations.isHomepageProviderAvailableAndEnabled());
Assert.assertEquals(TestPartnerBrowserCustomizationsProvider.HOMEPAGE_URI,
mPartnerBrowserCustomizations.getHomePageUrl());
mPartnerBrowserCustomizations.getHomePageUrl().getSpec());
Assert.assertFalse(HomepageManager.isHomepageEnabled());
Assert.assertNull(HomepageManager.getHomepageUri());
}
Expand Down Expand Up @@ -200,7 +200,7 @@ public void testCustomHomepage() throws InterruptedException {
Assert.assertTrue(mPartnerBrowserCustomizations.isInitialized());
Assert.assertTrue(mPartnerBrowserCustomizations.isHomepageProviderAvailableAndEnabled());
Assert.assertEquals(TestPartnerBrowserCustomizationsProvider.HOMEPAGE_URI,
mPartnerBrowserCustomizations.getHomePageUrl());
mPartnerBrowserCustomizations.getHomePageUrl().getSpec());
Assert.assertTrue(HomepageManager.isHomepageEnabled());
Assert.assertEquals(TEST_CUSTOM_HOMEPAGE_URI, HomepageManager.getHomepageUri());
}
Expand Down Expand Up @@ -284,7 +284,7 @@ public void testHomepageProviderDelayed() throws InterruptedException {
Assert.assertTrue(mPartnerBrowserCustomizations.isInitialized());
Assert.assertTrue(mPartnerBrowserCustomizations.isHomepageProviderAvailableAndEnabled());
Assert.assertEquals(TestPartnerBrowserCustomizationsDelayedProvider.HOMEPAGE_URI,
mPartnerBrowserCustomizations.getHomePageUrl());
mPartnerBrowserCustomizations.getHomePageUrl().getSpec());
Assert.assertTrue(HomepageManager.isHomepageEnabled());
Assert.assertEquals(TestPartnerBrowserCustomizationsDelayedProvider.HOMEPAGE_URI,
HomepageManager.getHomepageUri());
Expand Down Expand Up @@ -317,7 +317,7 @@ public void testReadHomepageProvider() throws InterruptedException {
Assert.assertTrue(mPartnerBrowserCustomizations.isInitialized());
Assert.assertTrue(mPartnerBrowserCustomizations.isHomepageProviderAvailableAndEnabled());
Assert.assertEquals(TestPartnerBrowserCustomizationsProvider.HOMEPAGE_URI,
mPartnerBrowserCustomizations.getHomePageUrl());
mPartnerBrowserCustomizations.getHomePageUrl().getSpec());
Assert.assertTrue(HomepageManager.isHomepageEnabled());
Assert.assertEquals(TestPartnerBrowserCustomizationsProvider.HOMEPAGE_URI,
HomepageManager.getHomepageUri());
Expand Down
79 changes: 78 additions & 1 deletion chrome/android/junit/src/org/chromium/chrome/browser/homepage/HomepageManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,30 @@
import androidx.test.filters.SmallTest;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Matchers;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.robolectric.annotation.Config;
import org.robolectric.annotation.Implementation;
import org.robolectric.annotation.Implements;

import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.partnercustomizations.PartnerBrowserCustomizations;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.url.JUnitTestGURLs;

/** Unit tests for {@link HomepageManager}. */
@RunWith(BaseRobolectricTestRunner.class)
@Config(shadows = {HomepageManagerTest.ShadowHomepagePolicyManager.class,
HomepageManagerTest.ShadowUrlUtilities.class})
HomepageManagerTest.ShadowUrlUtilities.class,
HomepageManagerTest.ShadowPartnerBrowserCustomizations.class})
public class HomepageManagerTest {
/** Shadow for {@link HomepagePolicyManager}. */
@Implements(HomepagePolicyManager.class)
Expand All @@ -46,6 +56,29 @@ public static boolean isNTPUrl(String url) {
}
}

@Implements(PartnerBrowserCustomizations.class)
static class ShadowPartnerBrowserCustomizations {
private static PartnerBrowserCustomizations sPartnerBrowserCustomizations;

@Implementation
public static PartnerBrowserCustomizations getInstance() {
return sPartnerBrowserCustomizations;
}

static void setPartnerBrowserCustomizations(
PartnerBrowserCustomizations partnerBrowserCustomizations) {
sPartnerBrowserCustomizations = partnerBrowserCustomizations;
}
}

@Mock
private PartnerBrowserCustomizations mPartnerBrowserCustomizations;

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
}

@Test
@SmallTest
public void testIsHomepageNonNtp() {
Expand All @@ -62,4 +95,48 @@ public void testIsHomepageNonNtp() {
ShadowHomepagePolicyManager.sHomepageUrl = UrlConstants.NTP_URL;
Assert.assertFalse("NTP should be considered the NTP", HomepageManager.isHomepageNonNtp());
}

@Test
@SmallTest
public void testGetDefaultHomepageUri() {
ShadowPartnerBrowserCustomizations.setPartnerBrowserCustomizations(
mPartnerBrowserCustomizations);
Mockito.doNothing()
.when(mPartnerBrowserCustomizations)
.setPartnerHomepageListener(Matchers.anyObject());
Mockito.doReturn(false)
.when(mPartnerBrowserCustomizations)
.isHomepageProviderAvailableAndEnabled();

SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_URI, null);
SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_GURL, null);
Assert.assertEquals(
UrlConstants.NTP_NON_NATIVE_URL, HomepageManager.getDefaultHomepageUri());

final String blueUrl = "https://www.blue.com/";
SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_URI, blueUrl);
SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_GURL, null);
Assert.assertEquals(blueUrl, HomepageManager.getDefaultHomepageUri());

final String redUrl = JUnitTestGURLs.RED_1;
final String serializedRedGurl = JUnitTestGURLs.getGURL(redUrl).serialize();
SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_URI, null);
SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_GURL, serializedRedGurl);
Assert.assertEquals(redUrl, HomepageManager.getDefaultHomepageUri());

final String url1 = JUnitTestGURLs.URL_1;
final String url2 = JUnitTestGURLs.URL_2;
final String serializedGurl2 = JUnitTestGURLs.getGURL(url2).serialize();
SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_URI, url1);
SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_GURL, serializedGurl2);
Assert.assertEquals(url2, HomepageManager.getDefaultHomepageUri());
}
}
1 change: 1 addition & 0 deletions chrome/browser/DEPS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ include_rules = [
"+components/webrtc_logging/common",
"+components/zoom",
"+content/public/android/java/src/org/chromium/content_public/browser",
"+content/public/android/java/src/org/chromium/content_public/common",
"+content/public/browser",
"+content/public/common",
"+courgette",
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/partnercustomizations/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@ android_library("java") {
"//chrome/browser/flags:java",
"//chrome/browser/preferences:java",
"//components/embedder_support/android:util_java",
"//components/url_formatter/android:url_formatter_java",
"//components/version_info/android:version_constants_java",
"//content/public/android:content_main_dex_java",
"//third_party/androidx:androidx_annotation_annotation_java",
"//url:gurl_java",
]
}

Expand Down Expand Up @@ -74,10 +76,15 @@ android_library("javatests") {
":test_support_java",
"//base:base_java_test_support",
"//build/android:build_java",
"//chrome/browser/flags:java",
"//chrome/browser/preferences:java",
"//chrome/test/android:chrome_java_test_support",
"//components/embedder_support/android:util_java",
"//components/url_formatter/android:url_formatter_java",
"//content/public/test/android:content_java_test_support",
"//third_party/android_support_test_runner:runner_java",
"//third_party/androidx:androidx_test_runner_java",
"//third_party/junit:junit",
"//url:gurl_java",
]
}
54 changes: 38 additions & 16 deletions ...a/src/org/chromium/chrome/browser/partnercustomizations/PartnerBrowserCustomizations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import android.content.Context;
import android.content.pm.ApplicationInfo;
import android.text.TextUtils;

import androidx.annotation.VisibleForTesting;

Expand All @@ -18,12 +17,17 @@
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.components.url_formatter.UrlFormatter;
import org.chromium.components.version_info.VersionInfo;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.content_public.common.ContentUrlConstants;
import org.chromium.url.GURL;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

/**
* Reads and caches partner browser customizations information if it exists.
Expand All @@ -49,7 +53,7 @@ public class PartnerBrowserCustomizations {

private static volatile PartnerBrowserCustomizations sInstance;

private volatile String mHomepage;
private volatile GURL mHomepage;
private volatile boolean mIncognitoModeDisabled;
private volatile boolean mBookmarksEditingDisabled;
private boolean mIsInitialized;
Expand Down Expand Up @@ -118,7 +122,8 @@ public static PartnerBrowserCustomizations getInstance() {
* been completed.
*/
public boolean isHomepageProviderAvailableAndEnabled() {
return !TextUtils.isEmpty(getHomePageUrl());
GURL homepageUrl = getHomePageUrl();
return homepageUrl != null && !homepageUrl.isEmpty();
}

/**
Expand Down Expand Up @@ -248,16 +253,31 @@ private void onFinalized() {
boolean refreshHomepage(CustomizationProviderDelegate delegate) {
boolean retVal = false;
try {
String homepage = delegate.getHomepage();
if (!isValidHomepage(homepage)) {
homepage = null;
String homepageUrl = delegate.getHomepage();
GURL homepageGurl;

// Loosely check the scheme. We call fixupUrl to handle about: schemes correctly, and
// also to keep consistent with previous behavior, where chrome: urls were fixed up when
// checking for NTP in isValidHomepage.
if (homepageUrl != null
&& (homepageUrl.startsWith(ContentUrlConstants.ABOUT_URL_SHORT_PREFIX)
|| homepageUrl.startsWith(UrlConstants.CHROME_URL_SHORT_PREFIX))) {
homepageGurl = UrlFormatter.fixupUrl(homepageUrl);
} else {
homepageGurl = new GURL(homepageUrl);
}
if (!TextUtils.equals(mHomepage, homepage)) {

if (!isValidHomepage(homepageGurl)) {
homepageGurl = null;
}
if (!Objects.equals(mHomepage, homepageGurl)) {
retVal = true;
}
mHomepage = homepage;
mHomepage = homepageGurl;
String valueToWrite =
mHomepage == null ? GURL.emptyGURL().serialize() : mHomepage.serialize();
SharedPreferencesManager.getInstance().writeString(
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_URI, mHomepage);
ChromePreferenceKeys.HOMEPAGE_PARTNER_CUSTOMIZED_DEFAULT_GURL, valueToWrite);
} catch (Exception e) {
Log.w(TAG, "Partner homepage delegate URL read failed : ", e);
}
Expand Down Expand Up @@ -330,35 +350,37 @@ private void destroyInternal() {
* @return Home page URL from Android provider. If null, that means either there is no homepage
* provider or provider set it to null to disable homepage.
*/
public String getHomePageUrl() {
public GURL getHomePageUrl() {
CommandLine commandLine = CommandLine.getInstance();
if (commandLine.hasSwitch(ChromeSwitches.PARTNER_HOMEPAGE_FOR_TESTING)) {
return commandLine.getSwitchValue(ChromeSwitches.PARTNER_HOMEPAGE_FOR_TESTING);
return new GURL(
commandLine.getSwitchValue(ChromeSwitches.PARTNER_HOMEPAGE_FOR_TESTING));
}
return mHomepage;
}

@VisibleForTesting
static boolean isValidHomepage(String url) {
static boolean isValidHomepage(GURL url) {
if (url == null) {
return false;
}
if (!UrlUtilities.isHttpOrHttps(url) && !UrlUtilities.isNTPUrl(url)) {

if (!url.isValid() || (!UrlUtilities.isHttpOrHttps(url) && !UrlUtilities.isNTPUrl(url))) {
Log.w(TAG,
"Partner homepage must be HTTP(S) or NewTabPage. "
+ "Got invalid URL \"%s\"",
url);
return false;
}
if (url.length() > HOMEPAGE_URL_MAX_LENGTH) {
Log.w(TAG, "The homepage URL \"%s\" is too long.", url);
if (url.getSpec().length() > HOMEPAGE_URL_MAX_LENGTH) {
Log.w(TAG, "The homepage URL \"%s\" is too long.", url.getSpec());
return false;
}
return true;
}

@VisibleForTesting
public void setHomepageForTests(String homepage) {
mHomepage = homepage;
mHomepage = new GURL(homepage);
}
}

0 comments on commit f445cd9

Please sign in to comment.