Commit 489deba0 authored by Michael Thiessen's avatar Michael Thiessen Committed by Chromium LUCI CQ

Update documentation for UrlUtilities#isNTPUrl(GURL)

Bug: 1139437
Change-Id: I71789d696eab9e692c30d963d16382f9105e8a89
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485307
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Auto-Submit: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835800}
parent e917f7b2
......@@ -220,12 +220,15 @@ public class UrlUtilities {
}
/**
* This variation of #isNTPUrl is for already parsed URLs, not for direct use on user-provided
* url input. Do not do isNTPUrl(new GURL(user_string)), as this will not handle legacy schemes
* like about: correctly. You should use {@link #isNTPUrl(String)} instead, or call
* {@link UrlFormatter#fixupUrl(String)} to create the GURL instead.
*
* @param gurl The GURL to check whether it is for the NTP.
* @return Whether the passed in URL is used to render the NTP.
*/
public static boolean isNTPUrl(GURL gurl) {
// TODO(crbug.com/1139437): isNTPUrl(new GURL("about:newtab")) returns false though
// it should return true for this legacy URL.
if (!gurl.isValid() || !isInternalScheme(gurl)) return false;
return UrlConstants.NTP_HOST.equals(gurl.getHost());
}
......@@ -233,7 +236,11 @@ public class UrlUtilities {
/**
* @param url The URL to check whether it is for the NTP.
* @return Whether the passed in URL is used to render the NTP.
* @deprecated For URLs coming from c++, those URLs should passed around in Java as a GURL.
* For URLs created in Java, coming from third parties or users, those URLs should be
* parsed into a GURL at their source using {@link UrlFormatter#fixupUrl(String)}.
*/
@Deprecated
public static boolean isNTPUrl(String url) {
// Also handle the legacy chrome://newtab and about:newtab URLs since they will redirect to
// chrome-native://newtab natively.
......
......@@ -211,7 +211,8 @@ public class UrlUtilitiesUnitTest {
Assert.assertTrue(UrlUtilities.isNTPUrl(new GURL("chrome-native://newtab")));
Assert.assertTrue(UrlUtilities.isNTPUrl(new GURL("chrome://newtab")));
// TODO(crbug.com/1139437): Differs from UrlUtilities#isNTPUrl(String)
// Note that this intentionally differs from UrlUtilities#isNTPUrl(String) (see comments on
// method).
Assert.assertFalse(UrlUtilities.isNTPUrl(new GURL("about:newtab")));
Assert.assertFalse(UrlUtilities.isNTPUrl(new GURL("http://www.example.com")));
......
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