Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[webview_flutter] Fixes bug when onNavigationRequestCallback returns false #5981

Merged
merged 2 commits into from
Jun 21, 2022
Merged

[webview_flutter] Fixes bug when onNavigationRequestCallback returns false #5981

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eaab52e
Fixes bug when onNavigationRequestCallback returns false
mvanbeusekom Jun 15, 2022
c06313d
Improve expectation in unit test
mvanbeusekom Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/webview_flutter/webview_flutter_android/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.8.13

* Fixes a bug which causes an exception when the `onNavigationRequestCallback` return `false`.

## 2.8.12

* Bumps mockito-inline from 3.11.1 to 4.6.1.
Expand Down
8 changes: 4 additions & 4 deletions packages/webview_flutter/webview_flutter_android/lib/webview_android_widget.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,8 @@ class WebViewAndroidWebViewClient extends android_webview.WebViewClient {

if (returnValue is bool && returnValue) {
loadUrl!(url, <String, String>{});
} else {
(returnValue as Future<bool>).then((bool shouldLoadUrl) {
} else if (returnValue is Future<bool>) {
returnValue.then((bool shouldLoadUrl) {
if (shouldLoadUrl) {
loadUrl!(url, <String, String>{});
}
Expand All @@ -677,8 +677,8 @@ class WebViewAndroidWebViewClient extends android_webview.WebViewClient {

if (returnValue is bool && returnValue) {
loadUrl!(request.url, <String, String>{});
} else {
(returnValue as Future<bool>).then((bool shouldLoadUrl) {
} else if (returnValue is Future<bool>) {
returnValue.then((bool shouldLoadUrl) {
if (shouldLoadUrl) {
loadUrl!(request.url, <String, String>{});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/webview_flutter/webview_flutter_android/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: webview_flutter_android
description: A Flutter plugin that provides a WebView widget on Android.
repository: https://github.com/flutter/plugins/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: 2.8.12
version: 2.8.13

environment:
sdk: ">=2.14.0 <3.0.0"
Expand Down
192 changes: 192 additions & 0 deletions packages/webview_flutter/webview_flutter_android/test/webview_android_widget_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';
import 'dart:typed_data';

import 'package:flutter/widgets.dart';
Expand All @@ -24,6 +25,7 @@ import 'webview_android_widget_test.mocks.dart';
android_webview.WebSettings,
android_webview.WebStorage,
android_webview.WebView,
android_webview.WebResourceRequest,
WebViewAndroidDownloadListener,
WebViewAndroidJavaScriptChannel,
WebViewAndroidWebChromeClient,
Expand Down Expand Up @@ -843,4 +845,194 @@ void main() {
verify(mockPlatformHostApi.setWebContentsDebuggingEnabled(false));
});
});

group('WebViewAndroidWebViewClient', () {
test(
'urlLoading should call loadUrl when onNavigationRequestCallback returns true',
() {
final Completer<void> completer = Completer<void>();
final WebViewAndroidWebViewClient webViewClient =
WebViewAndroidWebViewClient.handlesNavigation(
onPageStartedCallback: (_) {},
onPageFinishedCallback: (_) {},
onWebResourceErrorCallback: (_) {},
onNavigationRequestCallback: ({
required bool isForMainFrame,
required String url,
}) =>
true,
loadUrl: (String url, Map<String, String>? headers) async {
completer.complete();
});

webViewClient.urlLoading(MockWebView(), 'https://flutter.dev');
expect(completer.isCompleted, isTrue);
});

test(
'urlLoading should call loadUrl when onNavigationRequestCallback returns a Future true',
() async {
final Completer<void> completer = Completer<void>();
final WebViewAndroidWebViewClient webViewClient =
WebViewAndroidWebViewClient.handlesNavigation(
onPageStartedCallback: (_) {},
onPageFinishedCallback: (_) {},
onWebResourceErrorCallback: (_) {},
onNavigationRequestCallback: ({
required bool isForMainFrame,
required String url,
}) =>
Future<bool>.value(true),
loadUrl: (String url, Map<String, String>? headers) async {
completer.complete();
});

webViewClient.urlLoading(MockWebView(), 'https://flutter.dev');
await completer.future;
expect(completer.isCompleted, isTrue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You should be able to do

Suggested change
await completer.future;
expect(completer.isCompleted, isTrue);
expect(completer.future, completes);

});

test(
'urlLoading should not call laodUrl when onNavigationRequestCallback returns false',
() async {
final WebViewAndroidWebViewClient webViewClient =
WebViewAndroidWebViewClient.handlesNavigation(
onPageStartedCallback: (_) {},
onPageFinishedCallback: (_) {},
onWebResourceErrorCallback: (_) {},
onNavigationRequestCallback: ({
required bool isForMainFrame,
required String url,
}) =>
false,
loadUrl: (String url, Map<String, String>? headers) async {
fail(
'loadUrl should not be called if onNavigationRequestCallback returns false.');
});

webViewClient.urlLoading(MockWebView(), 'https://flutter.dev');
});

test(
'urlLoading should not call loadUrl when onNavigationRequestCallback returns a Future false',
() {
final WebViewAndroidWebViewClient webViewClient =
WebViewAndroidWebViewClient.handlesNavigation(
onPageStartedCallback: (_) {},
onPageFinishedCallback: (_) {},
onWebResourceErrorCallback: (_) {},
onNavigationRequestCallback: ({
required bool isForMainFrame,
required String url,
}) =>
Future<bool>.value(false),
loadUrl: (String url, Map<String, String>? headers) async {
fail(
'loadUrl should not be called if onNavigationRequestCallback returns false.');
});

webViewClient.urlLoading(MockWebView(), 'https://flutter.dev');
});

test(
'requestLoading should call loadUrl when onNavigationRequestCallback returns true',
() {
final Completer<void> completer = Completer<void>();
final MockWebResourceRequest mockRequest = MockWebResourceRequest();
when(mockRequest.isForMainFrame).thenReturn(true);
when(mockRequest.url).thenReturn('https://flutter.dev');
final WebViewAndroidWebViewClient webViewClient =
WebViewAndroidWebViewClient.handlesNavigation(
onPageStartedCallback: (_) {},
onPageFinishedCallback: (_) {},
onWebResourceErrorCallback: (_) {},
onNavigationRequestCallback: ({
required bool isForMainFrame,
required String url,
}) =>
true,
loadUrl: (String url, Map<String, String>? headers) async {
expect(url, 'https://flutter.dev');
completer.complete();
});

webViewClient.requestLoading(MockWebView(), mockRequest);
expect(completer.isCompleted, isTrue);
});

test(
'requestLoading should call loadUrl when onNavigationRequestCallback returns a Future true',
() async {
final Completer<void> completer = Completer<void>();
final MockWebResourceRequest mockRequest = MockWebResourceRequest();
when(mockRequest.isForMainFrame).thenReturn(true);
when(mockRequest.url).thenReturn('https://flutter.dev');
final WebViewAndroidWebViewClient webViewClient =
WebViewAndroidWebViewClient.handlesNavigation(
onPageStartedCallback: (_) {},
onPageFinishedCallback: (_) {},
onWebResourceErrorCallback: (_) {},
onNavigationRequestCallback: ({
required bool isForMainFrame,
required String url,
}) =>
Future<bool>.value(true),
loadUrl: (String url, Map<String, String>? headers) async {
expect(url, 'https://flutter.dev');
completer.complete();
});

webViewClient.requestLoading(MockWebView(), mockRequest);
await completer.future;
expect(completer.isCompleted, isTrue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: same as above

Suggested change
await completer.future;
expect(completer.isCompleted, isTrue);
expect(completer.future, completes);

});

test(
'requestLoading should not call onLoadUrlCallback when onNavigationRequestCallback returns false',
() {
final MockWebResourceRequest mockRequest = MockWebResourceRequest();
when(mockRequest.isForMainFrame).thenReturn(true);
when(mockRequest.url).thenReturn('https://flutter.dev');
final WebViewAndroidWebViewClient webViewClient =
WebViewAndroidWebViewClient.handlesNavigation(
onPageStartedCallback: (_) {},
onPageFinishedCallback: (_) {},
onWebResourceErrorCallback: (_) {},
onNavigationRequestCallback: ({
required bool isForMainFrame,
required String url,
}) =>
false,
loadUrl: (String url, Map<String, String>? headers) {
fail(
'loadUrl should not be called if onNavigationRequestCallback returns false.');
});

webViewClient.requestLoading(MockWebView(), mockRequest);
});

test(
'requestLoading should not call onLoadUrlCallback when onNavigationRequestCallback returns a Future false',
() {
final MockWebResourceRequest mockRequest = MockWebResourceRequest();
when(mockRequest.isForMainFrame).thenReturn(true);
when(mockRequest.url).thenReturn('https://flutter.dev');
final WebViewAndroidWebViewClient webViewClient =
WebViewAndroidWebViewClient.handlesNavigation(
onPageStartedCallback: (_) {},
onPageFinishedCallback: (_) {},
onWebResourceErrorCallback: (_) {},
onNavigationRequestCallback: ({
required bool isForMainFrame,
required String url,
}) =>
Future<bool>.value(false),
loadUrl: (String url, Map<String, String>? headers) {
fail(
'loadUrl should not be called if onNavigationRequestCallback returns false.');
});

webViewClient.requestLoading(MockWebView(), mockRequest);
});
});
}
30 changes: 30 additions & 0 deletions packages/webview_flutter/webview_flutter_android/test/webview_android_widget_test.mocks.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,36 @@ class MockWebView extends _i1.Mock implements _i2.WebView {
returnValueForMissingStub: Future<void>.value()) as _i4.Future<void>);
}

/// A class which mocks [WebResourceRequest].
///
/// See the documentation for Mockito's code generation for more information.
class MockWebResourceRequest extends _i1.Mock
implements _i2.WebResourceRequest {
MockWebResourceRequest() {
_i1.throwOnMissingStub(this);
}

@override
String get url =>
(super.noSuchMethod(Invocation.getter(#url), returnValue: '') as String);
@override
bool get isForMainFrame => (super
.noSuchMethod(Invocation.getter(#isForMainFrame), returnValue: false)
as bool);
@override
bool get hasGesture =>
(super.noSuchMethod(Invocation.getter(#hasGesture), returnValue: false)
as bool);
@override
String get method =>
(super.noSuchMethod(Invocation.getter(#method), returnValue: '')
as String);
@override
Map<String, String> get requestHeaders =>
(super.noSuchMethod(Invocation.getter(#requestHeaders),
returnValue: <String, String>{}) as Map<String, String>);
}

/// A class which mocks [WebViewAndroidDownloadListener].
///
/// See the documentation for Mockito's code generation for more information.
Expand Down