Commit ccbd4034 authored by Yaron Friedman's avatar Yaron Friedman Committed by Chromium LUCI CQ

[GURL] Remove WebContents.getVisibleUrlString()

BUG=783819

Change-Id: I403274b7afc4a34bc978a5717b05bccc22464973
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593874
Commit-Queue: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837377}
parent ff236036
......@@ -49,6 +49,7 @@ import org.chromium.ui.modaldialog.SimpleModalDialogController;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.mojom.WindowOpenDisposition;
import org.chromium.ui.util.ColorUtils;
import org.chromium.url.GURL;
/**
* {@link WebContentsDelegateAndroid} that interacts with {@link ChromeActivity} and those
......@@ -58,7 +59,7 @@ import org.chromium.ui.util.ColorUtils;
public class ActivityTabWebContentsDelegateAndroid extends TabWebContentsDelegateAndroid {
private static final String TAG = "ActivityTabWCDA";
private final ArrayMap<WebContents, String> mWebContentsUrlMapping = new ArrayMap<>();
private final ArrayMap<WebContents, GURL> mWebContentsUrlMapping = new ArrayMap<>();
private final Tab mTab;
......@@ -115,7 +116,7 @@ public class ActivityTabWebContentsDelegateAndroid extends TabWebContentsDelegat
@Override
public void webContentsCreated(WebContents sourceWebContents, long openerRenderProcessId,
long openerRenderFrameId, String frameName, String targetUrl,
long openerRenderFrameId, String frameName, GURL targetUrl,
WebContents newWebContents) {
// The URL can't be taken from the WebContents if it's paused. Save it for later.
assert !mWebContentsUrlMapping.containsKey(newWebContents);
......@@ -152,7 +153,7 @@ public class ActivityTabWebContentsDelegateAndroid extends TabWebContentsDelegat
assert tabCreator != null;
// Grab the URL, which might not be available via the Tab.
String url = mWebContentsUrlMapping.remove(webContents);
GURL url = mWebContentsUrlMapping.remove(webContents);
// Skip opening a new Tab if it doesn't make sense.
if (mTab.isClosing()) return false;
......@@ -177,7 +178,7 @@ public class ActivityTabWebContentsDelegateAndroid extends TabWebContentsDelegat
} else if (disposition == WindowOpenDisposition.NEW_POPUP) {
PolicyAuditor auditor = AppHooks.get().getPolicyAuditor();
auditor.notifyAuditEvent(ContextUtils.getApplicationContext(),
AuditEvent.OPEN_POPUP_URL_SUCCESS, url, "");
AuditEvent.OPEN_POPUP_URL_SUCCESS, url.getSpec(), "");
}
}
......
......@@ -773,7 +773,7 @@ public class OfflinePageUtils {
// TODO(crbug.com/1033178): dedupe the
// DomDistillerUrlUtils#getOriginalUrlFromDistillerUrl() calls.
String distilledUrl = DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(
webContents.getVisibleUrlString());
webContents.getVisibleUrl());
// If current page is an offline page, reload it with custom behavior defined in extra
// header respected.
LoadUrlParams params = new LoadUrlParams(distilledUrl, transitionTypeForReload);
......
......@@ -173,8 +173,7 @@ public class ChromePageInfoControllerDelegate extends PageInfoControllerDelegate
bridge.loadOriginal(mWebContents);
});
};
final String previewOriginalHost =
bridge.getOriginalHost(mWebContents.getVisibleUrlString());
final String previewOriginalHost = mWebContents.getVisibleUrl().getHost();
final String loadOriginalText = mContext.getString(
R.string.page_info_preview_load_original, previewOriginalHost);
final SpannableString loadOriginalSpan = SpanApplier.applySpans(loadOriginalText,
......
......@@ -37,8 +37,8 @@ public class SiteSettingsHelper {
PreviewsAndroidBridge.getInstance().shouldShowPreviewUI(webContents);
// TODO(crbug.com/1033178): dedupe the DomDistillerUrlUtils#getOriginalUrlFromDistillerUrl()
// calls.
String url = DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(
webContents.getVisibleUrlString());
String url =
DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(webContents.getVisibleUrl());
String scheme = GURLUtils.getScheme(url);
return !isOfflinePage && !isPreviewPage
&& (UrlConstants.HTTP_SCHEME.equals(scheme)
......
......@@ -8,9 +8,6 @@ import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.content_public.browser.WebContents;
import org.chromium.url.URI;
import java.net.URISyntaxException;
/**
* Java bridge class to C++ Previews code.
......@@ -37,17 +34,6 @@ public final class PreviewsAndroidBridge {
mNativePreviewsAndroidBridge, PreviewsAndroidBridge.this, webContents);
}
/**
* Returns the original host name for visibleURL. This should only be used on preview pages.
*/
public String getOriginalHost(String visibleURL) {
try {
return new URI(visibleURL).getHost();
} catch (URISyntaxException e) {
}
return "";
}
/**
* Requests that the original page be loaded.
*/
......
......@@ -372,7 +372,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
@Override
public String getOriginalUrl() {
return DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(getUrlString());
return DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(getUrl());
}
@CalledByNative
......
......@@ -210,7 +210,7 @@ final class TabWebContentsDelegateAndroidImpl extends TabWebContentsDelegateAndr
@Override
public void webContentsCreated(WebContents sourceWebContents, long openerRenderProcessId,
long openerRenderFrameId, String frameName, String targetUrl,
long openerRenderFrameId, String frameName, GURL targetUrl,
WebContents newWebContents) {
mDelegate.webContentsCreated(sourceWebContents, openerRenderProcessId, openerRenderFrameId,
frameName, targetUrl, newWebContents);
......
......@@ -239,7 +239,7 @@ public class ChromeTabCreator extends TabCreator {
@Override
public boolean createTabWithWebContents(
@Nullable Tab parent, WebContents webContents, @TabLaunchType int type, String url) {
@Nullable Tab parent, WebContents webContents, @TabLaunchType int type, GURL url) {
// The parent tab was already closed. Do not open child tabs.
int parentId = parent != null ? parent.getId() : Tab.INVALID_TAB_ID;
if (mTabModel.isClosurePending(parentId)) return false;
......
......@@ -10,6 +10,7 @@ import android.content.Intent;
import android.net.Uri;
import android.provider.Browser;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import org.chromium.base.ApplicationStatus;
......@@ -32,6 +33,7 @@ import org.chromium.chrome.browser.tabmodel.AsyncTabCreator;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.PageTransition;
import org.chromium.url.GURL;
/**
* Asynchronously creates Tabs by creating/starting up Activities.
......@@ -81,13 +83,10 @@ public class TabDelegate extends AsyncTabCreator {
}
@Override
public boolean createTabWithWebContents(
@Nullable Tab parent, WebContents webContents, @TabLaunchType int type, String url) {
if (url == null) url = "";
AsyncTabCreationParams asyncParams =
new AsyncTabCreationParams(
new LoadUrlParams(url, PageTransition.AUTO_TOPLEVEL), webContents);
public boolean createTabWithWebContents(@Nullable Tab parent, WebContents webContents,
@TabLaunchType int type, @NonNull GURL url) {
AsyncTabCreationParams asyncParams = new AsyncTabCreationParams(
new LoadUrlParams(url.getSpec(), PageTransition.AUTO_TOPLEVEL), webContents);
createNewTab(asyncParams, type, parent != null ? parent.getId() : Tab.INVALID_TAB_ID);
return true;
}
......
......@@ -7,11 +7,8 @@ package org.chromium.chrome.browser.webapps.addtohomescreen;
import android.app.Activity;
import android.content.Context;
import android.os.Bundle;
import android.text.TextUtils;
import androidx.annotation.StringRes;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.chrome.browser.app.ChromeActivity;
import org.chromium.chrome.browser.banners.AppBannerManager;
......@@ -74,7 +71,7 @@ public class AddToHomescreenCoordinator {
@VisibleForTesting
boolean showForAppMenu(WebContents webContents, @StringRes int titleId) {
// Don't start if there is no visible URL to add.
if (webContents == null || TextUtils.isEmpty(webContents.getVisibleUrlString())) {
if (webContents == null || webContents.getVisibleUrl().isEmpty()) {
return false;
}
......
......@@ -48,6 +48,7 @@ android_library("java") {
"//components/embedder_support/android:util_java",
"//content/public/android:content_java",
"//third_party/android_deps:androidx_annotation_annotation_java",
"//url:gurl_java",
]
}
......
......@@ -4,6 +4,7 @@
package org.chromium.chrome.browser.tabmodel;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import org.chromium.base.TraceEvent;
......@@ -13,6 +14,7 @@ import org.chromium.chrome.browser.tab.TabState;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.WebContents;
import org.chromium.url.GURL;
/**
* Creates Tabs. If the TabCreator creates Tabs asynchronously, null pointers will be returned
......@@ -68,8 +70,8 @@ public abstract class TabCreator {
* @param url URL to show in the Tab. (Needed only for asynchronous tab creation.)
* @return Whether a Tab was created successfully.
*/
public abstract boolean createTabWithWebContents(
@Nullable Tab parent, WebContents webContents, @TabLaunchType int type, String url);
public abstract boolean createTabWithWebContents(@Nullable Tab parent, WebContents webContents,
@TabLaunchType int type, @NonNull GURL url);
/**
* Creates a tab around the native web contents pointer.
......@@ -80,8 +82,7 @@ public abstract class TabCreator {
*/
public final boolean createTabWithWebContents(
Tab parent, WebContents webContents, @TabLaunchType int type) {
return createTabWithWebContents(
parent, webContents, type, webContents.getVisibleUrlString());
return createTabWithWebContents(parent, webContents, type, webContents.getVisibleUrl());
}
/**
......
......@@ -18,6 +18,7 @@ import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.WebContents;
import org.chromium.url.GURL;
/** MockTabCreator for use in tests. */
public class MockTabCreator extends TabCreator {
......@@ -62,7 +63,7 @@ public class MockTabCreator extends TabCreator {
@Override
public boolean createTabWithWebContents(
Tab parent, WebContents webContents, @TabLaunchType int type, String url) {
Tab parent, WebContents webContents, @TabLaunchType int type, GURL url) {
return false;
}
......
......@@ -35,15 +35,21 @@ public final class DomDistillerUrlUtils {
return DomDistillerUrlUtilsJni.get().getDistillerViewUrlFromUrl(scheme, url, title);
}
@Deprecated
public static String getOriginalUrlFromDistillerUrl(String url) {
if (TextUtils.isEmpty(url)) return url;
return DomDistillerUrlUtilsJni.get().getOriginalUrlFromDistillerUrl(url);
}
/**
* Returns the original URL of a distillation given the viewer URL.
*
* @param url The current viewer URL.
* @return the URL of the original page.
*/
public static String getOriginalUrlFromDistillerUrl(String url) {
if (TextUtils.isEmpty(url)) return url;
return DomDistillerUrlUtilsJni.get().getOriginalUrlFromDistillerUrl(url);
public static String getOriginalUrlFromDistillerUrl(GURL url) {
if (url.isEmpty()) return url.getSpec();
return DomDistillerUrlUtilsJni.get().getOriginalUrlFromDistillerUrl(url.getSpec());
}
public static boolean isDistilledPage(String url) {
......
......@@ -199,11 +199,12 @@ void WebContentsDelegateAndroid::WebContentsCreated(
if (new_contents)
jnew_contents = new_contents->GetJavaWebContents();
ScopedJavaLocalRef<jobject> java_gurl =
url::GURLAndroid::FromNativeGURL(env, target_url);
Java_WebContentsDelegateAndroid_webContentsCreated(
env, obj, jsource_contents, opener_render_process_id,
opener_render_frame_id,
base::android::ConvertUTF8ToJavaString(env, frame_name),
base::android::ConvertUTF8ToJavaString(env, target_url.spec()),
base::android::ConvertUTF8ToJavaString(env, frame_name), java_gurl,
jnew_contents);
}
......
......@@ -66,7 +66,7 @@ public class WebContentsDelegateAndroid {
@CalledByNative
public void webContentsCreated(WebContents sourceWebContents, long openerRenderProcessId,
long openerRenderFrameId, String frameName, String targetUrl,
long openerRenderFrameId, String frameName, GURL targetUrl,
WebContents newWebContents) {}
@CalledByNative
......
......@@ -175,8 +175,7 @@ public class PageInfoController implements PageInfoMainController, ModalDialogPr
// calls.
mFullUrl = mDelegate.isShowingOfflinePage()
? mDelegate.getOfflinePageUrl()
: DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(
webContents.getVisibleUrlString());
: DomDistillerUrlUtils.getOriginalUrlFromDistillerUrl(webContents.getVisibleUrl());
// This can happen if an invalid chrome-distiller:// url was entered.
if (mFullUrl == null) mFullUrl = "";
......
......@@ -168,7 +168,7 @@ public class WebContentsImpl implements WebContents, RenderFrameHostDelegate, Wi
(int) (coordinateSpace.getContentOffsetYPix()
/ coordinateSpace.getDeviceScaleFactor()));
Bundle bundle = new Bundle();
bundle.putString("url", getVisibleUrlString());
bundle.putString("url", getVisibleUrl().getSpec());
bundle.putString("title", getTitle());
bundle.putString("text", text);
bundle.putString("html", html);
......@@ -446,11 +446,6 @@ public class WebContentsImpl implements WebContents, RenderFrameHostDelegate, Wi
mNativeWebContentsAndroid, WebContentsImpl.this);
}
@Override
public String getVisibleUrlString() {
return getVisibleUrl().getSpec();
}
@Override
public String getEncoding() {
checkNotDestroyed();
......
......@@ -183,14 +183,6 @@ public interface WebContents extends Parcelable {
*/
GURL getVisibleUrl();
/**
* @return The URL for the current visible page.
*
* @deprecated Please use {@link #getVisibleUrl} instead.
*/
@Deprecated
String getVisibleUrlString();
/**
* @return The character encoding for the current visible page.
*/
......
......@@ -118,12 +118,7 @@ public class MockWebContents implements WebContents {
@Override
public GURL getVisibleUrl() {
return null;
}
@Override
public String getVisibleUrlString() {
return null;
return GURL.emptyGURL();
}
@Override
......
......@@ -94,6 +94,7 @@ android_library("content_shell_java") {
"//net/android:net_java",
"//ui/android:ui_java",
"//ui/base/cursor/mojom:cursor_type_java",
"//url:gurl_java",
]
sources = [
"java/src/org/chromium/content_shell/Shell.java",
......@@ -255,6 +256,7 @@ android_library("content_shell_test_java") {
"//third_party/hamcrest:hamcrest_java",
"//third_party/junit:junit",
"//ui/android:ui_java",
"//url:gurl_java",
]
sources = [
"javatests/src/org/chromium/content_shell_apk/ContentShellActivityTestRule.java",
......
......@@ -171,7 +171,7 @@ public class Shell extends LinearLayout {
mPrevButton.setVisibility(hasFocus ? GONE : VISIBLE);
mStopReloadButton.setVisibility(hasFocus ? GONE : VISIBLE);
if (!hasFocus) {
mUrlTextView.setText(mWebContents.getVisibleUrlString());
mUrlTextView.setText(mWebContents.getVisibleUrl().getSpec());
}
}
});
......@@ -306,9 +306,7 @@ public class Shell extends LinearLayout {
.setActionModeCallback(defaultActionCallback());
mNavigationController = mWebContents.getNavigationController();
if (getParent() != null) mWebContents.onShow();
if (mWebContents.getVisibleUrlString() != null) {
mUrlTextView.setText(mWebContents.getVisibleUrlString());
}
mUrlTextView.setText(mWebContents.getVisibleUrl().getSpec());
((FrameLayout) findViewById(R.id.contentview_holder)).addView(cv,
new FrameLayout.LayoutParams(
FrameLayout.LayoutParams.MATCH_PARENT,
......
......@@ -38,14 +38,14 @@ public class ContentShellShellManagementTest {
final ContentShellActivity activity =
mActivityTestRule.launchContentShellWithUrl(TEST_PAGE_1);
Assert.assertEquals(
TEST_PAGE_1, activity.getActiveShell().getWebContents().getVisibleUrlString());
TEST_PAGE_1, activity.getActiveShell().getWebContents().getVisibleUrl().getSpec());
Shell previousActiveShell = activity.getActiveShell();
Assert.assertFalse(previousActiveShell.isDestroyed());
mActivityTestRule.loadNewShell(TEST_PAGE_2);
Assert.assertEquals(
TEST_PAGE_2, activity.getActiveShell().getWebContents().getVisibleUrlString());
TEST_PAGE_2, activity.getActiveShell().getWebContents().getVisibleUrl().getSpec());
Assert.assertNotSame(previousActiveShell, activity.getActiveShell());
Assert.assertNull(previousActiveShell.getWebContents());
......
......@@ -35,6 +35,7 @@ public class ContentShellUrlTest {
Assert.assertNotNull(activity);
// Make sure that the URL is set as expected.
Assert.assertEquals(URL, activity.getActiveShell().getWebContents().getVisibleUrlString());
Assert.assertEquals(
URL, activity.getActiveShell().getWebContents().getVisibleUrl().getSpec());
}
}
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