[webview_flutter_android] Fixes iframe navigation with onNavigationRequest

packages/webview_flutter/webview_flutter_android/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 3.16.1
+
+* Fixes iframe navigation being handled in the main frame when `NavigationDelegate.onNavigationRequest` is present.
+
 ## 3.16.0
 
 * Adds onReceivedHttpError WebViewClient callback to support

packages/webview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewClientHostApiImpl.java

@@ -88,7 +88,10 @@ public void onReceivedError(
     public boolean shouldOverrideUrlLoading(
         @NonNull WebView view, @NonNull WebResourceRequest request) {
       flutterApi.requestLoading(this, view, request, reply -> {});
-      return returnValueForShouldOverrideUrlLoading;
+
+      // The client is only allowed to stop navigations that target the main frame because
+      // overridden URLs are passed to `loadUrl` and `loadUrl` cannot load a subframe.
+      return request.isForMainFrame() && returnValueForShouldOverrideUrlLoading;
     }
 
     // Legacy codepath for < 24; newer versions use the variant above.
@@ -187,7 +190,10 @@ public void onReceivedError(
     public boolean shouldOverrideUrlLoading(
         @NonNull WebView view, @NonNull WebResourceRequest request) {
       flutterApi.requestLoading(this, view, request, reply -> {});
-      return returnValueForShouldOverrideUrlLoading;
+
+      // The client is only allowed to stop navigations that target the main frame because
+      // overridden URLs are passed to `loadUrl` and `loadUrl` cannot load a subframe.
+      return request.isForMainFrame() && returnValueForShouldOverrideUrlLoading;
     }
 
     // Legacy codepath for < Lollipop; newer versions use the variant above.

packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientTest.java → packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientCompatImplTest.java

@@ -5,6 +5,8 @@
 package io.flutter.plugins.webviewflutter;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
@@ -28,15 +30,13 @@
 import org.mockito.junit.MockitoJUnit;
 import org.mockito.junit.MockitoRule;
 
-public class WebViewClientTest {
+public class WebViewClientCompatImplTest {
   @Rule public MockitoRule mockitoRule = MockitoJUnit.rule();
 
   @Mock public WebViewClientFlutterApiImpl mockFlutterApi;
 
   @Mock public WebView mockWebView;
 
-  @Mock public WebViewClientCompatImpl mockWebViewClient;
-
   InstanceManager instanceManager;
   WebViewClientHostApiImpl hostApiImpl;
   WebViewClientCompatImpl webViewClient;
@@ -51,7 +51,7 @@ public void setUp() {
           @NonNull
           public WebViewClient createWebViewClient(
               @NonNull WebViewClientFlutterApiImpl flutterApi) {
-            webViewClient = (WebViewClientCompatImpl) super.createWebViewClient(flutterApi);
+            webViewClient = new WebViewClientCompatImpl(flutterApi);
             return webViewClient;
           }
         };
@@ -93,6 +93,54 @@ public void urlLoading() {
         .urlLoading(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
   }
 
+  @Test
+  public void urlLoadingForMainFrame() {
+    webViewClient.setReturnValueForShouldOverrideUrlLoading(false);
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.isForMainFrame()).thenReturn(true);
+
+    assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
+    verify(mockFlutterApi)
+        .requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
+  }
+
+  @Test
+  public void urlLoadingForMainFrameWithOverride() {
+    webViewClient.setReturnValueForShouldOverrideUrlLoading(true);
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.isForMainFrame()).thenReturn(true);
+
+    assertTrue(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
+    verify(mockFlutterApi)
+        .requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
+  }
+
+  @Test
+  public void urlLoadingNotForMainFrame() {
+    webViewClient.setReturnValueForShouldOverrideUrlLoading(false);
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.isForMainFrame()).thenReturn(false);
+
+    assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
+    verify(mockFlutterApi)
+        .requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
+  }
+
+  @Test
+  public void urlLoadingNotForMainFrameWithOverride() {
+    webViewClient.setReturnValueForShouldOverrideUrlLoading(true);
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.isForMainFrame()).thenReturn(false);
+
+    assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
+    verify(mockFlutterApi)
+        .requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
+  }
+
   @Test
   public void convertWebResourceRequestWithNullHeaders() {
     final Uri mockUri = mock(Uri.class);
@@ -111,6 +159,7 @@ public void convertWebResourceRequestWithNullHeaders() {
 
   @Test
   public void setReturnValueForShouldOverrideUrlLoading() {
+    WebViewClientHostApiImpl.WebViewClientCompatImpl mockWebViewClient = mock();
     final WebViewClientHostApiImpl webViewClientHostApi =
         new WebViewClientHostApiImpl(
             instanceManager,

packages/webview_flutter/webview_flutter_android/android/src/test/java/io/flutter/plugins/webviewflutter/WebViewClientImplTest.java

@@ -0,0 +1,190 @@
+// Copyright 2013 The Flutter 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 io.flutter.plugins.webviewflutter;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import android.net.Uri;
+import android.webkit.WebResourceRequest;
+import android.webkit.WebResourceResponse;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+import androidx.annotation.NonNull;
+import io.flutter.plugins.webviewflutter.WebViewClientHostApiImpl.WebViewClientCreator;
+import java.util.HashMap;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+public class WebViewClientImplTest {
+  @Rule public MockitoRule mockitoRule = MockitoJUnit.rule();
+
+  @Mock public WebViewClientFlutterApiImpl mockFlutterApi;
+
+  @Mock public WebView mockWebView;
+
+  InstanceManager instanceManager;
+  WebViewClientHostApiImpl hostApiImpl;
+  WebViewClientHostApiImpl.WebViewClientImpl webViewClient;
+
+  @Before
+  public void setUp() {
+    instanceManager = InstanceManager.create(identifier -> {});
+
+    final WebViewClientCreator webViewClientCreator =
+        new WebViewClientCreator() {
+          @Override
+          @NonNull
+          public WebViewClient createWebViewClient(
+              @NonNull WebViewClientFlutterApiImpl flutterApi) {
+            webViewClient = new WebViewClientHostApiImpl.WebViewClientImpl(flutterApi);
+            return webViewClient;
+          }
+        };
+
+    hostApiImpl =
+        new WebViewClientHostApiImpl(instanceManager, webViewClientCreator, mockFlutterApi);
+    hostApiImpl.create(1L);
+  }
+
+  @After
+  public void tearDown() {
+    instanceManager.stopFinalizationListener();
+  }
+
+  @Test
+  public void onPageStarted() {
+    webViewClient.onPageStarted(mockWebView, "https://www.google.com", null);
+    verify(mockFlutterApi)
+        .onPageStarted(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
+  }
+
+  @Test
+  public void onReceivedError() {
+    webViewClient.onReceivedError(mockWebView, 32, "description", "https://www.google.com");
+    verify(mockFlutterApi)
+        .onReceivedError(
+            eq(webViewClient),
+            eq(mockWebView),
+            eq(32L),
+            eq("description"),
+            eq("https://www.google.com"),
+            any());
+  }
+
+  @Test
+  public void urlLoading() {
+    webViewClient.shouldOverrideUrlLoading(mockWebView, "https://www.google.com");
+    verify(mockFlutterApi)
+        .urlLoading(eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), any());
+  }
+
+  @Test
+  public void urlLoadingForMainFrame() {
+    webViewClient.setReturnValueForShouldOverrideUrlLoading(false);
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.isForMainFrame()).thenReturn(true);
+
+    assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
+    verify(mockFlutterApi)
+        .requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
+  }
+
+  @Test
+  public void urlLoadingForMainFrameWithOverride() {
+    webViewClient.setReturnValueForShouldOverrideUrlLoading(true);
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.isForMainFrame()).thenReturn(true);
+
+    assertTrue(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
+    verify(mockFlutterApi)
+        .requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
+  }
+
+  @Test
+  public void urlLoadingNotForMainFrame() {
+    webViewClient.setReturnValueForShouldOverrideUrlLoading(false);
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.isForMainFrame()).thenReturn(false);
+
+    assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
+    verify(mockFlutterApi)
+        .requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
+  }
+
+  @Test
+  public void urlLoadingNotForMainFrameWithOverride() {
+    webViewClient.setReturnValueForShouldOverrideUrlLoading(true);
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.isForMainFrame()).thenReturn(false);
+
+    assertFalse(webViewClient.shouldOverrideUrlLoading(mockWebView, mockRequest));
+    verify(mockFlutterApi)
+        .requestLoading(eq(webViewClient), eq(mockWebView), eq(mockRequest), any());
+  }
+
+  @Test
+  public void convertWebResourceRequestWithNullHeaders() {
+    final Uri mockUri = mock(Uri.class);
+    when(mockUri.toString()).thenReturn("");
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.getMethod()).thenReturn("method");
+    when(mockRequest.getUrl()).thenReturn(mockUri);
+    when(mockRequest.isForMainFrame()).thenReturn(true);
+    when(mockRequest.getRequestHeaders()).thenReturn(null);
+
+    final GeneratedAndroidWebView.WebResourceRequestData data =
+        WebViewClientFlutterApiImpl.createWebResourceRequestData(mockRequest);
+    assertEquals(data.getRequestHeaders(), new HashMap<String, String>());
+  }
+
+  @Test
+  public void doUpdateVisitedHistory() {
+    webViewClient.doUpdateVisitedHistory(mockWebView, "https://www.google.com", true);
+    verify(mockFlutterApi)
+        .doUpdateVisitedHistory(
+            eq(webViewClient), eq(mockWebView), eq("https://www.google.com"), eq(true), any());
+  }
+
+  @Test
+  public void onReceivedHttpError() {
+    final Uri mockUri = mock(Uri.class);
+    when(mockUri.toString()).thenReturn("");
+
+    final WebResourceRequest mockRequest = mock(WebResourceRequest.class);
+    when(mockRequest.getMethod()).thenReturn("method");
+    when(mockRequest.getUrl()).thenReturn(mockUri);
+    when(mockRequest.isForMainFrame()).thenReturn(true);
+    when(mockRequest.getRequestHeaders()).thenReturn(null);
+
+    final WebResourceResponse mockResponse = mock(WebResourceResponse.class);
+    when(mockResponse.getStatusCode()).thenReturn(404);
+
+    webViewClient.onReceivedHttpError(mockWebView, mockRequest, mockResponse);
+    verify(mockFlutterApi)
+        .onReceivedHttpError(
+            eq(webViewClient),
+            eq(mockWebView),
+            any(WebResourceRequest.class),
+            any(WebResourceResponse.class),
+            any());
+  }
+}

packages/webview_flutter/webview_flutter_android/example/lib/main.dart

@@ -23,10 +23,10 @@ const String kNavigationExamplePage = '''
 <head><title>Navigation Delegate Example</title></head>
 <body>
 <p>
-The navigation delegate is set to block navigation to the youtube website.
+The navigation delegate is set to block navigation to the pub.dev website.
 </p>
 <ul>
-<ul><a href="https://www.youtube.com/">https://www.youtube.com/</a></ul>
+<ul><a href="https://pub.dev/">https://pub.dev/</a></ul>
 <ul><a href="https://www.google.com/">https://www.google.com/</a></ul>
 </ul>
 </body>

packages/webview_flutter/webview_flutter_android/lib/src/android_webview_controller.dart

@@ -1464,7 +1464,11 @@ class AndroidNavigationDelegate extends PlatformNavigationDelegate {
     final LoadRequestCallback? onLoadRequest = _onLoadRequest;
     final NavigationRequestCallback? onNavigationRequest = _onNavigationRequest;
 
-    if (onNavigationRequest == null || onLoadRequest == null) {
+    // The client is only allowed to stop navigations that target the main frame because
+    // overridden URLs are passed to `loadUrl` and `loadUrl` cannot load a subframe.
+    if (!isForMainFrame ||
+        onNavigationRequest == null ||
+        onLoadRequest == null) {
       return;
     }
 

packages/webview_flutter/webview_flutter_android/pubspec.yaml

@@ -2,7 +2,7 @@ name: webview_flutter_android
 description: A Flutter plugin that provides a WebView widget on Android.
 repository: https://github.com/flutter/packages/tree/main/packages/webview_flutter/webview_flutter_android
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+webview%22
-version: 3.16.0
+version: 3.16.1
 
 environment:
   sdk: ^3.1.0