[Android] Clean InterceptNavigationDelegateImpl of Tab deps

This CL removes the remaining deps on Tab from
InterceptNavigationDelegateImpl in order to make it suitable for
componentization and sharing with WebLayer. To do so, observance of the
Tab is moved to InterceptNavigationClientImpl. There is a slight
reordering of method ordering calls in order to so:
InterceptNavigationClientImpl starts observing the Tab immediately
after the construction of InterceptNavigationDelegateImpl whereas
that observance currently starts in the middle of the
InterceptNavigationDelegateImpl constructor. However, no observer
callbacks can happen as a result of the code running in the
InterceptNavigationDelegateImpl constructor (and the TabImpl#addObserver
call itself does not cause any observer callbacks), so this reordering
is harmless.

Bug: 1031465
Change-Id: Ic1f43d91076c414f095dd84f37bc24ca8a904e34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135749
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758754}

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

@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.tab;
 
 import android.app.Activity;
 
+import androidx.annotation.Nullable;
+
 import org.chromium.chrome.browser.AppHooks;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
@@ -13,7 +15,9 @@ import org.chromium.components.external_intents.AuthenticatorNavigationIntercept
 import org.chromium.components.external_intents.ExternalNavigationHandler;
 import org.chromium.components.external_intents.InterceptNavigationDelegateClient;
 import org.chromium.components.external_intents.RedirectHandlerImpl;
+import org.chromium.content_public.browser.NavigationHandle;
 import org.chromium.content_public.browser.WebContents;
+import org.chromium.ui.base.WindowAndroid;
 
 /**
  * Class that provides embedder-level information to InterceptNavigationDelegateImpl based off a
@@ -21,9 +25,35 @@ import org.chromium.content_public.browser.WebContents;
  */
 public class InterceptNavigationDelegateClientImpl implements InterceptNavigationDelegateClient {
     private TabImpl mTab;
+    private final TabObserver mTabObserver;
+    private InterceptNavigationDelegateImpl mInterceptNavigationDelegate;
 
     InterceptNavigationDelegateClientImpl(Tab tab) {
         mTab = (TabImpl) tab;
+        mTabObserver = new EmptyTabObserver() {
+            @Override
+            public void onContentChanged(Tab tab) {
+                mInterceptNavigationDelegate.associateWithWebContents(tab.getWebContents());
+            }
+
+            @Override
+            public void onActivityAttachmentChanged(Tab tab, @Nullable WindowAndroid window) {
+                if (window != null) {
+                    mInterceptNavigationDelegate.setExternalNavigationHandler(
+                            createExternalNavigationHandler());
+                }
+            }
+
+            @Override
+            public void onDidFinishNavigation(Tab tab, NavigationHandle navigation) {
+                mInterceptNavigationDelegate.onNavigationFinished(navigation);
+            }
+
+            @Override
+            public void onDestroyed(Tab tab) {
+                mInterceptNavigationDelegate.associateWithWebContents(null);
+            }
+        };
     }
 
     @Override
@@ -84,4 +114,15 @@ public class InterceptNavigationDelegateClientImpl implements InterceptNavigatio
     public void closeTab() {
         TabModelSelector.from(mTab).closeTab(mTab);
     }
+
+    public void initializeWithDelegate(InterceptNavigationDelegateImpl delegate) {
+        mInterceptNavigationDelegate = delegate;
+        mTab.addObserver(mTabObserver);
+    }
+
+    public void destroy() {
+        assert mInterceptNavigationDelegate != null;
+        mTab.removeObserver(mTabObserver);
+        mInterceptNavigationDelegate = null;
+    }
 }

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

@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.tab;
 
-import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.ContextUtils;
@@ -25,7 +24,6 @@ import org.chromium.content_public.browser.NavigationHandle;
 import org.chromium.content_public.browser.UiThreadTaskTraits;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.common.ConsoleMessageLevel;
