Commit d623c16c authored by Michael Thiessen's avatar Michael Thiessen Committed by Commit Bot

Prepare to migrate Tab#getUrl users to GURL (3 of 3)

Going through each of the usages will take some care, so I want to
essentially fix the API in a no-op pass and properly migrate each of
the callsites independently (or in batches).

I'm still working on migrating users of WebContents#getVisibleUrl, but
some to the APIs those usages call into are shared by users of
Tab#getUrl, so exposing a GURL Tab#getUrl early makes this less painful
and requires less duplication and converting.

This will be a 3-sided patch:
1. Add the getUrlString() method and migrate upstream usage.
2. Migrate downstream usage to getUrlString().
3. Change getUrl() to return a GURL (this CL).

Bug: 783819
Change-Id: If841d509bff8ea9d2da048021da6ba8635a8cd1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2044379
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742257}
parent 7618d41f
...@@ -91,7 +91,7 @@ public class TabContext { ...@@ -91,7 +91,7 @@ public class TabContext {
String referrerUrl = getReferrerUrlFromTab(tab); String referrerUrl = getReferrerUrlFromTab(tab);
return new TabInfo(tab.getId(), tab.getTitle(), tab.getUrlString(), return new TabInfo(tab.getId(), tab.getTitle(), tab.getUrlString(),
((TabImpl) tab).getOriginalUrl(), referrerUrl != null ? referrerUrl : "", ((TabImpl) tab).getOriginalUrl(), referrerUrl != null ? referrerUrl : "",
tab.getTimestampMillis(), ((TabImpl) tab).getProfile(), tab.getUrl()); tab.getTimestampMillis(), ((TabImpl) tab).getProfile(), tab.getUrlString());
} }
public double getSiteEngagementScore() { public double getSiteEngagementScore() {
......
...@@ -23,7 +23,7 @@ public class ShareUtils { ...@@ -23,7 +23,7 @@ public class ShareUtils {
return false; return false;
} }
String url = tab.getUrl(); String url = tab.getUrlString();
boolean isChromeScheme = url.startsWith(UrlConstants.CHROME_URL_PREFIX) boolean isChromeScheme = url.startsWith(UrlConstants.CHROME_URL_PREFIX)
|| url.startsWith(UrlConstants.CHROME_NATIVE_URL_PREFIX); || url.startsWith(UrlConstants.CHROME_NATIVE_URL_PREFIX);
boolean isShowingInterstitialPage = boolean isShowingInterstitialPage =
......
...@@ -16,6 +16,7 @@ import org.chromium.components.embedder_support.view.ContentView; ...@@ -16,6 +16,7 @@ import org.chromium.components.embedder_support.view.ContentView;
import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.WindowAndroid; import org.chromium.ui.base.WindowAndroid;
import org.chromium.url.GURL;
/** /**
* Tab is a visual/functional unit that encapsulates the content (not just web site content * Tab is a visual/functional unit that encapsulates the content (not just web site content
...@@ -99,17 +100,17 @@ public interface Tab extends TabLifecycle { ...@@ -99,17 +100,17 @@ public interface Tab extends TabLifecycle {
/** /**
* @return The URL that is loaded in the current tab. This may not be the same as * @return The URL that is loaded in the current tab. This may not be the same as
* the last committed URL if a new navigation is in progress. * the last committed URL if a new navigation is in progress.
*
* @deprecated Please use {@link #getUrl()} instead.
*/ */
@Deprecated
String getUrlString(); String getUrlString();
/** /**
* Note: This function is currently being refactored to return a GURL. Please use
* {@link #getUrlString} for now.
*
* @return The URL that is loaded in the current tab. This may not be the same as * @return The URL that is loaded in the current tab. This may not be the same as
* the last committed URL if a new navigation is in progress. * the last committed URL if a new navigation is in progress.
*/ */
String getUrl(); GURL getUrl();
/** /**
* @return The tab title. * @return The tab title.
......
...@@ -53,6 +53,7 @@ import org.chromium.content_public.browser.WebContentsAccessibility; ...@@ -53,6 +53,7 @@ import org.chromium.content_public.browser.WebContentsAccessibility;
import org.chromium.content_public.common.ResourceRequestBody; import org.chromium.content_public.common.ResourceRequestBody;
import org.chromium.ui.base.PageTransition; import org.chromium.ui.base.PageTransition;
import org.chromium.ui.base.WindowAndroid; import org.chromium.ui.base.WindowAndroid;
import org.chromium.url.GURL;
/** /**
* Implementation of the interface {@link Tab}. Contains and manages a {@link ContentView}. * Implementation of the interface {@link Tab}. Contains and manages a {@link ContentView}.
...@@ -156,7 +157,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer { ...@@ -156,7 +157,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
/** /**
* URL of the page currently loading. Used as a fall-back in case tab restore fails. * URL of the page currently loading. Used as a fall-back in case tab restore fails.
*/ */
private String mUrl; private GURL mUrl;
/** /**
* True while a page load is in progress. * True while a page load is in progress.
...@@ -351,21 +352,21 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer { ...@@ -351,21 +352,21 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
@Override @Override
public String getUrlString() { public String getUrlString() {
return getUrl(); return getUrl().getSpec();
} }
@CalledByNative @CalledByNative
@Override @Override
public String getUrl() { public GURL getUrl() {
String url = getWebContents() != null ? getWebContents().getVisibleUrlString() : ""; GURL url = getWebContents() != null ? getWebContents().getVisibleUrl() : GURL.emptyGURL();
// If we have a ContentView, or a NativePage, or the url is not empty, we have a WebContents // If we have a ContentView, or a NativePage, or the url is not empty, we have a WebContents
// so cache the WebContent's url. If not use the cached version. // so cache the WebContent's url. If not use the cached version.
if (getWebContents() != null || isNativePage() || !TextUtils.isEmpty(url)) { if (getWebContents() != null || isNativePage() || !url.getSpec().isEmpty()) {
mUrl = url; mUrl = url;
} }
return mUrl != null ? mUrl : ""; return mUrl != null ? mUrl : GURL.emptyGURL();
} }
@CalledByNative @CalledByNative
...@@ -836,7 +837,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer { ...@@ -836,7 +837,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
mLaunchTypeAtCreation = mLaunchType; mLaunchTypeAtCreation = mLaunchType;
mPendingLoadParams = loadUrlParams; mPendingLoadParams = loadUrlParams;
if (loadUrlParams != null) mUrl = loadUrlParams.getUrl(); if (loadUrlParams != null) mUrl = new GURL(loadUrlParams.getUrl());
TabHelpers.initTabHelpers(this, parent, creationState); TabHelpers.initTabHelpers(this, parent, creationState);
...@@ -887,7 +888,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer { ...@@ -887,7 +888,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
assert state != null; assert state != null;
mFrozenContentsState = state.contentsState; mFrozenContentsState = state.contentsState;
mTimestampMillis = state.timestampMillis; mTimestampMillis = state.timestampMillis;
mUrl = state.getVirtualUrlFromState(); mUrl = new GURL(state.getVirtualUrlFromState());
mTitle = state.getDisplayTitleFromState(); mTitle = state.getDisplayTitleFromState();
mLaunchTypeAtCreation = state.tabLaunchTypeAtCreation; mLaunchTypeAtCreation = state.tabLaunchTypeAtCreation;
mRootId = state.rootId == Tab.INVALID_TAB_ID ? mId : state.rootId; mRootId = state.rootId == Tab.INVALID_TAB_ID ? mId : state.rootId;
...@@ -1442,7 +1443,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer { ...@@ -1442,7 +1443,7 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
initWebContents(webContents); initWebContents(webContents);
if (failedToRestore) { if (failedToRestore) {
String url = TextUtils.isEmpty(mUrl) ? UrlConstants.NTP_URL : mUrl; String url = mUrl.getSpec().isEmpty() ? UrlConstants.NTP_URL : mUrl.getSpec();
loadUrl(new LoadUrlParams(url, PageTransition.GENERATED)); loadUrl(new LoadUrlParams(url, PageTransition.GENERATED));
} }
} finally { } finally {
......
...@@ -61,6 +61,8 @@ ...@@ -61,6 +61,8 @@
#include "ui/android/view_android.h" #include "ui/android/view_android.h"
#include "ui/android/window_android.h" #include "ui/android/window_android.h"
#include "ui/base/resource/resource_bundle.h" #include "ui/base/resource/resource_bundle.h"
#include "url/android/gurl_android.h"
#include "url/gurl.h"
using base::android::AttachCurrentThread; using base::android::AttachCurrentThread;
using base::android::ConvertUTF8ToJavaString; using base::android::ConvertUTF8ToJavaString;
...@@ -162,8 +164,9 @@ bool TabAndroid::IsNativePage() const { ...@@ -162,8 +164,9 @@ bool TabAndroid::IsNativePage() const {
GURL TabAndroid::GetURL() const { GURL TabAndroid::GetURL() const {
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
return GURL(base::android::ConvertJavaStringToUTF8( std::unique_ptr<GURL> gurl = url::GURLAndroid::ToNativeGURL(
Java_TabImpl_getUrl(env, weak_java_tab_.get(env)))); env, Java_TabImpl_getUrl(env, weak_java_tab_.get(env)));
return std::move(*gurl);
} }
bool TabAndroid::IsUserInteractable() const { bool TabAndroid::IsUserInteractable() const {
......
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