WebLayer: add NavigateParams to NavigationController.navigate()

This struct should more or less mirror LoadURLParams, or at least
as much of that struct as necessary. For now it only contains
mShouldReplaceCurrentEntry.

This makes replace() obsolete. Since this is designed to be merged
back to 83, replace() was added in 82, and 82 did not make it to
stable, replace() can be removed without the normal deprecation
process.

Bug: 1070851
Change-Id: I0845bab184a182e9a616b01b6115615275422fc9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2150968Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759766}

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/NavigationTest.java

@@ -32,6 +32,7 @@ import org.chromium.weblayer.NavigationState;
 import org.chromium.weblayer.Tab;
 import org.chromium.weblayer.TabCallback;
 import org.chromium.weblayer.shell.InstrumentationActivity;
+import org.chromium.weblayer_private.interfaces.NavigateParams;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -244,20 +245,56 @@ public class NavigationTest {
         assertTrue(startProgressIndex < finishProgressIndex);
     }
 
+    @Test
+    @SmallTest
+    public void testDefaultNavigateParams() throws Exception {
+        InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(URL1);
+        setNavigationCallback(activity);
+
+        NavigateParams params = new NavigateParams();
+        navigateAndWaitForCompletion(URL2,
+                ()
+                        -> activity.getTab().getNavigationController().navigate(
+                                Uri.parse(URL2), params));
+        runOnUiThreadBlocking(() -> {
+            NavigationController navigationController = activity.getTab().getNavigationController();
+            assertFalse(navigationController.canGoForward());
+            assertTrue(navigationController.canGoBack());
+            assertEquals(2, navigationController.getNavigationListSize());
+        });
+    }
+
     @Test
     @SmallTest
     public void testReplace() throws Exception {
         InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(URL1);
         setNavigationCallback(activity);
 
-        navigateAndWaitForCompletion(
-                URL2, () -> activity.getTab().getNavigationController().replace(Uri.parse(URL2)));
+        final NavigateParams params = new NavigateParams();
+        params.mShouldReplaceCurrentEntry = true;
+        navigateAndWaitForCompletion(URL2,
+                ()
+                        -> activity.getTab().getNavigationController().navigate(
+                                Uri.parse(URL2), params));
         runOnUiThreadBlocking(() -> {
             NavigationController navigationController = activity.getTab().getNavigationController();
             assertFalse(navigationController.canGoForward());
             assertFalse(navigationController.canGoBack());
             assertEquals(1, navigationController.getNavigationListSize());
         });
+
+        // Verify that a default NavigateParams does not replace.
+        final NavigateParams params2 = new NavigateParams();
+        navigateAndWaitForCompletion(URL3,
+                ()
+                        -> activity.getTab().getNavigationController().navigate(
+                                Uri.parse(URL3), params2));
+        runOnUiThreadBlocking(() -> {
+            NavigationController navigationController = activity.getTab().getNavigationController();
+            assertFalse(navigationController.canGoForward());
+            assertTrue(navigationController.canGoBack());
+            assertEquals(2, navigationController.getNavigationListSize());
+        });
     }
 
     @Test

weblayer/browser/java/BUILD.gn

