Update WebContentsObserver to use GURL.

Stopped when I hit TabObserver but this covers most of the clients

BUG=783819

Change-Id: I499c6105c7c64f09d65bc07436cdb7c2112e7dbf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2572244Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834522}

android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java

@@ -13,6 +13,7 @@ import org.chromium.content_public.browser.WebContentsObserver;
 import org.chromium.content_public.common.ContentUrlConstants;
 import org.chromium.net.NetError;
 import org.chromium.ui.base.PageTransition;
+import org.chromium.url.GURL;
 
 import java.lang.ref.WeakReference;
 
@@ -52,18 +53,20 @@ public class AwWebContentsObserver extends WebContentsObserver {
     }
 
     @Override
-    public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {
+    public void didFinishLoad(long frameId, GURL url, boolean isKnownValid, boolean isMainFrame) {
+        String validatedUrl = isKnownValid ? url.getSpec() : url.getPossiblyInvalidSpec();
         if (isMainFrame && getClientIfNeedToFireCallback(validatedUrl) != null) {
             mLastDidFinishLoadUrl = validatedUrl;
         }
     }
 
     @Override
-    public void didStopLoading(String validatedUrl) {
-        if (validatedUrl.length() == 0) validatedUrl = ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL;
-        AwContentsClient client = getClientIfNeedToFireCallback(validatedUrl);
-        if (client != null && validatedUrl.equals(mLastDidFinishLoadUrl)) {
-            client.getCallbackHelper().postOnPageFinished(validatedUrl);
+    public void didStopLoading(GURL gurl, boolean isKnownValid) {
+        String url = isKnownValid ? gurl.getSpec() : gurl.getPossiblyInvalidSpec();
+        if (url.length() == 0) url = ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL;
+        AwContentsClient client = getClientIfNeedToFireCallback(url);
+        if (client != null && url.equals(mLastDidFinishLoadUrl)) {
+            client.getCallbackHelper().postOnPageFinished(url);
             mLastDidFinishLoadUrl = null;
         }
     }
@@ -76,7 +79,8 @@ public class AwWebContentsObserver extends WebContentsObserver {
     }
 
     @Override
-    public void didFailLoad(boolean isMainFrame, @NetError int errorCode, String failingUrl) {
+    public void didFailLoad(boolean isMainFrame, @NetError int errorCode, GURL failingGurl) {
+        String failingUrl = failingGurl.getPossiblyInvalidSpec();
         AwContentsClient client = mAwContentsClient.get();
         if (client == null) return;
         String unreachableWebDataUrl = AwContentsStatics.getUnreachableWebDataUrl();
@@ -109,7 +113,7 @@ public class AwWebContentsObserver extends WebContentsObserver {
     public void didFinishNavigation(NavigationHandle navigation) {
         String url = navigation.getUrlString();
         if (navigation.errorCode() != NetError.OK && !navigation.isDownload()) {
-            didFailLoad(navigation.isInMainFrame(), navigation.errorCode(), url);
+            didFailLoad(navigation.isInMainFrame(), navigation.errorCode(), navigation.getUrl());
         }
 
         if (!navigation.hasCommitted()) return;

android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java

@@ -9,7 +9,6 @@ import android.graphics.Picture;
 import android.net.http.SslError;
 
 import org.chromium.android_webview.AwConsoleMessage;
-import org.chromium.android_webview.AwContentsClient.AwWebResourceRequest;
 import org.chromium.base.Callback;
 import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayContentDelegate.java

@@ -43,9 +43,8 @@ public class OverlayContentDelegate {
 
     /**
      * Called when content started loading in the panel.
-     * @param url The URL that is loading.
      */
-    public void onContentLoadStarted(String url) {}
+    public void onContentLoadStarted() {}
 
     /**
      * Called when the navigation entry has been committed.

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java

@@ -35,6 +35,7 @@ import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.browser.WebContentsObserver;
 import org.chromium.content_public.common.ResourceRequestBody;
 import org.chromium.ui.base.ViewAndroidDelegate;
+import org.chromium.url.GURL;
 
 /**
  * Content container for an OverlayPanel. This class is responsible for the management of the
@@ -349,8 +350,8 @@ public class OverlayPanelContent {
         mWebContentsObserver =
                 new WebContentsObserver(mWebContents) {
                     @Override
-                    public void didStartLoading(String url) {
-                        mContentDelegate.onContentLoadStarted(url);
+                    public void didStartLoading(GURL url) {
+                        mContentDelegate.onContentLoadStarted();
                     }
 
                     @Override

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java

@@ -1039,7 +1039,7 @@ public class ContextualSearchManager
         }
 
         @Override
-        public void onContentLoadStarted(String url) {
+        public void onContentLoadStarted() {
             mDidPromoteSearchNavigation = false;
         }
 

chrome/android/java/src/org/chromium/chrome/browser/payments/handler/PaymentHandlerMediator.java

@@ -30,6 +30,7 @@ import org.chromium.content_public.browser.WebContentsObserver;
 import org.chromium.payments.mojom.PaymentEventResponseType;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.util.TokenHolder;
+import org.chromium.url.GURL;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -274,7 +275,7 @@ import java.lang.annotation.RetentionPolicy;
 
     // Implement WebContentsObserver:
     @Override
-    public void didFailLoad(boolean isMainFrame, int errorCode, String failingUrl) {
+    public void didFailLoad(boolean isMainFrame, int errorCode, GURL failingUrl) {
         if (!isMainFrame) return;
         mHandler.post(() -> {
             mCloseReason = CloseReason.FAIL_LOAD;

chrome/android/java/src/org/chromium/chrome/browser/payments/handler/toolbar/PaymentHandlerToolbarMediator.java

@@ -13,6 +13,7 @@ import org.chromium.content_public.browser.NavigationHandle;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.browser.WebContentsObserver;
 import org.chromium.ui.modelutil.PropertyModel;
+import org.chromium.url.GURL;
 
 /**
  * PaymentHandlerToolbar mediator, which is responsible for receiving events from the view and
@@ -70,7 +71,7 @@ import org.chromium.ui.modelutil.PropertyModel;
 
     // WebContentsObserver:
     @Override
-    public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {
+    public void didFinishLoad(long frameId, GURL url, boolean isKnownValid, boolean isMainFrame) {
         // Hides the Progress Bar after a delay to make sure it is rendered for at least
         // a few frames, otherwise its completion won't be visually noticeable.
         mHideProgressBarHandler = new Handler();
@@ -81,7 +82,7 @@ import org.chromium.ui.modelutil.PropertyModel;
     }
 
     @Override
-    public void didFailLoad(boolean isMainFrame, int errorCode, String failingUrl) {
+    public void didFailLoad(boolean isMainFrame, int errorCode, GURL failingUrl) {
         mModel.set(PaymentHandlerToolbarProperties.PROGRESS_VISIBLE, false);
     }
 

chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsObserver.java

@@ -224,18 +224,22 @@ public class TabWebContentsObserver extends TabWebContentsUserData {
         }
 
         @Override
-        public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {
+        public void didFinishLoad(
+                long frameId, GURL url, boolean isKnownValid, boolean isMainFrame) {
+            assert isKnownValid;
             if (mTab.getNativePage() != null) {
                 mTab.pushNativePageStateToNavigationEntry();
             }
-            if (isMainFrame) mTab.didFinishPageLoad(validatedUrl);
+            if (isMainFrame) mTab.didFinishPageLoad(url.getSpec());
             PolicyAuditor auditor = AppHooks.get().getPolicyAuditor();
             auditor.notifyAuditEvent(ContextUtils.getApplicationContext(),
-                    AuditEvent.OPEN_URL_SUCCESS, validatedUrl, "");
+                    AuditEvent.OPEN_URL_SUCCESS, url.getSpec(), "");
         }
 
         @Override
-        public void didFailLoad(boolean isMainFrame, int errorCode, String failingUrl) {
+        public void didFailLoad(boolean isMainFrame, int errorCode, GURL failingGurl) {
+            String failingUrl = failingGurl.getSpec();
+
             RewindableIterator<TabObserver> observers = mTab.getTabObservers();
             while (observers.hasNext()) {
                 observers.next().onDidFailLoad(mTab, isMainFrame, errorCode, failingUrl);

content/browser/android/web_contents_observer_proxy.cc

@@ -21,6 +21,7 @@
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
+#include "url/android/gurl_android.h"
 
 using base::android::AttachCurrentThread;
 using base::android::JavaParamRef;
@@ -100,25 +101,23 @@ void WebContentsObserverProxy::RenderProcessGone(
 
 void WebContentsObserverProxy::DidStartLoading() {
   JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jstring> jstring_url(
-      ConvertUTF8ToJavaString(env, web_contents()->GetVisibleURL().spec()));
   if (auto* entry = web_contents()->GetController().GetPendingEntry()) {
     base_url_of_last_started_data_url_ = entry->GetBaseURLForDataURL();
   }
-  Java_WebContentsObserverProxy_didStartLoading(env, java_observer_,
-                                                jstring_url);
+  Java_WebContentsObserverProxy_didStartLoading(
+      env, java_observer_,
+      url::GURLAndroid::FromNativeGURL(env, web_contents()->GetVisibleURL()));
 }
 
 void WebContentsObserverProxy::DidStopLoading() {
   JNIEnv* env = AttachCurrentThread();
-  std::string url_string = web_contents()->GetLastCommittedURL().spec();
-  SetToBaseURLForDataURLIfNeeded(&url_string);
+  GURL url = web_contents()->GetLastCommittedURL();
+  bool assume_valid = SetToBaseURLForDataURLIfNeeded(&url);
   // DidStopLoading is the last event we should get.
   base_url_of_last_started_data_url_ = GURL::EmptyGURL();
-  ScopedJavaLocalRef<jstring> jstring_url(ConvertUTF8ToJavaString(
-      env, url_string));
-  Java_WebContentsObserverProxy_didStopLoading(env, java_observer_,
-                                               jstring_url);
+  Java_WebContentsObserverProxy_didStopLoading(
+      env, java_observer_, url::GURLAndroid::FromNativeGURL(env, url),
+      assume_valid);
 }
 
 void WebContentsObserverProxy::LoadProgressChanged(double progress) {
@@ -130,12 +129,9 @@ void WebContentsObserverProxy::DidFailLoad(RenderFrameHost* render_frame_host,
                                            const GURL& validated_url,
                                            int error_code) {
   JNIEnv* env = AttachCurrentThread();
-  ScopedJavaLocalRef<jstring> jstring_url(
-      ConvertUTF8ToJavaString(env, validated_url.spec()));
-
-  Java_WebContentsObserverProxy_didFailLoad(env, java_observer_,
-                                            !render_frame_host->GetParent(),
-                                            error_code, jstring_url);
+  Java_WebContentsObserverProxy_didFailLoad(
+      env, java_observer_, !render_frame_host->GetParent(), error_code,
+      url::GURLAndroid::FromNativeGURL(env, validated_url));
 }
 
 void WebContentsObserverProxy::DidChangeVisibleSecurityState() {
@@ -177,13 +173,12 @@ void WebContentsObserverProxy::DidFinishLoad(RenderFrameHost* render_frame_host,
                                              const GURL& validated_url) {
   JNIEnv* env = AttachCurrentThread();
 
-  std::string url_string = validated_url.spec();
-  SetToBaseURLForDataURLIfNeeded(&url_string);
+  GURL url = validated_url;
+  bool assume_valid = SetToBaseURLForDataURLIfNeeded(&url);
 
-  ScopedJavaLocalRef<jstring> jstring_url(
-      ConvertUTF8ToJavaString(env, url_string));
   Java_WebContentsObserverProxy_didFinishLoad(
-      env, java_observer_, render_frame_host->GetRoutingID(), jstring_url,
+      env, java_observer_, render_frame_host->GetRoutingID(),
+      url::GURLAndroid::FromNativeGURL(env, url), assume_valid,
       !render_frame_host->GetParent());
 }
 
@@ -252,21 +247,23 @@ void WebContentsObserverProxy::TitleWasSet(NavigationEntry* entry) {
   Java_WebContentsObserverProxy_titleWasSet(env, java_observer_, jstring_title);
 }
 
-void WebContentsObserverProxy::SetToBaseURLForDataURLIfNeeded(
-    std::string* url) {
+bool WebContentsObserverProxy::SetToBaseURLForDataURLIfNeeded(GURL* url) {
   NavigationEntry* entry =
       web_contents()->GetController().GetLastCommittedEntry();
   // Note that GetBaseURLForDataURL is only used by the Android WebView.
   // FIXME: Should we only return valid specs and "about:blank" for invalid
   // ones? This may break apps.
   if (entry && !entry->GetBaseURLForDataURL().is_empty()) {
-    *url = entry->GetBaseURLForDataURL().possibly_invalid_spec();
+    *url = entry->GetBaseURLForDataURL();
+    return false;
   } else if (!base_url_of_last_started_data_url_.is_empty()) {
     // NavigationController can lose the pending entry and recreate it without
     // a base URL if there has been a loadUrl("javascript:...") after
     // loadDataWithBaseUrl.
-    *url = base_url_of_last_started_data_url_.possibly_invalid_spec();
+    *url = base_url_of_last_started_data_url_;
+    return false;
   }
+  return true;
 }
 
 void WebContentsObserverProxy::ViewportFitChanged(

content/browser/android/web_contents_observer_proxy.h

@@ -62,7 +62,7 @@ class WebContentsObserverProxy : public WebContentsObserver {
   void WebContentsDestroyed() override;
   void DidChangeThemeColor() override;
   void MediaEffectivelyFullscreenChanged(bool is_fullscreen) override;
-  void SetToBaseURLForDataURLIfNeeded(std::string* url);
+  bool SetToBaseURLForDataURLIfNeeded(GURL* url);
   void ViewportFitChanged(blink::mojom::ViewportFit value) override;
   void OnWebContentsFocused(RenderWidgetHost*) override;
   void OnWebContentsLostFocus(RenderWidgetHost*) override;

content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java

@@ -13,6 +13,7 @@ import org.chromium.base.annotations.NativeMethods;
 import org.chromium.content_public.browser.NavigationHandle;
 import org.chromium.content_public.browser.WebContentsObserver;
 import org.chromium.ui.base.WindowAndroid;
+import org.chromium.url.GURL;
 
 /**
  * Serves as a compound observer proxy for dispatching WebContentsObserver callbacks,
@@ -121,7 +122,7 @@ class WebContentsObserverProxy extends WebContentsObserver {
 
     @Override
     @CalledByNative
-    public void didStartLoading(String url) {
+    public void didStartLoading(GURL url) {
         for (mObserversIterator.rewind(); mObserversIterator.hasNext();) {
             mObserversIterator.next().didStartLoading(url);
         }
@@ -129,9 +130,9 @@ class WebContentsObserverProxy extends WebContentsObserver {
 
     @Override
     @CalledByNative
-    public void didStopLoading(String url) {
+    public void didStopLoading(GURL url, boolean isKnownValid) {
         for (mObserversIterator.rewind(); mObserversIterator.hasNext();) {
-            mObserversIterator.next().didStopLoading(url);
+            mObserversIterator.next().didStopLoading(url, isKnownValid);
         }
     }
 
@@ -153,7 +154,7 @@ class WebContentsObserverProxy extends WebContentsObserver {
 
     @Override
     @CalledByNative
-    public void didFailLoad(boolean isMainFrame, int errorCode, String failingUrl) {
+    public void didFailLoad(boolean isMainFrame, int errorCode, GURL failingUrl) {
         for (mObserversIterator.rewind(); mObserversIterator.hasNext();) {
             mObserversIterator.next().didFailLoad(isMainFrame, errorCode, failingUrl);
         }
@@ -201,9 +202,9 @@ class WebContentsObserverProxy extends WebContentsObserver {
 
     @Override
     @CalledByNative
-    public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {
+    public void didFinishLoad(long frameId, GURL url, boolean isKnownValid, boolean isMainFrame) {
         for (mObserversIterator.rewind(); mObserversIterator.hasNext();) {
-            mObserversIterator.next().didFinishLoad(frameId, validatedUrl, isMainFrame);
+            mObserversIterator.next().didFinishLoad(frameId, url, isKnownValid, isMainFrame);
         }
     }
 

content/public/android/java/src/org/chromium/content_public/browser/WebContentsObserver.java

@@ -9,6 +9,7 @@ import androidx.annotation.Nullable;
 
 import org.chromium.blink.mojom.ViewportFit;
 import org.chromium.ui.base.WindowAndroid;
+import org.chromium.url.GURL;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -87,13 +88,17 @@ public abstract class WebContentsObserver {
      * Called when the a page starts loading.
      * @param url The validated url for the loading page.
      */
-    public void didStartLoading(String url) {}
+    public void didStartLoading(GURL url) {}
 
     /**
      * Called when the a page finishes loading.
-     * @param url The validated url for the page.
+     * @param url The url for the page.
+     * @param isKnownValid Whether the url is known to be valid.
+     * TODO(yfriedman): There's currently a layering violation and this is needed for aw/
+     *     For chrome, the url will always be valid.
+     *
      */
-    public void didStopLoading(String url) {}
+    public void didStopLoading(GURL url, boolean isKnownValid) {}
 
     /**
      * Called when a page's load progress has changed.
@@ -112,7 +117,7 @@ public abstract class WebContentsObserver {
      * @param errorCode Error code for the occurring error.
      * @param failingUrl The url that was loading when the error occurred.
      */
-    public void didFailLoad(boolean isMainFrame, int errorCode, String failingUrl) {}
+    public void didFailLoad(boolean isMainFrame, int errorCode, GURL failingUrl) {}
 
     /**
      * Called when the page had painted something non-empty.
@@ -143,10 +148,11 @@ public abstract class WebContentsObserver {
     /**
      * Notifies that a load has finished for a given frame.
      * @param frameId A positive, non-zero integer identifying the navigating frame.
-     * @param validatedUrl The validated URL that is being navigated to.
+     * @param url The validated URL that is being navigated to.
+     * @param isKnownValid Whether the URL is known to be valid.
      * @param isMainFrame Whether the load is happening for the main frame.
      */
-    public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {}
+    public void didFinishLoad(long frameId, GURL url, boolean isKnownValid, boolean isMainFrame) {}
 
     /**
      * Notifies that the document has finished loading for the given frame.

content/public/test/android/javatests/src/org/chromium/content_public/browser/test/util/TestWebContentsObserver.java

@@ -10,6 +10,7 @@ import org.chromium.content_public.browser.WebContentsObserver;
 import org.chromium.content_public.browser.test.util.TestCallbackHelperContainer.OnPageFinishedHelper;
 import org.chromium.content_public.browser.test.util.TestCallbackHelperContainer.OnPageStartedHelper;
 import org.chromium.content_public.browser.test.util.TestCallbackHelperContainer.OnReceivedErrorHelper;
+import org.chromium.url.GURL;
 
 /**
  * The default WebContentsObserver used by ContentView tests. The below callbacks can be
@@ -52,21 +53,21 @@ public class TestWebContentsObserver extends WebContentsObserver {
      * stop working!
      */
     @Override
-    public void didStartLoading(String url) {
+    public void didStartLoading(GURL url) {
         super.didStartLoading(url);
-        mOnPageStartedHelper.notifyCalled(url);
+        mOnPageStartedHelper.notifyCalled(url.getPossiblyInvalidSpec());
     }
 
     @Override
-    public void didStopLoading(String url) {
-        super.didStopLoading(url);
-        mOnPageFinishedHelper.notifyCalled(url);
+    public void didStopLoading(GURL url, boolean isKnownValid) {
+        super.didStopLoading(url, isKnownValid);
+        mOnPageFinishedHelper.notifyCalled(url.getPossiblyInvalidSpec());
     }
 
     @Override
-    public void didFailLoad(boolean isMainFrame, int errorCode, String failingUrl) {
+    public void didFailLoad(boolean isMainFrame, int errorCode, GURL failingUrl) {
         super.didFailLoad(isMainFrame, errorCode, failingUrl);
-        mOnReceivedErrorHelper.notifyCalled(errorCode, "Error " + errorCode, failingUrl);
+        mOnReceivedErrorHelper.notifyCalled(errorCode, "Error " + errorCode, failingUrl.getSpec());
     }
 
     @Override