-import org.chromium.ui.base.WindowAndroid;
 
 /**
  * Class that controls navigations and allows to intercept them. It is used on Android to 'convert'
@@ -36,12 +34,10 @@ import org.chromium.ui.base.WindowAndroid;
  * See https://crbug.com/732260.
  */
 public class InterceptNavigationDelegateImpl implements InterceptNavigationDelegate {
-    private final TabImpl mTab;
     private final AuthenticatorNavigationInterceptor mAuthenticatorHelper;
     private InterceptNavigationDelegateClient mClient;
     private @OverrideUrlLoadingResult int mLastOverrideUrlLoadingResult =
             OverrideUrlLoadingResult.NO_OVERRIDE;
-    private final TabObserver mDelegateObserver;
     private WebContents mWebContents;
     private ExternalNavigationHandler mExternalNavHandler;
 
@@ -55,48 +51,24 @@ public class InterceptNavigationDelegateImpl implements InterceptNavigationDeleg
      * Default constructor of {@link InterceptNavigationDelegateImpl}.
      */
     @VisibleForTesting
-    InterceptNavigationDelegateImpl(Tab tab, InterceptNavigationDelegateClient client) {
-        mTab = (TabImpl) tab;
+    InterceptNavigationDelegateImpl(InterceptNavigationDelegateClient client) {
         mClient = client;
         mAuthenticatorHelper = mClient.createAuthenticatorNavigationInterceptor();
-        mDelegateObserver = new EmptyTabObserver() {
-            @Override
-            public void onContentChanged(Tab tab) {
-                associateWithWebContents(tab.getWebContents());
-            }
-
-            @Override
-            public void onActivityAttachmentChanged(Tab tab, @Nullable WindowAndroid window) {
-                if (window != null) {
-                    setExternalNavigationHandler(mClient.createExternalNavigationHandler());
-                }
-            }
-
-            @Override
-            public void onDidFinishNavigation(Tab tab, NavigationHandle navigation) {
-                if (!navigation.hasCommitted() || !navigation.isInMainFrame()) return;
-                maybeUpdateNavigationHistory();
-            }
-
-            @Override
-            public void onDestroyed(Tab tab) {
-                associateWithWebContents(null);
-            }
-        };
-        mTab.addObserver(mDelegateObserver);
         associateWithWebContents(mClient.getWebContents());
     }
 
-    public void destroy() {
-        mTab.removeObserver(mDelegateObserver);
+    // Invoked by the client when a navigation has finished in the context in which this object is
+    // operating.
+    public void onNavigationFinished(NavigationHandle navigation) {
+        if (!navigation.hasCommitted() || !navigation.isInMainFrame()) return;
+        maybeUpdateNavigationHistory();
     }
 
-    @VisibleForTesting
-    void setExternalNavigationHandler(ExternalNavigationHandler handler) {
+    public void setExternalNavigationHandler(ExternalNavigationHandler handler) {
         mExternalNavHandler = handler;
     }
 
-    private void associateWithWebContents(WebContents webContents) {
+    public void associateWithWebContents(WebContents webContents) {
         if (mWebContents == webContents) return;
         mWebContents = webContents;
         if (mWebContents == null) return;
@@ -322,11 +294,6 @@ public class InterceptNavigationDelegateImpl implements InterceptNavigationDeleg
                 ContextUtils.getApplicationContext().getString(resId, url));
     }
 
-    @VisibleForTesting
-    static void initDelegateForTesting(Tab tab, InterceptNavigationDelegateImpl delegate) {
-        delegate.associateWithWebContents(tab.getWebContents());
-    }
-
     @NativeMethods
     interface Natives {
         void associateWithWebContents(

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

@@ -14,6 +14,7 @@ public class InterceptNavigationDelegateTabHelper implements UserData {
             InterceptNavigationDelegateTabHelper.class;
 
     private final InterceptNavigationDelegateImpl mInterceptNavigationDelegate;
+    private final InterceptNavigationDelegateClientImpl mInterceptNavigationDelegateClient;
 
     public static void createForTab(Tab tab) {
         assert get(tab) == null;
@@ -29,12 +30,14 @@ public class InterceptNavigationDelegateTabHelper implements UserData {
     }
 
     InterceptNavigationDelegateTabHelper(Tab tab) {
-        mInterceptNavigationDelegate = new InterceptNavigationDelegateImpl(
-                tab, new InterceptNavigationDelegateClientImpl(tab));
+        mInterceptNavigationDelegateClient = new InterceptNavigationDelegateClientImpl(tab);
+        mInterceptNavigationDelegate =
+                new InterceptNavigationDelegateImpl(mInterceptNavigationDelegateClient);
+        mInterceptNavigationDelegateClient.initializeWithDelegate(mInterceptNavigationDelegate);
     }
 
     @Override
     public void destroy() {
-        mInterceptNavigationDelegate.destroy();
+        mInterceptNavigationDelegateClient.destroy();
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateTest.java

@@ -100,16 +100,18 @@ public class InterceptNavigationDelegateTest {
         mActivity = mActivityTestRule.getActivity();
         final Tab tab = mActivity.getActivityTab();
         TestThreadUtils.runOnUiThreadBlocking(() -> {
-            InterceptNavigationDelegateImpl delegate = new InterceptNavigationDelegateImpl(
-                    tab, new InterceptNavigationDelegateClientImpl(tab)) {
+            InterceptNavigationDelegateClientImpl client =
+                    new InterceptNavigationDelegateClientImpl(tab);
+            InterceptNavigationDelegateImpl delegate = new InterceptNavigationDelegateImpl(client) {
                 @Override
                 public boolean shouldIgnoreNavigation(NavigationParams navigationParams) {
                     mNavParamHistory.add(navigationParams);
                     return super.shouldIgnoreNavigation(navigationParams);
                 }
             };
+            client.initializeWithDelegate(delegate);
             delegate.setExternalNavigationHandler(new TestExternalNavigationHandler());
-            InterceptNavigationDelegateImpl.initDelegateForTesting(tab, delegate);
+            delegate.associateWithWebContents(tab.getWebContents());
         });
         mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
     }