@@ -207,6 +207,7 @@ android_library("interfaces_java") {
     "org/chromium/weblayer_private/interfaces/DownloadError.java",
     "org/chromium/weblayer_private/interfaces/DownloadState.java",
     "org/chromium/weblayer_private/interfaces/LoadError.java",
+    "org/chromium/weblayer_private/interfaces/NavigateParams.java",
     "org/chromium/weblayer_private/interfaces/NavigationState.java",
     "org/chromium/weblayer_private/interfaces/NewTabType.java",
     "org/chromium/weblayer_private/interfaces/ObjectWrapper.java",

weblayer/browser/java/org/chromium/weblayer_private/NavigationControllerImpl.java

@@ -11,6 +11,7 @@ import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.weblayer_private.interfaces.INavigationController;
 import org.chromium.weblayer_private.interfaces.INavigationControllerClient;
+import org.chromium.weblayer_private.interfaces.NavigateParams;
 import org.chromium.weblayer_private.interfaces.StrictModeWorkaround;
 
 /**
@@ -30,17 +31,18 @@ public final class NavigationControllerImpl extends INavigationController.Stub {
     }
 
     @Override
-    public void navigate(String uri) {
+    public void navigate(String uri, NavigateParams params) throws RemoteException {
         StrictModeWorkaround.apply();
-        NavigationControllerImplJni.get().navigate(
-                mNativeNavigationController, NavigationControllerImpl.this, uri);
-    }
-
-    @Override
-    public void replace(String uri) {
-        StrictModeWorkaround.apply();
-        NavigationControllerImplJni.get().replace(
-                mNativeNavigationController, NavigationControllerImpl.this, uri);
+        if (WebLayerFactoryImpl.getClientMajorVersion() < 83) {
+            assert params == null;
+        }
+        if (params == null) {
+            NavigationControllerImplJni.get().navigate(
+                    mNativeNavigationController, NavigationControllerImpl.this, uri);
+        } else {
+            NavigationControllerImplJni.get().navigateWithParams(mNativeNavigationController,
+                    NavigationControllerImpl.this, uri, params.mShouldReplaceCurrentEntry);
+        }
     }
 
     @Override
@@ -173,8 +175,8 @@ public final class NavigationControllerImpl extends INavigationController.Stub {
         long getNavigationController(long tab);
         void navigate(
                 long nativeNavigationControllerImpl, NavigationControllerImpl caller, String uri);
-        void replace(
-                long nativeNavigationControllerImpl, NavigationControllerImpl caller, String uri);
+        void navigateWithParams(long nativeNavigationControllerImpl,
+                NavigationControllerImpl caller, String uri, boolean shouldReplaceCurrentEntry);
         void goBack(long nativeNavigationControllerImpl, NavigationControllerImpl caller);
         void goForward(long nativeNavigationControllerImpl, NavigationControllerImpl caller);
         boolean canGoBack(long nativeNavigationControllerImpl, NavigationControllerImpl caller);

weblayer/browser/java/org/chromium/weblayer_private/interfaces/INavigationController.aidl

@@ -4,8 +4,10 @@
 
 package org.chromium.weblayer_private.interfaces;
 
+import org.chromium.weblayer_private.interfaces.NavigateParams;
+
 interface INavigationController {
-  void navigate(in String uri) = 0;
+  void navigate(in String uri, in NavigateParams params) = 0;
 
   void goBack() = 1;
 
@@ -29,5 +31,6 @@ interface INavigationController {
 
   String getNavigationEntryTitle(in int index) = 11;
 
-  void replace(in String uri) = 12;
+  // Added in 82, removed in 83.
+  // void replace(in String uri) = 12;
 }

weblayer/browser/java/org/chromium/weblayer_private/interfaces/NavigateParams.aidl

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer_private.interfaces;
+
+parcelable NavigateParams;

weblayer/browser/java/org/chromium/weblayer_private/interfaces/NavigateParams.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer_private.interfaces;
+
+import android.os.Parcel;
+import android.os.Parcelable;
+
+/**
+ * Extra optional parameters for {@link NavigationController#navigate}.
+ *
+ * Default values should be kept in sync with C++ NavigationController::LoadURLParams.
+ *
+ * @since 83
+ */
+public final class NavigateParams implements Parcelable {
+    // Indicates whether the navigation should replace the current navigation entry in the history
+    // stack.
+    public boolean mShouldReplaceCurrentEntry = false;
+
+    public static final Parcelable.Creator<NavigateParams> CREATOR =
+            new Parcelable.Creator<NavigateParams>() {
+                @Override
+                public NavigateParams createFromParcel(Parcel in) {
+                    return new NavigateParams(in);
+                }
+
+                @Override
+                public NavigateParams[] newArray(int size) {
+                    return new NavigateParams[size];
+                }
+            };
+
+    public NavigateParams() {}
+
+    private NavigateParams(Parcel in) {
+        readFromParcel(in);
+    }
+
+    @Override
+    public void writeToParcel(Parcel out, int flags) {
+        out.writeInt(mShouldReplaceCurrentEntry ? 1 : 0);
+    }
+
+    public void readFromParcel(Parcel in) {
+        mShouldReplaceCurrentEntry = in.readInt() == 1;
+    }
+
+    @Override
+    public int describeContents() {
+        return 0;
+    }
+}

weblayer/browser/navigation_controller_impl.cc

@@ -117,10 +117,15 @@ void NavigationControllerImpl::Navigate(JNIEnv* env,
   Navigate(GURL(base::android::ConvertJavaStringToUTF8(env, url)));
 }
 
-void NavigationControllerImpl::Replace(JNIEnv* env,
-                                       const JavaParamRef<jobject>& obj,
-                                       const JavaParamRef<jstring>& url) {
-  Replace(GURL(base::android::ConvertJavaStringToUTF8(env, url)));
+void NavigationControllerImpl::NavigateWithParams(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj,
+    const JavaParamRef<jstring>& url,
+    jboolean should_replace_current_entry) {
+  content::NavigationController::LoadURLParams params(
+      GURL(base::android::ConvertJavaStringToUTF8(env, url)));
+  params.should_replace_current_entry = should_replace_current_entry;
+  DoNavigate(std::move(params));
 }
 
 ScopedJavaLocalRef<jstring>
@@ -176,10 +181,13 @@ void NavigationControllerImpl::Navigate(const GURL& url) {
   DoNavigate(content::NavigationController::LoadURLParams(url));
 }
 
-void NavigationControllerImpl::Replace(const GURL& url) {
-  content::NavigationController::LoadURLParams params(url);
-  params.should_replace_current_entry = true;
-  DoNavigate(std::move(params));
+void NavigationControllerImpl::Navigate(
+    const GURL& url,
+    const NavigationController::NavigateParams& params) {
+  content::NavigationController::LoadURLParams load_params(url);
+  load_params.should_replace_current_entry =
+      params.should_replace_current_entry;
+  DoNavigate(std::move(load_params));
 }
 
 void NavigationControllerImpl::GoBack() {

weblayer/browser/navigation_controller_impl.h

@@ -44,9 +44,10 @@ class NavigationControllerImpl : public NavigationController,
   void Navigate(JNIEnv* env,
                 const base::android::JavaParamRef<jobject>& obj,
                 const base::android::JavaParamRef<jstring>& url);
-  void Replace(JNIEnv* env,
-               const base::android::JavaParamRef<jobject>& obj,
-               const base::android::JavaParamRef<jstring>& url);
+  void NavigateWithParams(JNIEnv* env,
+                          const base::android::JavaParamRef<jobject>& obj,
+                          const base::android::JavaParamRef<jstring>& url,
+                          jboolean should_replace_current_entry);
   void GoBack(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj) {
     GoBack();
   }
@@ -100,7 +101,7 @@ class NavigationControllerImpl : public NavigationController,
   void AddObserver(NavigationObserver* observer) override;
   void RemoveObserver(NavigationObserver* observer) override;
   void Navigate(const GURL& url) override;
-  void Replace(const GURL& url) override;
+  void Navigate(const GURL& url, const NavigateParams& params) override;
   void GoBack() override;
   void GoForward() override;
   bool CanGoBack() override;

weblayer/public/java/org/chromium/weblayer/NavigationController.java

@@ -8,6 +8,7 @@ import android.net.Uri;
 import android.os.RemoteException;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 
 import org.chromium.weblayer_private.interfaces.APICallException;
 import org.chromium.weblayer_private.interfaces.IClientNavigation;
@@ -15,6 +16,7 @@ import org.chromium.weblayer_private.interfaces.INavigation;
 import org.chromium.weblayer_private.interfaces.INavigationController;
 import org.chromium.weblayer_private.interfaces.INavigationControllerClient;
 import org.chromium.weblayer_private.interfaces.ITab;
+import org.chromium.weblayer_private.interfaces.NavigateParams;
 import org.chromium.weblayer_private.interfaces.StrictModeWorkaround;
 
 /**
@@ -41,28 +43,24 @@ public class NavigationController {
     }
 
     public void navigate(@NonNull Uri uri) {
-        ThreadCheck.ensureOnUiThread();
-        try {
-            mNavigationController.navigate(uri.toString());
-        } catch (RemoteException e) {
-            throw new APICallException(e);
-        }
+        navigate(uri, null);
     }
 
     /**
-     * Navigates to the given URI, replacing the current navigation entry rather than appending.
+     * Navigates to the given URI, with optional settings.
      *
      * @param uri the destination URI.
+     * @param params extra parameters for the navigation.
      *
-     * @since 82
+     * @since 83
      */
-    public void replace(@NonNull Uri uri) {
+    public void navigate(@NonNull Uri uri, @Nullable NavigateParams params) {
         ThreadCheck.ensureOnUiThread();
-        if (WebLayer.getSupportedMajorVersionInternal() < 82) {
+        if ((WebLayer.getSupportedMajorVersionInternal() < 83) && (params != null)) {
             throw new UnsupportedOperationException();
         }
         try {
-            mNavigationController.replace(uri.toString());
+            mNavigationController.navigate(uri.toString(), params);
         } catch (RemoteException e) {
             throw new APICallException(e);
         }

weblayer/public/navigation_controller.h

@@ -15,7 +15,13 @@ class NavigationObserver;
 
 class NavigationController {
  public:
-  virtual ~NavigationController() {}
+  // The members of this struct and their defaults should be kept in sync with
+  // |NavigationController::LoadURLParams|.
+  struct NavigateParams {
+    bool should_replace_current_entry = false;
+  };
+
+  virtual ~NavigationController() = default;
 
   virtual void AddObserver(NavigationObserver* observer) = 0;
 
@@ -23,7 +29,7 @@ class NavigationController {
 
   virtual void Navigate(const GURL& url) = 0;
 
-  virtual void Replace(const GURL& url) = 0;
+  virtual void Navigate(const GURL& url, const NavigateParams& params) = 0;
 
   virtual void GoBack() = 0;